From: Miroslav Lichvar <mlichvar@redhat.com>
Date: Wed, 19 Nov 2025 13:12:09 +0000 (+0100)
Subject: cmdmon: refactor handling of sources report
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0fec9e5cfae390fd884199849d5d88ebad436c64;p=thirdparty%2Fchrony.git

cmdmon: refactor handling of sources report

The NSR_ReportSource() and RCL_ReportSource() functions assume that the
provided report already has some data prefilled by SRC_ReportSource()
and it's assumed these functions cannot fail.

Change them to accept the required data (refid and IP address) as
a parameter, remove unneeded parameters, and return an error status
(if the refid/address doesn't exist) to be handled in cmdmon
handle_source_data(). Also, catch unexpected values of the source state
and mode to make chronyc report an error instead of incorrect data.
---

diff --git a/cmdmon.c b/cmdmon.c
index a21c3ada..c477b6df 100644
--- a/cmdmon.c
+++ b/cmdmon.c
@@ -504,15 +504,18 @@ handle_source_data(CMD_Request *rx_message, CMD_Reply *tx_message)
   if (SRC_ReportSource(ntohl(rx_message->data.source_data.index), &report, &now_corr)) {
     switch (SRC_GetType(ntohl(rx_message->data.source_data.index))) {
       case SRC_NTP:
-        NSR_ReportSource(&report, &now_corr);
+        if (!NSR_ReportSource(&report.ip_addr, &report))
+          return;
         break;
       case SRC_REFCLOCK:
-        RCL_ReportSource(&report, &now_corr);
+        if (report.ip_addr.family != IPADDR_INET4 ||
+            !RCL_ReportSource(report.ip_addr.addr.in4, &report))
+          return;
         break;
+      default:
+        return;
     }
     
-    tx_message->reply  = htons(RPY_SOURCE_DATA);
-    
     UTI_IPHostToNetwork(&report.ip_addr, &tx_message->data.source_data.ip_addr);
     tx_message->data.source_data.stratum = htons(report.stratum);
     tx_message->data.source_data.poll    = htons(report.poll);
@@ -535,6 +538,8 @@ handle_source_data(CMD_Request *rx_message, CMD_Reply *tx_message)
       case RPT_SELECTED:
         tx_message->data.source_data.state   = htons(RPY_SD_ST_SELECTED);
         break;
+      default:
+        return;
     }
     switch (report.mode) {
       case RPT_NTP_CLIENT:
@@ -546,6 +551,8 @@ handle_source_data(CMD_Request *rx_message, CMD_Reply *tx_message)
       case RPT_LOCAL_REFERENCE:
         tx_message->data.source_data.mode    = htons(RPY_SD_MD_REF);
         break;
+      default:
+        return;
     }
     tx_message->data.source_data.flags = htons(0);
     tx_message->data.source_data.reachability = htons(report.reachability);
@@ -553,6 +560,8 @@ handle_source_data(CMD_Request *rx_message, CMD_Reply *tx_message)
     tx_message->data.source_data.orig_latest_meas = UTI_FloatHostToNetwork(report.orig_latest_meas);
     tx_message->data.source_data.latest_meas = UTI_FloatHostToNetwork(report.latest_meas);
     tx_message->data.source_data.latest_meas_err = UTI_FloatHostToNetwork(report.latest_meas_err);
+
+    tx_message->reply = htons(RPY_SOURCE_DATA);
   } else {
     tx_message->status = htons(STT_NOSUCHSOURCE);
   }
