From: Rafael Sadowski <rafael@sizeofvoid.org>
Subject: [3/7] relayd: convert control imsg forwarding to imsg_forward()
To: tech@openbsd.org
Date: Sun, 7 Jun 2026 09:01:24 +0200

This is a series of commits for an imsg APIv2 rework in relayd. These
were committed and tested individually. They can be reviewed in their
entirety here:

gothub: https://rsadowski.gothub.org/?action=summary&headref=imsg_v2&path=relayd.git
PR: https://codeberg.org/rsadowski/relayd/pulls/1/files

commit cdac3e43ca7856c92948a16de7bd0e4ec3e12f26
Author: Rafael Sadowski <rafael@sizeofvoid.org>
Date:   Sat Jun 6 16:22:49 2026 +0200

    relayd: convert control imsg forwarding to imsg_forward()
    
    Rework control_imsg_forward() to forward the message unaltered via
    imsg_forward() instead of rebuilding it with imsg_compose_event().
    
    read the type via imsg_get_type(), dropping the manual header-length
    Switch to use read the payload with imsg_get_data() and checks and the
    memcpy() that wrote the data back into the imsg before forwarding.

diff --git a/control.c b/control.c
index 6c29f2b..fe829ce 100644
--- a/control.c
+++ b/control.c
@@ -228,7 +228,7 @@ control_dispatch_imsg(int fd, short event, void *arg)
 	struct imsg		 imsg;
 	struct ctl_id		 id;
 	int			 n;
-	int			 verbose;
+	int			 verb;
 	struct relayd		*env = cs->cs_env;
 	struct privsep		*ps = env->sc_ps;
 
@@ -238,7 +238,11 @@ control_dispatch_imsg(int fd, short event, void *arg)
 	}
 
 	if (event & EV_READ) {
-		if (imsgbuf_read(&c->iev.ibuf) != 1) {
+		switch (imsgbuf_read(&c->iev.ibuf)) {
+		case -1:
+			fatal("%s: imsgbuf_read", __func__);
+			break;
+		case 0:
 			control_close(fd, cs);
 			return;
 		}
@@ -262,13 +266,13 @@ control_dispatch_imsg(int fd, short event, void *arg)
 
 		if (c->waiting) {
 			log_debug("%s: unexpected imsg %d",
-			    __func__, imsg.hdr.type);
+			    __func__, imsg_get_type(&imsg));
 			imsg_free(&imsg);
 			control_close(fd, cs);
 			return;
 		}
 
-		switch (imsg.hdr.type) {
+		switch (imsg_get_type(&imsg)) {
 		case IMSG_CTL_SHOW_SUM:
 			show(c);
 			break;
@@ -276,85 +280,79 @@ control_dispatch_imsg(int fd, short event, void *arg)
 			show_sessions(c);
 			break;
 		case IMSG_CTL_RDR_DISABLE:
-			if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(id))
-				fatalx("invalid imsg header len");
-			memcpy(&id, imsg.data, sizeof(id));
+			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
+				fatalx("%s: imsg_get_data", __func__);
+
 			if (disable_rdr(c, &id))
 				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
 				    0, ps->ps_instance + 1, -1, NULL, 0);
 			else {
-				memcpy(imsg.data, &id, sizeof(id));
-				control_imsg_forward(ps, &imsg);
+				control_imsg_forward(&imsg);
 				imsg_compose_event(&c->iev, IMSG_CTL_OK,
 				    0, ps->ps_instance + 1, -1, NULL, 0);
 			}
 			break;
 		case IMSG_CTL_RDR_ENABLE:
-			if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(id))
-				fatalx("invalid imsg header len");
-			memcpy(&id, imsg.data, sizeof(id));
+			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
+				fatalx("%s: imsg_get_data", __func__);
+
 			if (enable_rdr(c, &id))
 				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
 				    0, ps->ps_instance + 1, -1, NULL, 0);
 			else {
-				memcpy(imsg.data, &id, sizeof(id));
-				control_imsg_forward(ps, &imsg);
+				control_imsg_forward(&imsg);
 				imsg_compose_event(&c->iev, IMSG_CTL_OK,
 				    0, ps->ps_instance + 1, -1, NULL, 0);
 			}
 			break;
 		case IMSG_CTL_TABLE_DISABLE:
