1) Re-resolve and re-get constraints once the clock is synced. Constraints

are relative to monotime; so they shift when time is being adjusted.
2) Fix a race between SIGCHLD delivery and reading the result imsg.
3) Some cleanup: use a number to distinguish pools internally

4 files changed, 43 insertions, 18 deletions

diff --git a/usr.sbin/ntpd/constraint.c b/usr.sbin/ntpd/constraint.c
index f478ec789e9..656142aec00 100644
--- a/usr.sbin/ntpd/constraint.c
+++ b/usr.sbin/ntpd/constraint.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: constraint.c,v 1.47 2019/06/28 13:32:49 deraadt Exp $	*/
+/*	$OpenBSD: constraint.c,v 1.48 2019/07/16 14:15:40 otto Exp $	*/
 
 /*
  * Copyright (c) 2015 Reyk Floeter <reyk@openbsd.org>
@@ -49,6 +49,7 @@
 #define	X509_DATE	"%Y-%m-%d %T UTC"
 
 int	 constraint_addr_init(struct constraint *);
+void	 constraint_addr_head_clear(struct constraint *);
 struct constraint *
 	 constraint_byid(u_int32_t);
 struct constraint *
@@ -142,6 +143,14 @@ constraint_addr_init(struct constraint *cstr)
 	return (1);
 }
 
+void
+constraint_addr_head_clear(struct constraint *cstr)
+{
+	host_dns_free(cstr->addr_head.a);
+	cstr->addr_head.a = NULL;
+	cstr->addr = NULL;
+}
+
 int
 constraint_query(struct constraint *cstr)
 {
@@ -614,7 +623,10 @@ priv_constraint_dispatch(struct pollfd *pfd)
 		return (0);
 
 	if (((n = imsg_read(&cstr->ibuf)) == -1 && errno != EAGAIN) || n == 0) {
-		priv_constraint_close(pfd->fd, 1);
+		/* there's a race between SIGCHLD delivery and reading imsg
+		   but if we've seen the reply, we're good */
+		priv_constraint_close(pfd->fd, cstr->state !=
+		    STATE_REPLY_RECEIVED);
 		return (1);
 	}
 
@@ -631,6 +643,9 @@ priv_constraint_dispatch(struct pollfd *pfd)
 			 if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(tv))
 				fatalx("invalid IMSG_CONSTRAINT received");
 
+			/* state is maintained by child, but we want to
+			   remember we've seen the result */
+			cstr->state = STATE_REPLY_RECEIVED;
 			/* forward imsg to ntp child, don't parse it here */
 			imsg_compose(ibuf, imsg.hdr.type,
 			    cstr->id, 0, -1, imsg.data, sizeof(tv));
@@ -716,7 +731,7 @@ constraint_msg_dns(u_int32_t id, u_int8_t *data, size_t len)
 	struct ntp_addr		*h;
 
 	if ((cstr = constraint_byid(id)) == NULL) {
-		log_warnx("IMSG_CONSTRAINT_DNS with invalid constraint id");
+		log_debug("IMSG_CONSTRAINT_DNS with invalid constraint id");
 		return;
 	}
 	if (cstr->addr != NULL) {
@@ -732,6 +747,16 @@ constraint_msg_dns(u_int32_t id, u_int8_t *data, size_t len)
 	if (len % (sizeof(struct sockaddr_storage) + sizeof(int)) != 0)
 		fatalx("IMSG_CONSTRAINT_DNS len");
 
+	if (cstr->addr_head.pool) {
+		struct constraint *n, *tmp;
+		TAILQ_FOREACH_SAFE(n, &conf->constraints, entry, tmp) {
+			if (cstr->id == n->id)
+				continue;
+			if (cstr->addr_head.pool == n->addr_head.pool)
+				constraint_remove(n);
+		}
+	}
+
 	p = data;
 	do {
 		if ((h = calloc(1, sizeof(*h))) == NULL)
@@ -826,6 +851,8 @@ constraint_reset(void)
 		if (cstr->state == STATE_QUERY_SENT)
 			continue;
 		constraint_close(cstr->id);
+		constraint_addr_head_clear(cstr);
+		constraint_init(cstr);
 	}
 	conf->constraint_errors = 0;
 }
