]> git.ipfire.org Git - thirdparty/chrony.git/commitdiff
sources: improve handling of dump files and their format
authorMiroslav Lichvar <mlichvar@redhat.com>
Wed, 3 Feb 2021 16:13:39 +0000 (17:13 +0100)
committerMiroslav Lichvar <mlichvar@redhat.com>
Thu, 4 Feb 2021 16:44:27 +0000 (17:44 +0100)
Check for write errors when saving dump files. Don't save files with no
samples. Add more sanity checks for loaded data.

Extend the file format to include an identifier, the reachability
register, leap status, name, and authentication flag. Avoid loading
unauthenticated data after switching authentication on. Change format
and order of some fields to simplify parsing. Drop fields that were kept
only for compatibility.

The dump files now contain all information needed to perform the source
selection and update the reference.

There is no support kept for the old file format. Loading of old dump
files will fail after upgrading to new version.

sources.c
sourcestats.c
sourcestats.h

index e2621862e53bf4befc1646abb2a32ba03c71e5f3..04f292c942a16ff327e3ae9757de51ea794a24ee 100644 (file)
--- a/sources.c
+++ b/sources.c
@@ -177,6 +177,9 @@ static double reselect_distance;
 static double stratum_weight;
 static double combine_limit;
 
+/* Identifier of the dump file */
+#define DUMP_IDENTIFIER "SRC0\n"
+
 /* ================================================== */
 /* Forward prototype */
 
@@ -1325,24 +1328,60 @@ add_dispersion(double dispersion, void *anything)
 
 /* ================================================== */
 
-static
-FILE *open_dumpfile(SRC_Instance inst, char mode)
+static int
+get_dumpfile(SRC_Instance inst, char *filename, size_t len)
+{
+  /* Use the IP address, or reference ID with reference clocks */
+  switch (inst->type) {
+    case SRC_NTP:
+      if (!UTI_IsIPReal(inst->ip_addr) ||
+          snprintf(filename, len, "%s", source_to_string(inst)) >= len)
+        return 0;
+      break;
+    case SRC_REFCLOCK:
+      if (snprintf(filename, len, "refid:%08"PRIx32, inst->ref_id) >= len)
+        return 0;
+      break;
+    default:
+      assert(0);
+  }
+
+  return 1;
+}
+
+/* ================================================== */
+
+static void
+save_source(SRC_Instance inst)
 {
-  char filename[64], *dumpdir;
+  char filename[64], *dumpdir, *ntp_name;
+  FILE *f;
 
   dumpdir = CNF_GetDumpDir();
   if (!dumpdir)
-    return NULL;
+    return;
 
-  /* Include IP address in the name for NTP sources, or reference ID in hex */
-  if (inst->type == SRC_NTP && UTI_IsIPReal(inst->ip_addr))
-    snprintf(filename, sizeof (filename), "%s", source_to_string(inst));
-  else if (inst->type == SRC_REFCLOCK)
-    snprintf(filename, sizeof (filename), "refid:%08"PRIx32, inst->ref_id);
-  else
-    return NULL;
+  if (!get_dumpfile(inst, filename, sizeof (filename)))
+    return;
+
+  f = UTI_OpenFile(dumpdir, filename, ".dat", 'w', 0644);
+  if (!f)
+    return;
+
+  ntp_name = inst->type == SRC_NTP ? NSR_GetName(inst->ip_addr) : NULL;
+
+  if (fprintf(f, "%s%s\n%d %o %d %d %d\n",
+              DUMP_IDENTIFIER, ntp_name, inst->authenticated,
+              (unsigned int)inst->reachability, inst->reachability_size,
+              inst->stratum, (int)inst->leap) < 0 ||
+      !SST_SaveToFile(inst->stats, f)) {
+    fclose(f);
+    if (!UTI_RemoveFile(dumpdir, filename, ".dat"))
+      ;
+    return;
+  }
 
-  return UTI_OpenFile(dumpdir, filename, ".dat", mode, 0644);
+  fclose(f);
 }
 
 /* ================================================== */
