Concurrent calls of "relayctl show sessions" could crash relayd. Fix

the show sessions handler by implementing it in an asynchronous way.

Closes PR 6509
ok pyr@

5 files changed, 63 insertions, 54 deletions

diff --git a/usr.sbin/relayctl/relayctl.c b/usr.sbin/relayctl/relayctl.c
index 1f83e8109ad..382f9551d84 100644
--- a/usr.sbin/relayctl/relayctl.c
+++ b/usr.sbin/relayctl/relayctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: relayctl.c,v 1.44 2011/05/19 08:56:49 reyk Exp $	*/
+/*	$OpenBSD: relayctl.c,v 1.45 2011/05/20 09:43:53 reyk Exp $	*/
 
 /*
  * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -447,7 +447,8 @@ show_session_msg(struct imsg *imsg)
 			fatal("show_session_msg: gettimeofday");
 		print_time(&tv_now, &con->se_tv_start, a, sizeof(a));
 		print_time(&tv_now, &con->se_tv_last, b, sizeof(b));
-		printf("\tage %s, idle %s, relay %u", a, b, con->se_relayid);
+		printf("\tage %s, idle %s, relay %u, pid %u",
+		    a, b, con->se_relayid, con->se_pid);
 		if (con->se_mark)
 			printf(", mark %u", con->se_mark);
 		printf("\n");
diff --git a/usr.sbin/relayd/control.c b/usr.sbin/relayd/control.c
index 7397c34cc01..cdb4f0b063d 100644
--- a/usr.sbin/relayd/control.c
+++ b/usr.sbin/relayd/control.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: control.c,v 1.39 2011/05/19 08:56:49 reyk Exp $	*/
+/*	$OpenBSD: control.c,v 1.40 2011/05/20 09:43:53 reyk Exp $	*/
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -235,6 +235,14 @@ control_dispatch_imsg(int fd, short event, void *arg)
 		if (n == 0)
 			break;
 
+		if (c->waiting) {
+			log_debug("%s: unexpected imsg %d",
+			    __func__, imsg.hdr.type);
+			imsg_free(&imsg);
+			control_close(fd);
+			return;
+		}
+
 		switch (imsg.hdr.type) {
 		case IMSG_CTL_SHOW_SUM:
 			show(c);
diff --git a/usr.sbin/relayd/pfe.c b/usr.sbin/relayd/pfe.c
index 4ed78c9f88d..a56d135d7c4 100644
--- a/usr.sbin/relayd/pfe.c
+++ b/usr.sbin/relayd/pfe.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfe.c,v 1.69 2011/05/19 08:56:49 reyk Exp $	*/
+/*	$OpenBSD: pfe.c,v 1.70 2011/05/20 09:43:53 reyk Exp $	*/
 
 /*
  * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -222,6 +222,9 @@ pfe_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg)
 	struct ctl_natlook	 cnl;
 	struct ctl_stats	 crs;
 	struct relay		*rlay;
+	struct ctl_conn		*c;
+	struct rsession		 con;
+	int			 cid;
 
 	switch (imsg->hdr.type) {
 	case IMSG_NATLOOK:
@@ -247,6 +250,35 @@ pfe_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg)
 		rlay->rl_stats[crs.proc].interval =
 		    env->sc_statinterval.tv_sec;
 		break;
+	case IMSG_CTL_SESSION:
+		IMSG_SIZE_CHECK(imsg, &con);
+		memcpy(&con, imsg->data, sizeof(con));
+		if ((c = control_connbyfd(con.se_cid)) == NULL) {
+			log_debug("%s: control connection %d not found",
+			    __func__, con.se_cid);
+			return (0);
+		}
+		imsg_compose_event(&c->iev,
+		    IMSG_CTL_SESSION, 0, 0, -1,
+		    &con, sizeof(con));
+		break;
+	case IMSG_CTL_END:
+		IMSG_SIZE_CHECK(imsg, &cid);
+		memcpy(&cid, imsg->data, sizeof(cid));
+		if ((c = control_connbyfd(cid)) == NULL) {
+			log_debug("%s: control connection %d not found",
+			    __func__, cid);
+			return (0);
+		}
+		if (c->waiting == 0) {
+			log_debug("%s: no pending control requests", __func__);
+			return (0);
+		} else if (--c->waiting == 0) {
+			/* Last ack for a previous request */
+			imsg_compose_event(&c->iev, IMSG_CTL_END,
+			    0, 0, -1, NULL, 0);
+		}
+		break;
 	default:
 		return (-1);
 	}