@@ -847,7 +874,6 @@ constraint_check(double val)
 	diff = fabs(val - gettime_from_timeval(&tv));
 
 	if (diff > CONSTRAINT_MARGIN) {
-		/* XXX get new constraint if too many errors happened */
 		if (conf->constraint_errors++ >
 		    (CONSTRAINT_ERROR_MARGIN * peer_cnt)) {
 			constraint_reset();
diff --git a/usr.sbin/ntpd/ntp.c b/usr.sbin/ntpd/ntp.c
index 9fa68a4c6e6..8127498802d 100644
--- a/usr.sbin/ntpd/ntp.c
+++ b/usr.sbin/ntpd/ntp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ntp.c,v 1.158 2019/07/07 07:14:57 otto Exp $ */
+/*	$OpenBSD: ntp.c,v 1.159 2019/07/16 14:15:40 otto Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -453,6 +453,7 @@ ntp_dispatch_imsg(void)
 				log_info("clock is now synced");
 				conf->status.synced = 1;
 				priv_dns(IMSG_SYNCED, NULL, 0);
+				constraint_reset();
 			} else if (n == 0 && conf->status.synced) {
 				log_info("clock is now unsynced");
 				conf->status.synced = 0;
@@ -539,8 +540,8 @@ ntp_dispatch_imsg_dns(void)
 
 				TAILQ_FOREACH_SAFE(npeer, &conf->ntp_peers,
 				    entry, tmp) {
-					if (strcmp(npeer->addr_head.name,
-					    peer->addr_head.name) != 0)
+					if (npeer->addr_head.pool !=
+					    peer->addr_head.pool)
 						continue;
 					peercount++;
 					if (npeer->id == peer->id)
@@ -596,7 +597,8 @@ ntp_dispatch_imsg_dns(void)
 					npeer->addr_head.a = h;
 					npeer->addr_head.name =
 					    peer->addr_head.name;
-					npeer->addr_head.pool = 1;
+					npeer->addr_head.pool =
+					    peer->addr_head.pool;
 					client_peer_init(npeer);
 					npeer->state = STATE_DNS_DONE;
 					peer_add(npeer);
@@ -653,12 +655,7 @@ peer_remove(struct ntp_peer *p)
 void
 peer_addr_head_clear(struct ntp_peer *p)
 {
-	struct ntp_addr *a = p->addr_head.a;
-	while (a) {
-		struct ntp_addr *next = a->next;
-		free(a);
-		a = next;
-	}
+	host_dns_free(p->addr_head.a);
 	p->addr_head.a = NULL;
 	p->addr = NULL;
 }
diff --git a/usr.sbin/ntpd/ntpd.h b/usr.sbin/ntpd/ntpd.h
index bef25b886fc..d6d8d80be5c 100644
--- a/usr.sbin/ntpd/ntpd.h
+++ b/usr.sbin/ntpd/ntpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ntpd.h,v 1.145 2019/06/27 15:18:42 otto Exp $ */
+/*	$OpenBSD: ntpd.h,v 1.146 2019/07/16 14:15:40 otto Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -371,6 +371,7 @@ void	set_next(struct ntp_peer *, time_t);
 void	 constraint_add(struct constraint *);
 void	 constraint_remove(struct constraint *);
 void	 constraint_purge(void);
+void	 constraint_reset(void);
 int	 constraint_init(struct constraint *);
 int	 constraint_query(struct constraint *);
 int	 constraint_check(double);
diff --git a/usr.sbin/ntpd/parse.y b/usr.sbin/ntpd/parse.y
index c5b5cbf8369..a58da2f2ee7 100644
--- a/usr.sbin/ntpd/parse.y
+++ b/usr.sbin/ntpd/parse.y
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.y,v 1.72 2019/06/12 05:04:45 otto Exp $ */
+/*	$OpenBSD: parse.y,v 1.73 2019/07/16 14:15:40 otto Exp $ */
 
 /*
  * Copyright (c) 2002, 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -60,6 +60,7 @@ int		 findeol(void);
 struct ntpd_conf		*conf;
 struct sockaddr_in		 query_addr4;
 struct sockaddr_in6		 query_addr6;
+int				 poolseqnum;
 
 struct opts {
 	int		weight;
@@ -183,7 +184,7 @@ main		: LISTEN ON address listen_opts	{
 				p->query_addr6 = query_addr6;
 				p->addr = h;
 				p->addr_head.a = h;
-				p->addr_head.pool = 1;
+				p->addr_head.pool = ++poolseqnum;
 				p->addr_head.name = strdup($2->name);
 				if (p->addr_head.name == NULL)
 					fatal(NULL);
@@ -256,7 +257,7 @@ main		: LISTEN ON address listen_opts	{
 				p = new_constraint();
 				p->addr = h;
 				p->addr_head.a = h;
-				p->addr_head.pool = 1;
+				p->addr_head.pool = ++poolseqnum;
 				p->addr_head.name = strdup($3->name);
 				p->addr_head.path = strdup($3->path);
 				if (p->addr_head.name == NULL ||