-			if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(id))
-				fatalx("invalid imsg header len");
-			memcpy(&id, imsg.data, sizeof(id));
+			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
+				fatalx("%s: imsg_get_data", __func__);
+
 			if (disable_table(c, &id))
 				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
 				    0, ps->ps_instance + 1, -1, NULL, 0);
 			else {
-				memcpy(imsg.data, &id, sizeof(id));
-				control_imsg_forward(ps, &imsg);
+				control_imsg_forward(&imsg);
 				imsg_compose_event(&c->iev, IMSG_CTL_OK,
 				    0, ps->ps_instance + 1, -1, NULL, 0);
 			}
 			break;
 		case IMSG_CTL_TABLE_ENABLE:
-			if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(id))
-				fatalx("invalid imsg header len");
-			memcpy(&id, imsg.data, sizeof(id));
+			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
+				fatalx("%s: imsg_get_data", __func__);
+
 			if (enable_table(c, &id))
 				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
 				    0, ps->ps_instance + 1, -1, NULL, 0);
 			else {
-				memcpy(imsg.data, &id, sizeof(id));
-				control_imsg_forward(ps, &imsg);
+				control_imsg_forward(&imsg);
 				imsg_compose_event(&c->iev, IMSG_CTL_OK,
 				    0, ps->ps_instance + 1, -1, NULL, 0);
 			}
 			break;
 		case IMSG_CTL_HOST_DISABLE:
-			if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(id))
-				fatalx("invalid imsg header len");
-			memcpy(&id, imsg.data, sizeof(id));
+			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
+				fatalx("%s: imsg_get_data", __func__);
+
 			if (disable_host(c, &id, NULL))
 				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
 				    0, ps->ps_instance + 1, -1, NULL, 0);
 			else {
-				memcpy(imsg.data, &id, sizeof(id));
-				control_imsg_forward(ps, &imsg);
+				control_imsg_forward(&imsg);
 				imsg_compose_event(&c->iev, IMSG_CTL_OK,
 				    0, ps->ps_instance + 1, -1, NULL, 0);
 			}
 			break;
 		case IMSG_CTL_HOST_ENABLE:
-			if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(id))
-				fatalx("invalid imsg header len");
-			memcpy(&id, imsg.data, sizeof(id));
+			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
+				fatalx("%s: imsg_get_data", __func__);
+
 			if (enable_host(c, &id, NULL))
 				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
 				    0, ps->ps_instance + 1, -1, NULL, 0);
 			else {
-				memcpy(imsg.data, &id, sizeof(id));
-				control_imsg_forward(ps, &imsg);
+				control_imsg_forward(&imsg);
 				imsg_compose_event(&c->iev, IMSG_CTL_OK,
 				    0, ps->ps_instance + 1, -1, NULL, 0);
 			}
@@ -381,21 +379,19 @@ control_dispatch_imsg(int fd, short event, void *arg)
 			c->flags |= CTL_CONN_NOTIFY;
 			break;
 		case IMSG_CTL_VERBOSE:
-			IMSG_SIZE_CHECK(&imsg, &verbose);
-
-			memcpy(&verbose, imsg.data, sizeof(verbose));
+			if (imsg_get_data(&imsg, &verb, sizeof(verb)) == -1)
+				fatalx("%s: imsg_get_data", __func__);
 
 			proc_forward_imsg(env->sc_ps, &imsg, PROC_PARENT);
 			proc_forward_imsg(env->sc_ps, &imsg, PROC_HCE);
 			proc_forward_imsg(env->sc_ps, &imsg, PROC_RELAY);
 
