From 4c6157fa123ed17e3cb781c19aaaea0fdb7d4b05 Mon Sep 17 00:00:00 2001
From: bluhm <bluhm@openbsd.org>
Date: Wed, 21 Apr 2021 09:38:11 +0000
Subject: [PATCH] Improve ntpd offset handling.  Call the index of the offset
 loops "shift" consistently.  Merge the two offset loops in client_update()
 into one.  Use a simple assignment for the best value instead of memcpy(). 
 Use the same mechanism to loop over the offset array everywhere to avoid an
 invalid best value. tested by weerd@; OK claudio@

---
 usr.sbin/ntpd/client.c  | 29 ++++++++++++-----------------
 usr.sbin/ntpd/control.c | 27 ++++++++++++++-------------
 2 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/usr.sbin/ntpd/client.c b/usr.sbin/ntpd/client.c
index db10c8abb78..4ce70f59f48 100644
--- a/usr.sbin/ntpd/client.c
+++ b/usr.sbin/ntpd/client.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: client.c,v 1.115 2021/03/18 11:06:41 bluhm Exp $ */
+/*	$OpenBSD: client.c,v 1.116 2021/04/21 09:38:11 bluhm Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -473,7 +473,7 @@ client_dispatch(struct ntp_peer *p, u_int8_t settime, u_int8_t automatic)
 int
 client_update(struct ntp_peer *p)
 {
-	int	i, best = 0, good = 0;
+	int	shift, best = -1, good = 0;
 
 	/*
 	 * clock filter
@@ -482,27 +482,22 @@ client_update(struct ntp_peer *p)
 	 * invalidate it and all older ones
 	 */
 
-	for (i = 0; good == 0 && i < OFFSET_ARRAY_SIZE; i++)
-		if (p->reply[i].good) {
+	for (shift = 0; shift < OFFSET_ARRAY_SIZE; shift++)
+		if (p->reply[shift].good) {
 			good++;
-			best = i;
+			if (best == -1 ||
+			    p->reply[shift].delay < p->reply[best].delay)
+				best = shift;
 		}
 
-	for (; i < OFFSET_ARRAY_SIZE; i++)
-		if (p->reply[i].good) {
-			good++;
-			if (p->reply[i].delay < p->reply[best].delay)
-				best = i;
-		}
-
-	if (good < 8)
+	if (best == -1 || good < 8)
 		return (-1);
 
-	memcpy(&p->update, &p->reply[best], sizeof(p->update));
+	p->update = p->reply[best];
 	if (priv_adjtime() == 0) {
-		for (i = 0; i < OFFSET_ARRAY_SIZE; i++)
-			if (p->reply[i].rcvd <= p->reply[best].rcvd)
-				p->reply[i].good = 0;
+		for (shift = 0; shift < OFFSET_ARRAY_SIZE; shift++)
+			if (p->reply[shift].rcvd <= p->reply[best].rcvd)
+				p->reply[shift].good = 0;
 	}
 	return (0);
 }
diff --git a/usr.sbin/ntpd/control.c b/usr.sbin/ntpd/control.c
index 751e9cb4c66..b86e615e86c 100644
--- a/usr.sbin/ntpd/control.c
+++ b/usr.sbin/ntpd/control.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: control.c,v 1.18 2020/02/12 19:14:56 otto Exp $ */
+/*	$OpenBSD: control.c,v 1.19 2021/04/21 09:38:11 bluhm Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -341,7 +341,7 @@ build_show_peer(struct ctl_show_peer *cp, struct ntp_peer *p)
 	const char	*a = "not resolved";
 	const char	*pool = "", *addr_head_name = "";
 	const char	*auth = "";
-	u_int8_t	 shift, best, validdelaycnt, jittercnt;
+	int		 shift, best = -1, validdelaycnt = 0, jittercnt = 0;
 	time_t		 now;
 
 	now = getmonotime();
@@ -360,14 +360,14 @@ build_show_peer(struct ctl_show_peer *cp, struct ntp_peer *p)
 	snprintf(cp->peer_desc, sizeof(cp->peer_desc),
 	    "%s %s%s%s", a, pool, addr_head_name, auth);
 
-	validdelaycnt = best = 0;
 	cp->offset = cp->delay = 0.0;
 	for (shift = 0; shift < OFFSET_ARRAY_SIZE; shift++) {
 		if (p->reply[shift].delay > 0.0) {
 			cp->offset += p->reply[shift].offset;
 			cp->delay += p->reply[shift].delay;
 
-			if (p->reply[shift].delay < p->reply[best].delay)
+			if (best == -1 ||
+			    p->reply[shift].delay < p->reply[best].delay)
 				best = shift;
 
 			validdelaycnt++;
@@ -379,18 +379,19 @@ build_show_peer(struct ctl_show_peer *cp, struct ntp_peer *p)
 		cp->delay /= validdelaycnt;
 	}
 
-	jittercnt = 0;
 	cp->jitter = 0.0;
-	for (shift = 0; shift < OFFSET_ARRAY_SIZE; shift++) {
-		if (p->reply[shift].delay > 0.0 && shift != best) {
-			cp->jitter += square(p->reply[shift].delay -
-			    p->reply[best].delay);
-			jittercnt++;
+	if (best != -1) {
+		for (shift = 0; shift < OFFSET_ARRAY_SIZE; shift++) {
+			if (p->reply[shift].delay > 0.0 && shift != best) {
+				cp->jitter += square(p->reply[shift].delay -
+				    p->reply[best].delay);
+				jittercnt++;
+			}
 		}
+		if (jittercnt > 1)
+			cp->jitter /= jittercnt;
+		cp->jitter = sqrt(cp->jitter);
 	}
-	if (jittercnt > 1)
-		cp->jitter /= jittercnt;
-	cp->jitter = sqrt(cp->jitter);
 
 	if (p->shift == 0)
 		shift = OFFSET_ARRAY_SIZE - 1;
-- 
2.20.1