@@ -1351,16 +1390,60 @@ FILE *open_dumpfile(SRC_Instance inst, char mode)
 void
 SRC_DumpSources(void)
 {
-  FILE *out;
   int i;
 
-  for (i = 0; i < n_sources; i++) {
-    out = open_dumpfile(sources[i], 'w');
-    if (!out)
-      continue;
-    SST_SaveToFile(sources[i]->stats, out);
-    fclose(out);
+  for (i = 0; i < n_sources; i++)
+    save_source(sources[i]);
+}
+
+/* ================================================== */
+
+#define MAX_WORDS 1
+
+static void
+load_source(SRC_Instance inst)
+{
+  char filename[64], line[256], *dumpdir, *ntp_name, *words[MAX_WORDS];
+  int auth, leap, reach_size, stratum;
+  unsigned int reach;
+  FILE *f;
+
+  dumpdir = CNF_GetDumpDir();
+  if (!dumpdir)
+    return;
+
+  if (!get_dumpfile(inst, filename, sizeof (filename)))
+    return;
+
+  f = UTI_OpenFile(dumpdir, filename, ".dat", 'r', 0);
+  if (!f)
+    return;
+
+  ntp_name = inst->type == SRC_NTP ? NSR_GetName(inst->ip_addr) : NULL;
+
+  if (!fgets(line, sizeof (line), f) || strcmp(line, DUMP_IDENTIFIER) != 0 ||
+      !fgets(line, sizeof (line), f) || UTI_SplitString(line, words, MAX_WORDS) != 1 ||
+        (inst->type == SRC_NTP && (!ntp_name || strcmp(words[0], ntp_name) != 0)) ||
+      !fgets(line, sizeof (line), f) ||
+        sscanf(words[0], "%d %o %d %d %d",
+               &auth, &reach, &reach_size, &stratum, &leap) != 5 ||
+        (!auth && inst->authenticated) ||
+        stratum < 1 || stratum >= NTP_MAX_STRATUM ||
+        leap < LEAP_Normal || leap >= LEAP_Unsynchronised ||
+      !SST_LoadFromFile(inst->stats, f)) {
+    LOG(LOGS_WARN, "Could not load dump file for %s", source_to_string(inst));
+    fclose(f);
+    return;
   }
+
+  inst->reachability = reach & ((1U << SOURCE_REACH_BITS) - 1);
+  inst->reachability_size = CLAMP(0, reach_size, SOURCE_REACH_BITS);
+  inst->stratum = stratum;
+  inst->leap = leap;
+
+  LOG(LOGS_INFO, "Loaded dump file for %s", source_to_string(inst));
+
+  fclose(f);
 }
 
 /* ================================================== */
@@ -1368,20 +1451,10 @@ SRC_DumpSources(void)
 void
 SRC_ReloadSources(void)
 {
-  FILE *in;
   int i;
 
   for (i = 0; i < n_sources; i++) {
-    in = open_dumpfile(sources[i], 'r');
-    if (!in)
-      continue;
-    if (!SST_LoadFromFile(sources[i]->stats, in))
-      LOG(LOGS_WARN, "Could not load dump file for %s",
-          source_to_string(sources[i]));
-    else
-      LOG(LOGS_INFO, "Loaded dump file for %s",
-          source_to_string(sources[i]));
-    fclose(in);
+    load_source(sources[i]);
   }
 }
 
index f35ba841b7660defa31b7b592c4c69fd38147594..d8b1646f999ae3c73ebe322867d047646218150c 100644 (file)
@@ -852,38 +852,30 @@ SST_GetDelayTestData(SST_Stats inst, struct timespec *sample_time,
 /* This is used to save the register to a file, so that we can reload
    it after restarting the daemon */
 
-void
+int
 SST_SaveToFile(SST_Stats inst, FILE *out)
 {
   int m, i, j;
 
-  fprintf(out, "%d\n", inst->n_samples);
+  if (inst->n_samples < 1)
+    return 0;
+
+  if (fprintf(out, "%d %d\n", inst->n_samples, inst->asymmetry_run) < 0)
+    return 0;
 
   for(m = 0; m < inst->n_samples; m++) {
     i = get_runsbuf_index(inst, m);
     j = get_buf_index(inst, m);
 
-    fprintf(out,
-#ifdef HAVE_LONG_TIME_T
-            "%08"PRIx64" %08lx %.6e %.6e %.6e %.6e %.6e %.6e %.6e %d\n",
-            (uint64_t)inst->sample_times[i].tv_sec,
-#else
-            "%08lx %08lx %.6e %.6e %.6e %.6e %.6e %.6e %.6e %d\n",
-            (unsigned long)inst->sample_times[i].tv_sec,
-#endif
-            (unsigned long)inst->sample_times[i].tv_nsec / 1000,
-            inst->offsets[i],
-            inst->orig_offsets[j],
-            inst->peer_delays[i],
-            inst->peer_dispersions[j],
-            inst->root_delays[j],
-            inst->root_dispersions[j],
-            1.0, /* used to be inst->weights[i] */
-            0 /* used to be an array of strata */);
-
+    if (fprintf(out, "%s %.6e %.6e %.6e %.6e %.6e %.6e\n",
+                UTI_TimespecToString(&inst->sample_times[i]),
+                inst->offsets[i], inst->orig_offsets[j],
+                inst->peer_delays[i], inst->peer_dispersions[j],
+                inst->root_delays[j], inst->root_dispersions[j]) < 0)
+      return 0;
   }
 
-  fprintf(out, "%d\n", inst->asymmetry_run);
+  return 1;
 }
 
 /* ================================================== */
@@ -892,66 +884,32 @@ SST_SaveToFile(SST_Stats inst, FILE *out)
 int
 SST_LoadFromFile(SST_Stats inst, FILE *in)
 {
-#ifdef HAVE_LONG_TIME_T
-  uint64_t sec;
-#else
-  unsigned long sec;
-#endif
-  unsigned long usec;
-  int i;
-  char line[1024];
-  double weight;
-  int stratum;
+  int i, n_samples, arun;
+  double sample_time;
+  char line[256];
 
-  SST_ResetInstance(inst);
+  if (!fgets(line, sizeof (line), in) ||
+      sscanf(line, "%d %d", &n_samples, &arun) != 2 ||
+      n_samples < 1 || n_samples > MAX_SAMPLES)
+    return 0;
 
-  if (fgets(line, sizeof(line), in) &&
-      sscanf(line, "%d", &inst->n_samples) == 1 &&
-      inst->n_samples >= 0 && inst->n_samples <= MAX_SAMPLES) {
+  SST_ResetInstance(inst);
 
-    for (i=0; i<inst->n_samples; i++) {
-      if (!fgets(line, sizeof(line), in) ||
-          (sscanf(line,
-#ifdef HAVE_LONG_TIME_T
-                  "%"SCNx64"%lx%lf%lf%lf%lf%lf%lf%lf%d\n",
-#else
-                  "%lx%lx%lf%lf%lf%lf%lf%lf%lf%d\n",
-#endif
-                  &(sec), &(usec),
-                  &(inst->offsets[i]),
-                  &(inst->orig_offsets[i]),
-                  &(inst->peer_delays[i]),
-                  &(inst->peer_dispersions[i]),
-                  &(inst->root_delays[i]),
-                  &(inst->root_dispersions[i]),
-                  &weight, /* not used anymore */
-                  &stratum /* not used anymore */) != 10)) {
-
-        /* This is the branch taken if the read FAILED */
-
-        inst->n_samples = 0; /* Load abandoned if any sign of corruption */
-        return 0;
-      } else {
-
-        /* This is the branch taken if the read is SUCCESSFUL */
-        inst->sample_times[i].tv_sec = sec;
-        inst->sample_times[i].tv_nsec = 1000 * usec;
-        UTI_NormaliseTimespec(&inst->sample_times[i]);
-      }
-    }
+  for (i = 0; i < n_samples; i++) {
+    if (!fgets(line, sizeof (line), in) ||
+        sscanf(line, "%lf %lf %lf %lf %lf %lf %lf",
+               &sample_time, &inst->offsets[i], &inst->orig_offsets[i],
+               &inst->peer_delays[i], &inst->peer_dispersions[i],
+               &inst->root_delays[i], &inst->root_dispersions[i]) != 7)
+      return 0;
 
-    /* This field was not saved in older versions */
-    if (!fgets(line, sizeof(line), in) || sscanf(line, "%d\n", &inst->asymmetry_run) != 1)
-      inst->asymmetry_run = 0;
-  } else {
-    inst->n_samples = 0; /* Load abandoned if any sign of corruption */
-    return 0;
+    /* Some resolution is lost in the double format, but that's ok */
+    UTI_DoubleToTimespec(sample_time, &inst->sample_times[i]);
   }
 
-  if (!inst->n_samples)
-    return 1;
-
+  inst->n_samples = n_samples;
   inst->last_sample = inst->n_samples - 1;
+  inst->asymmetry_run = CLAMP(-MAX_ASYMMETRY_RUN, arun, MAX_ASYMMETRY_RUN);
 
   find_min_delay_sample(inst);
   SST_DoNewRegression(inst);
index a0f6af968cb229dd15bd715bdcbc6bb3402008c2..bb5fb714e952928849e6c2aef6b79f9d056d3349 100644 (file)
@@ -119,7 +119,7 @@ extern int SST_GetDelayTestData(SST_Stats inst, struct timespec *sample_time,
                                 double *last_sample_ago, double *predicted_offset,
                                 double *min_delay, double *skew, double *std_dev);
 
-extern void SST_SaveToFile(SST_Stats inst, FILE *out);
+extern int SST_SaveToFile(SST_Stats inst, FILE *out);
 
 extern int SST_LoadFromFile(SST_Stats inst, FILE *in);