-			memcpy(imsg.data, &verbose, sizeof(verbose));
-			control_imsg_forward(ps, &imsg);
-			log_setverbose(verbose);
+			control_imsg_forward(&imsg);
+			log_setverbose(verb);
 			break;
 		default:
 			log_debug("%s: error handling imsg %d",
-			    __func__, imsg.hdr.type);
+			    __func__, imsg_get_type(&imsg));
 			break;
 		}
 		imsg_free(&imsg);
@@ -405,13 +401,15 @@ control_dispatch_imsg(int fd, short event, void *arg)
 }
 
 void
-control_imsg_forward(struct privsep *ps, struct imsg *imsg)
+control_imsg_forward(struct imsg *imsg)
 {
-	struct ctl_conn *c;
+        struct ctl_conn *c;
 
-	TAILQ_FOREACH(c, &ctl_conns, entry)
-		if (c->flags & CTL_CONN_NOTIFY)
-			imsg_compose_event(&c->iev, imsg->hdr.type,
-			    0, ps->ps_instance + 1, -1, imsg->data,
-			    imsg->hdr.len - IMSG_HEADER_SIZE);
+	TAILQ_FOREACH(c, &ctl_conns, entry) {
+		if (c->flags & CTL_CONN_NOTIFY) {
+			if (imsg_forward(&c->iev.ibuf, imsg) == -1)
+				fatal("%s: imsg_forward", __func__);
+			imsg_event_add(&c->iev);
+		}
+	}
 }
diff --git a/pfe.c b/pfe.c
index 0f865b8..0856ae9 100644
--- a/pfe.c
+++ b/pfe.c
@@ -124,7 +124,7 @@ pfe_dispatch_hce(int fd, struct privsep_proc *p, struct imsg *imsg)
 	struct table		*table;
 	struct ctl_status	 st;
 
-	control_imsg_forward(p->p_ps, imsg);
+	control_imsg_forward(imsg);
 
 	switch (imsg_get_type(imsg)) {
 	case IMSG_HOST_STATUS:
@@ -737,7 +737,7 @@ pfe_sync(void)
 			imsg.hdr.len = sizeof(id) + IMSG_HEADER_SIZE;
 			imsg.data = &id;
 			sync_table(env, rdr, active);
-			control_imsg_forward(env->sc_ps, &imsg);
+			control_imsg_forward(&imsg);
 		}
 
 		if (rdr->conf.flags & F_DOWN) {
@@ -750,7 +750,7 @@ pfe_sync(void)
 				imsg.hdr.len = sizeof(id) + IMSG_HEADER_SIZE;
 				imsg.data = &id;
 				sync_ruleset(env, rdr, 0);
-				control_imsg_forward(env->sc_ps, &imsg);
+				control_imsg_forward(&imsg);
 			}
 		} else if (!(rdr->conf.flags & F_ACTIVE_RULESET)) {
 			log_debug("%s: enabling ruleset", __func__);
@@ -760,7 +760,7 @@ pfe_sync(void)
 			imsg.hdr.len = sizeof(id) + IMSG_HEADER_SIZE;
 			imsg.data = &id;
 			sync_ruleset(env, rdr, 1);
-			control_imsg_forward(env->sc_ps, &imsg);
+			control_imsg_forward(&imsg);
 		}
 	}
 
diff --git a/relayd.h b/relayd.h
index 16fe2bf..373255e 100644
--- a/relayd.h
+++ b/relayd.h
@@ -1154,7 +1154,7 @@ int	 control_init(struct privsep *, struct control_sock *);
 int	 control_listen(struct control_sock *);
 void	 control_cleanup(struct control_sock *);
 void	 control_dispatch_imsg(int, short, void *);
-void	 control_imsg_forward(struct privsep *ps, struct imsg *);
+void	 control_imsg_forward(struct imsg *);
 struct ctl_conn	*
 	 control_connbyfd(int);