Don't attempt to kill() the constraint in the wrong process. The

process management of the contraint processes has been moved from ntp
to the parent, for better privsep and pledge, but the ntp process
still attempted to kill the constraints on timeout directly.  Fix this
regression by introducing a new imsg from ntp to the parent and the
related logic to kill a constraint at the right place.

Reported & tested by bcook@
Ok bcook@

3 files changed, 38 insertions, 8 deletions

diff --git a/usr.sbin/ntpd/constraint.c b/usr.sbin/ntpd/constraint.c
index 91d6ceb5cd7..169007e4bde 100644
--- a/usr.sbin/ntpd/constraint.c
+++ b/usr.sbin/ntpd/constraint.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: constraint.c,v 1.24 2015/12/19 17:55:29 reyk Exp $	*/
+/*	$OpenBSD: constraint.c,v 1.25 2016/01/27 21:48:34 reyk Exp $	*/
 
 /*
  * Copyright (c) 2015 Reyk Floeter <reyk@openbsd.org>
@@ -163,7 +163,10 @@ constraint_query(struct constraint *cstr)
 		}
 
 		/* Timeout, just kill the process to reset it. */
-		kill(cstr->pid, SIGTERM);
+		imsg_compose(ibuf_main, IMSG_CONSTRAINT_KILL,
+		    cstr->id, 0, -1, NULL, 0);
+
+		cstr->state = STATE_TIMEOUT;
 		return (-1);
 	case STATE_INVALID:
 		if (cstr->last + CONSTRAINT_SCAN_INTERVAL > now) {
@@ -380,6 +383,7 @@ priv_constraint_check_child(pid_t pid, int status)
 {
 	struct constraint	*cstr;
 	int			 fail, sig;
+	char			*signame;
 
 	fail = sig = 0;
 	if (WIFSIGNALED(status)) {
@@ -391,15 +395,35 @@ priv_constraint_check_child(pid_t pid, int status)
 		fatalx("unexpected cause of SIGCHLD");
 
 	if ((cstr = constraint_bypid(pid)) != NULL) {
-		if (sig)
-			fatalx("constraint %s, signal %d",
-			    log_sockaddr((struct sockaddr *)
-			    &cstr->addr->ss), sig);
+		if (sig) {
+			if (sig != SIGTERM) {
+				signame = strsignal(sig) ?
+				    strsignal(sig) : "unknown";
+				log_warnx("constraint %s; "
+				    "terminated with signal %d (%s)",
+				    log_sockaddr((struct sockaddr *)
+				    &cstr->addr->ss), sig, signame);
+			}
+			fail = 1;
+		}
 
 		priv_constraint_close(cstr->fd, fail);
 	}
 }
 
+void
+priv_constraint_kill(u_int32_t id)
+{
+	struct constraint	*cstr;
+
+	if ((cstr = constraint_byid(id)) == NULL) {
+		log_warnx("IMSG_CONSTRAINT_KILL for invalid id %d", id);
+		return;
+	}
+
+	kill(cstr->pid, SIGTERM);
+}
+
 struct constraint *
 constraint_byid(u_int32_t id)
 {
diff --git a/usr.sbin/ntpd/ntpd.c b/usr.sbin/ntpd/ntpd.c
index 9a54c524d57..fe79f6d9bc6 100644
--- a/usr.sbin/ntpd/ntpd.c
+++ b/usr.sbin/ntpd/ntpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ntpd.c,v 1.104 2016/01/27 21:36:25 bcook Exp $ */
+/*	$OpenBSD: ntpd.c,v 1.105 2016/01/27 21:48:34 reyk Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -413,6 +413,9 @@ dispatch_imsg(struct ntpd_conf *lconf, const char *pw_dir,
 			    imsg.data, imsg.hdr.len - IMSG_HEADER_SIZE,
 			    pw_dir, pw_uid, pw_gid);
 			break;
+		case IMSG_CONSTRAINT_KILL:
+			priv_constraint_kill(imsg.hdr.peerid);
+			break;
 		default:
 			break;
 		}
diff --git a/usr.sbin/ntpd/ntpd.h b/usr.sbin/ntpd/ntpd.h
index 5e9099e023d..51b6751636d 100644
--- a/usr.sbin/ntpd/ntpd.h
+++ b/usr.sbin/ntpd/ntpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ntpd.h,v 1.128 2016/01/27 21:36:25 bcook Exp $ */
+/*	$OpenBSD: ntpd.h,v 1.129 2016/01/27 21:48:34 reyk Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -89,6 +89,7 @@ enum client_state {
 	STATE_DNS_DONE,
 	STATE_QUERY_SENT,
 	STATE_REPLY_RECEIVED,
+	STATE_TIMEOUT,
 	STATE_INVALID
 };
 
@@ -286,6 +287,7 @@ enum imsg_type {
 	IMSG_CONSTRAINT_QUERY,
 	IMSG_CONSTRAINT_RESULT,
 	IMSG_CONSTRAINT_CLOSE,
+	IMSG_CONSTRAINT_KILL,
 	IMSG_CTL_SHOW_STATUS,
 	IMSG_CTL_SHOW_PEERS,
 	IMSG_CTL_SHOW_PEERS_END,
@@ -357,6 +359,7 @@ void	 constraint_msg_result(u_int32_t, u_int8_t *, size_t);
 void	 constraint_msg_close(u_int32_t, u_int8_t *, size_t);
 void	 priv_constraint_msg(u_int32_t, u_int8_t *, size_t,
 	    const char *, uid_t, gid_t);
+void	 priv_constraint_kill(u_int32_t);
 int	 priv_constraint_dispatch(struct pollfd *);
 void	 priv_constraint_check_child(pid_t, int);
 char	*get_string(u_int8_t *, size_t);