diff --git a/ntp_core.c b/ntp_core.c
index 7395a638..5df416d7 100644
--- a/ntp_core.c
+++ b/ntp_core.c
@@ -3098,7 +3098,7 @@ NCR_InitiateSampleBurst(NCR_Instance inst, int n_good_samples, int n_total_sampl
 /* ================================================== */
 
 void
-NCR_ReportSource(NCR_Instance inst, RPT_SourceReport *report, struct timespec *now)
+NCR_ReportSource(NCR_Instance inst, RPT_SourceReport *report)
 {
   report->poll = get_transmit_poll(inst);
 
diff --git a/ntp_core.h b/ntp_core.h
index 67971ead..44468ec7 100644
--- a/ntp_core.h
+++ b/ntp_core.h
@@ -119,7 +119,7 @@ extern void NCR_ModifyPolltarget(NCR_Instance inst, int new_poll_target);
 
 extern void NCR_InitiateSampleBurst(NCR_Instance inst, int n_good_samples, int n_total_samples);
 
-extern void NCR_ReportSource(NCR_Instance inst, RPT_SourceReport *report, struct timespec *now);
+extern void NCR_ReportSource(NCR_Instance inst, RPT_SourceReport *report);
 extern void NCR_GetAuthReport(NCR_Instance inst, RPT_AuthReport *report);
 extern void NCR_GetNTPReport(NCR_Instance inst, RPT_NTPReport *report);
 
diff --git a/ntp_sources.c b/ntp_sources.c
index 7bf08894..aa53c425 100644
--- a/ntp_sources.c
+++ b/ntp_sources.c
@@ -1523,20 +1523,17 @@ NSR_InitiateSampleBurst(int n_good_samples, int n_total_samples,
 }
 
 /* ================================================== */
-/* The ip address is assumed to be completed on input, that is how we
-   identify the source record. */
 
-void
-NSR_ReportSource(RPT_SourceReport *report, struct timespec *now)
+int
+NSR_ReportSource(IPAddr *ip_addr, RPT_SourceReport *report)
 {
   int slot;
 
-  if (find_slot(&report->ip_addr, &slot)) {
-    NCR_ReportSource(get_record(slot)->data, report, now);
-  } else {
-    report->poll = 0;
-    report->latest_meas_ago = 0;
-  }
+  if (!find_slot(ip_addr, &slot))
+    return 0;
+
+  NCR_ReportSource(get_record(slot)->data, report);
+  return 1;
 }
 
 /* ================================================== */
diff --git a/ntp_sources.h b/ntp_sources.h
index 89a322e9..d16a2a1c 100644
--- a/ntp_sources.h
+++ b/ntp_sources.h
@@ -146,7 +146,7 @@ extern int NSR_ModifyPolltarget(IPAddr *address, int new_poll_target);
 
 extern int NSR_InitiateSampleBurst(int n_good_samples, int n_total_samples, IPAddr *mask, IPAddr *address);
 
-extern void NSR_ReportSource(RPT_SourceReport *report, struct timespec *now);
+extern int NSR_ReportSource(IPAddr *ip_addr, RPT_SourceReport *report);
 
 extern int NSR_GetAuthReport(IPAddr *address, RPT_AuthReport *report);
 
diff --git a/refclock.c b/refclock.c
index 98b43a56..d73e3c01 100644
--- a/refclock.c
+++ b/refclock.c
@@ -294,23 +294,21 @@ RCL_StartRefclocks(void)
   }
 }
 
-void
-RCL_ReportSource(RPT_SourceReport *report, struct timespec *now)
+int
+RCL_ReportSource(uint32_t ref_id, RPT_SourceReport *report)
 {
   unsigned int i;
-  uint32_t ref_id;
-
-  assert(report->ip_addr.family == IPADDR_INET4);
-  ref_id = report->ip_addr.addr.in4;
 
   for (i = 0; i < ARR_GetSize(refclocks); i++) {
     RCL_Instance inst = get_refclock(i);
     if (inst->ref_id == ref_id) {
       report->poll = inst->poll;
       report->mode = RPT_LOCAL_REFERENCE;
-      break;
+      return 1;
     }
   }
+
+  return 0;
 }
 
 int
diff --git a/refclock.h b/refclock.h
index b1816b9c..37d1ef38 100644
--- a/refclock.h
+++ b/refclock.h
@@ -68,7 +68,7 @@ extern void RCL_Initialise(void);
 extern void RCL_Finalise(void);
 extern int RCL_AddRefclock(RefclockParameters *params);
 extern void RCL_StartRefclocks(void);
-extern void RCL_ReportSource(RPT_SourceReport *report, struct timespec *now);
+extern int RCL_ReportSource(uint32_t ref_id, RPT_SourceReport *report);
 extern int RCL_ModifyOffset(uint32_t ref_id, double offset);
 
 /* functions used by drivers */
diff --git a/stubs.c b/stubs.c
index f7c85abf..afa925e2 100644
--- a/stubs.c
+++ b/stubs.c
@@ -106,10 +106,10 @@ RCL_StartRefclocks(void)
 {
 }
 
-void
-RCL_ReportSource(RPT_SourceReport *report, struct timespec *now)
+int
+RCL_ReportSource(uint32_t ref_id, RPT_SourceReport *report)
 {
-  memset(report, 0, sizeof (*report));
+  return 0;
 }
 
 int