@@ -346,59 +378,18 @@ end:
 void
 show_sessions(struct ctl_conn *c)
 {
-	int			 n, proc, done;
-	struct imsg		 imsg;
-	struct imsgbuf		*ibuf;
-	struct rsession		 con;
+	int			 proc, cid;
 
 	for (proc = 0; proc < env->sc_prefork_relay; proc++) {
-		ibuf = proc_ibuf(env->sc_ps, PROC_RELAY, proc);
+		cid = c->iev.ibuf.fd;
 
 		/*
 		 * Request all the running sessions from the process
 		 */
 		proc_compose_imsg(env->sc_ps, PROC_RELAY, proc,
-		    IMSG_CTL_SESSION, -1, NULL, 0);
-		proc_flush_imsg(env->sc_ps, PROC_RELAY, proc);
-		
-		/*
-		 * Wait for the reply and forward the messages to the
-		 * control connection.
-		 */
-		done = 0;
-		while (!done) {
-			do {
-				if ((n = imsg_read(ibuf)) == -1)
-					fatalx("imsg_read error");
-			} while (n == -2); /* handle non-blocking I/O */
-			while (!done) {
-				if ((n = imsg_get(ibuf, &imsg)) == -1)
-					fatalx("imsg_get error");
-				if (n == 0)
-					break;
-
-				switch (imsg.hdr.type) {
-				case IMSG_CTL_SESSION:
-					IMSG_SIZE_CHECK(&imsg, &con);
-					memcpy(&con, imsg.data, sizeof(con));
-
-					imsg_compose_event(&c->iev,
-					    IMSG_CTL_SESSION, 0, 0, -1,
-					    &con, sizeof(con));
-					break;
-				case IMSG_CTL_END:
-					done = 1;
-					break;
-				default:
-					fatalx("wrong message for session");
-					break;
-				}
-				imsg_free(&imsg);
-			}
-		}
+		    IMSG_CTL_SESSION, -1, &cid, sizeof(cid));
+		c->waiting++;
 	}
-
-	imsg_compose_event(&c->iev, IMSG_CTL_END, 0, 0, -1, NULL, 0);
 }
 
 int
diff --git a/usr.sbin/relayd/relay.c b/usr.sbin/relayd/relay.c
index 00d50e93a79..26ce54f0ed3 100644
--- a/usr.sbin/relayd/relay.c
+++ b/usr.sbin/relayd/relay.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: relay.c,v 1.137 2011/05/19 08:56:49 reyk Exp $	*/
+/*	$OpenBSD: relay.c,v 1.138 2011/05/20 09:43:53 reyk Exp $	*/
 
 /*
  * Copyright (c) 2006, 2007, 2008 Reyk Floeter <reyk@openbsd.org>
@@ -1939,6 +1939,7 @@ relay_accept(int fd, short sig, void *arg)
 	con->se_relay = rlay;
 	con->se_id = ++relay_conid;
 	con->se_relayid = rlay->rl_conf.id;
+	con->se_pid = getpid();
 	con->se_hashkey = rlay->rl_dstkey;
 	con->se_in.tree = &proto->request_tree;
 	con->se_out.tree = &proto->response_tree;
@@ -2375,13 +2376,14 @@ int
 relay_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
 {
 	struct relay		*rlay;
-	struct rsession		*con;
+	struct rsession		*con, se;
 	struct ctl_natlook	 cnl;
 	struct timeval		 tv;
 	struct host		*host;
 	struct table		*table;
 	struct ctl_status	 st;
 	objid_t			 id;
+	int			 cid;
 
 	switch (imsg->hdr.type) {
 	case IMSG_HOST_DISABLE:
@@ -2468,16 +2470,20 @@ relay_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
 		evtimer_add(&con->se_ev, &tv);
 		break;
 	case IMSG_CTL_SESSION:
+		IMSG_SIZE_CHECK(imsg, &cid);
+		memcpy(&cid, imsg->data, sizeof(cid));
 		TAILQ_FOREACH(rlay, env->sc_relays, rl_entry) {
 			SPLAY_FOREACH(con, session_tree,
 			    &rlay->rl_sessions) {
+				memcpy(&se, con, sizeof(se));
+				se.se_cid = cid;
 				proc_compose_imsg(env->sc_ps, p->p_id, -1,
 				    IMSG_CTL_SESSION,
-				    -1, con, sizeof(*con));
+				    -1, &se, sizeof(se));
 			}
 		}
 		proc_compose_imsg(env->sc_ps, p->p_id, -1, IMSG_CTL_END,
-		    -1, NULL, 0);
+		    -1, &cid, sizeof(cid));
 		break;
 	default:
 		return (-1);
diff --git a/usr.sbin/relayd/relayd.h b/usr.sbin/relayd/relayd.h
index 8da1a96285d..5a649b83ae1 100644
--- a/usr.sbin/relayd/relayd.h
+++ b/usr.sbin/relayd/relayd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: relayd.h,v 1.147 2011/05/19 08:56:49 reyk Exp $	*/
+/*	$OpenBSD: relayd.h,v 1.148 2011/05/20 09:43:53 reyk Exp $	*/
 
 /*
  * Copyright (c) 2006, 2007 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -428,6 +428,8 @@ struct rsession {
 	struct ctl_natlook		*se_cnl;
 	int				 se_bnds;
 
+	int				 se_cid;
+	pid_t				 se_pid;
 	SPLAY_ENTRY(rsession)		 se_nodes;
 };
 SPLAY_HEAD(session_tree, rsession);
@@ -705,6 +707,7 @@ struct imsgev {
 struct ctl_conn {
 	TAILQ_ENTRY(ctl_conn)	 entry;
 	u_int8_t		 flags;
+	u_int			 waiting;
 #define CTL_CONN_NOTIFY		 0x01
 	struct imsgev	 	 iev;