Index | Thread | Search

From:
Florian Obser <florian@openbsd.org>
Subject:
slaacd(8): do not peek inside of struct imsg
To:
tech <tech@openbsd.org>
Date:
Fri, 23 Aug 2024 17:36:59 +0200

Download raw body.

Thread
Two commits on top of each other, first introduce i2s to simplify imsg
type logging (inspired by smtpd(8)).
The second diff switches to imsg_get_data to fetch data out of the imsg
and validate sizes.

I always wanted to simplify the imsg logging but I was worried about the
churn. Now is a good time since I have to touch everything anyway.

OK?

commit 0d133cb516f2323c3ee0e025acc133be7f9ae8bc
Author: Florian Obser <florian@narrans.de>
Date:   Fri Aug 23 17:07:59 2024 +0200

    Helper function for logging imsg type names.

diff --git slaacd.c slaacd.c
index 05af06e68de..c6c84d25e2c 100644
--- slaacd.c
+++ slaacd.c
@@ -899,3 +899,54 @@ open_icmp6sock(int rdomain)
 	main_imsg_compose_frontend(IMSG_ICMP6SOCK, icmp6sock, &rdomain,
 	    sizeof(rdomain));
 }
+
+#ifndef	SMALL
+
+#define	I2S(x) case x: return #x
+
+const char*
+i2s(uint32_t type)
+{
+	static char	unknown[sizeof("IMSG_4294967295")];
+
+	switch (type) {
+	I2S(IMSG_NONE);
+	I2S(IMSG_CTL_LOG_VERBOSE);
+	I2S(IMSG_CTL_SHOW_INTERFACE_INFO);
+	I2S(IMSG_CTL_SHOW_INTERFACE_INFO_RA);
+	I2S(IMSG_CTL_SHOW_INTERFACE_INFO_RA_PREFIX);
+	I2S(IMSG_CTL_SHOW_INTERFACE_INFO_RA_RDNS);
+	I2S(IMSG_CTL_SHOW_INTERFACE_INFO_ADDR_PROPOSALS);
+	I2S(IMSG_CTL_SHOW_INTERFACE_INFO_ADDR_PROPOSAL);
+	I2S(IMSG_CTL_SHOW_INTERFACE_INFO_DFR_PROPOSALS);
+	I2S(IMSG_CTL_SHOW_INTERFACE_INFO_DFR_PROPOSAL);
+	I2S(IMSG_CTL_SHOW_INTERFACE_INFO_RDNS_PROPOSALS);
+	I2S(IMSG_CTL_SHOW_INTERFACE_INFO_RDNS_PROPOSAL);
+	I2S(IMSG_CTL_END);
+	I2S(IMSG_PROPOSE_RDNS);
+	I2S(IMSG_REPROPOSE_RDNS);
+	I2S(IMSG_CTL_SEND_SOLICITATION);
+	I2S(IMSG_SOCKET_IPC);
+	I2S(IMSG_OPEN_ICMP6SOCK);
+	I2S(IMSG_ICMP6SOCK);
+	I2S(IMSG_ROUTESOCK);
+	I2S(IMSG_CONTROLFD);
+	I2S(IMSG_STARTUP);
+	I2S(IMSG_UPDATE_IF);
+	I2S(IMSG_REMOVE_IF);
+	I2S(IMSG_RA);
+	I2S(IMSG_CONFIGURE_ADDRESS);
+	I2S(IMSG_WITHDRAW_ADDRESS);
+	I2S(IMSG_DEL_ADDRESS);
+	I2S(IMSG_DEL_ROUTE);
+	I2S(IMSG_CONFIGURE_DFR);
+	I2S(IMSG_WITHDRAW_DFR);
+	I2S(IMSG_DUP_ADDRESS);
+	default:
+		snprintf(unknown, sizeof(unknown), "IMSG_%u", type);
+		return unknown;
+	}
+}
+#undef	I2S
+
+#endif	/* SMALL */
diff --git slaacd.h slaacd.h
index 2844f4a63b7..910392bfaf2 100644
--- slaacd.h
+++ slaacd.h
@@ -206,6 +206,8 @@ int		imsg_compose_event(struct imsgev *, uint16_t, uint32_t, pid_t,
 		    int, void *, uint16_t);
 #ifndef	SMALL
 const char	*sin6_to_str(struct sockaddr_in6 *);
+const char	*i2s(uint32_t);
 #else
 #define	sin6_to_str(x...)	""
+#define	i2s(x...)		""
 #endif	/* SMALL */

commit ad9dff9d8cdbad3151933969eb155e0b0d61384d
Author: Florian Obser <florian@narrans.de>
Date:   Fri Aug 23 17:22:42 2024 +0200

    Do not peek inside of struct imsg.
    
    While here use i2s helper function for error logging.

diff --git control.c control.c
index bd3f2290f95..11a4f2ee608 100644
--- control.c
+++ control.c
@@ -225,6 +225,8 @@ control_dispatch_imsg(int fd, short event, void *bula)
 	struct imsg	 imsg;
 	ssize_t		 n;
 	int		 verbose;
+	uint32_t	 if_index, type;
+	pid_t		 pid;
 
 	if ((c = control_connbyfd(fd)) == NULL) {
 		log_warnx("%s: fd %d: not found", __func__, fd);
@@ -253,37 +255,34 @@ control_dispatch_imsg(int fd, short event, void *bula)
 		if (n == 0)
 			break;
 
-		switch (imsg.hdr.type) {
+		type = imsg_get_type(&imsg);
+		pid = imsg_get_pid(&imsg);
+		switch (type) {
 		case IMSG_CTL_LOG_VERBOSE:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(verbose))
+			if (imsg_get_data(&imsg, &verbose,
+			    sizeof(verbose)) == -1)
 				break;
 
 			/* Forward to all other processes. */
-			frontend_imsg_compose_main(imsg.hdr.type, imsg.hdr.pid,
-			    imsg.data, IMSG_DATA_SIZE(imsg));
-			frontend_imsg_compose_engine(imsg.hdr.type, 0,
-			    imsg.hdr.pid, imsg.data, IMSG_DATA_SIZE(imsg));
+			frontend_imsg_compose_main(type, pid, &verbose,
+			    sizeof(verbose));
+			frontend_imsg_compose_engine(type, 0, pid, &verbose,
+			    sizeof(verbose));
 
-			memcpy(&verbose, imsg.data, sizeof(verbose));
 			log_setverbose(verbose);
 			break;
 		case IMSG_CTL_SHOW_INTERFACE_INFO:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(uint32_t))
-				break;
-			c->iev.ibuf.pid = imsg.hdr.pid;
-			frontend_imsg_compose_engine(imsg.hdr.type, 0,
-			    imsg.hdr.pid, imsg.data, IMSG_DATA_SIZE(imsg));
-			break;
 		case IMSG_CTL_SEND_SOLICITATION:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(uint32_t))
+			if (imsg_get_data(&imsg, &if_index,
+			    sizeof(if_index)) == -1)
 				break;
-			c->iev.ibuf.pid = imsg.hdr.pid;
-			frontend_imsg_compose_engine(imsg.hdr.type, 0,
-			    imsg.hdr.pid, imsg.data, IMSG_DATA_SIZE(imsg));
+
+			c->iev.ibuf.pid = pid;
+			frontend_imsg_compose_engine(type, 0, pid, &if_index,
+			    sizeof(if_index));
 			break;
 		default:
-			log_debug("%s: error handling imsg %d", __func__,
-			    imsg.hdr.type);
+			log_debug("%s: error handling imsg %d", __func__, type);
 			break;
 		}
 		imsg_free(&imsg);
diff --git engine.c engine.c
index f50fd6a821e..818c2d75bf4 100644
--- engine.c
+++ engine.c
@@ -462,7 +462,7 @@ engine_dispatch_frontend(int fd, short event, void *bula)
 #ifndef	SMALL
 	int				 verbose;
 #endif	/* SMALL */
-	uint32_t			 if_index;
+	uint32_t			 if_index, type;
 
 	if (event & EV_READ) {
 		if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN)
@@ -483,36 +483,36 @@ engine_dispatch_frontend(int fd, short event, void *bula)
 		if (n == 0)	/* No more messages. */
 			break;
 
-		switch (imsg.hdr.type) {
+		type = imsg_get_type(&imsg);
+
+		switch (type) {
 #ifndef	SMALL
 		case IMSG_CTL_LOG_VERBOSE:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(verbose))
-				fatalx("%s: IMSG_CTL_LOG_VERBOSE wrong length: "
-				    "%lu", __func__, IMSG_DATA_SIZE(imsg));
-			memcpy(&verbose, imsg.data, sizeof(verbose));
+			if (imsg_get_data(&imsg, &verbose,
+			    sizeof(verbose)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			log_setverbose(verbose);
 			break;
 		case IMSG_CTL_SHOW_INTERFACE_INFO:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(if_index))
-				fatalx("%s: IMSG_CTL_SHOW_INTERFACE_INFO wrong "
-				    "length: %lu", __func__,
-				    IMSG_DATA_SIZE(imsg));
-			memcpy(&if_index, imsg.data, sizeof(if_index));
+			if (imsg_get_data(&imsg, &if_index,
+			    sizeof(if_index)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			engine_showinfo_ctl(&imsg, if_index);
 			break;
 #endif	/* SMALL */
 		case IMSG_REMOVE_IF:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(if_index))
-				fatalx("%s: IMSG_REMOVE_IF wrong length: %lu",
-				    __func__, IMSG_DATA_SIZE(imsg));
-			memcpy(&if_index, imsg.data, sizeof(if_index));
+			if (imsg_get_data(&imsg, &if_index,
+			    sizeof(if_index)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			remove_slaacd_iface(if_index);
 			break;
 		case IMSG_RA:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(ra))
-				fatalx("%s: IMSG_RA wrong length: %lu",
-				    __func__, IMSG_DATA_SIZE(imsg));
-			memcpy(&ra, imsg.data, sizeof(ra));
+			if (imsg_get_data(&imsg, &ra, sizeof(ra)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			iface = get_slaacd_iface_by_id(ra.if_index);
 
 			/*
@@ -524,11 +524,10 @@ engine_dispatch_frontend(int fd, short event, void *bula)
 				parse_ra(iface, &ra);
 			break;
 		case IMSG_CTL_SEND_SOLICITATION:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(if_index))
-				fatalx("%s: IMSG_CTL_SEND_SOLICITATION wrong "
-				    "length: %lu", __func__,
-				    IMSG_DATA_SIZE(imsg));
-			memcpy(&if_index, imsg.data, sizeof(if_index));
+			if (imsg_get_data(&imsg, &if_index,
+			    sizeof(if_index)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			iface = get_slaacd_iface_by_id(if_index);
 			if (iface == NULL)
 				log_warnx("requested to send solicitation on "
@@ -539,10 +538,10 @@ engine_dispatch_frontend(int fd, short event, void *bula)
 			}
 			break;
 		case IMSG_DEL_ADDRESS:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(del_addr))
-				fatalx("%s: IMSG_DEL_ADDRESS wrong length: %lu",
-				    __func__, IMSG_DATA_SIZE(imsg));
-			memcpy(&del_addr, imsg.data, sizeof(del_addr));
+			if (imsg_get_data(&imsg, &del_addr,
+			    sizeof(del_addr)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			iface = get_slaacd_iface_by_id(del_addr.if_index);
 			if (iface == NULL) {
 				log_debug("IMSG_DEL_ADDRESS: unknown interface"
@@ -562,10 +561,10 @@ engine_dispatch_frontend(int fd, short event, void *bula)
 				free_address_proposal(addr_proposal);
 			break;
 		case IMSG_DEL_ROUTE:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(del_route))
-				fatalx("%s: IMSG_DEL_ROUTE wrong length: %lu",
-				    __func__, IMSG_DATA_SIZE(imsg));
-			memcpy(&del_route, imsg.data, sizeof(del_route));
+			if (imsg_get_data(&imsg, &del_route,
+			    sizeof(del_route)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			iface = get_slaacd_iface_by_id(del_route.if_index);
 			if (iface == NULL) {
 				log_debug("IMSG_DEL_ROUTE: unknown interface"
@@ -582,10 +581,10 @@ engine_dispatch_frontend(int fd, short event, void *bula)
 			}
 			break;
 		case IMSG_DUP_ADDRESS:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(dup_addr))
-				fatalx("%s: IMSG_DUP_ADDRESS wrong length: %lu",
-				    __func__, IMSG_DATA_SIZE(imsg));
-			memcpy(&dup_addr, imsg.data, sizeof(dup_addr));
+			if (imsg_get_data(&imsg, &dup_addr,
+			    sizeof(dup_addr)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			iface = get_slaacd_iface_by_id(dup_addr.if_index);
 			if (iface == NULL) {
 				log_debug("IMSG_DUP_ADDRESS: unknown interface"
@@ -606,8 +605,7 @@ engine_dispatch_frontend(int fd, short event, void *bula)
 				    iface->rdomain);
 			break;
 		default:
-			log_debug("%s: unexpected imsg %d", __func__,
-			    imsg.hdr.type);
+			log_debug("%s: unexpected imsg %d", __func__, type);
 			break;
 		}
 		imsg_free(&imsg);
@@ -629,6 +627,7 @@ engine_dispatch_main(int fd, short event, void *bula)
 	struct imsgbuf		*ibuf = &iev->ibuf;
 	struct imsg_ifinfo	 imsg_ifinfo;
 	ssize_t			 n;
+	uint32_t		 type;
 	int			 shut = 0;
 
 	if (event & EV_READ) {
@@ -650,7 +649,9 @@ engine_dispatch_main(int fd, short event, void *bula)
 		if (n == 0)	/* No more messages. */
 			break;
 
-		switch (imsg.hdr.type) {
+		type = imsg_get_type(&imsg);
+
+		switch (type) {
 		case IMSG_SOCKET_IPC:
 			/*
 			 * Setup pipe and event handler to the frontend
@@ -681,15 +682,14 @@ engine_dispatch_main(int fd, short event, void *bula)
 				fatal("pledge");
 			break;
 		case IMSG_UPDATE_IF:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(imsg_ifinfo))
-				fatalx("%s: IMSG_UPDATE_IF wrong length: %lu",
-				    __func__, IMSG_DATA_SIZE(imsg));
-			memcpy(&imsg_ifinfo, imsg.data, sizeof(imsg_ifinfo));
+			if (imsg_get_data(&imsg, &imsg_ifinfo,
+			    sizeof(imsg_ifinfo)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			engine_update_iface(&imsg_ifinfo);
 			break;
 		default:
-			log_debug("%s: unexpected imsg %d", __func__,
-			    imsg.hdr.type);
+			log_debug("%s: unexpected imsg %d", __func__, type);
 			break;
 		}
 		imsg_free(&imsg);
diff --git frontend.c frontend.c
index 7e5024a5c84..0b7659bac1d 100644
--- frontend.c
+++ frontend.c
@@ -282,6 +282,7 @@ frontend_dispatch_main(int fd, short event, void *bula)
 	struct imsgev		*iev = bula;
 	struct imsgbuf		*ibuf = &iev->ibuf;
 	ssize_t			 n;
+	uint32_t		 type;
 	int			 shut = 0, icmp6sock, rdomain;
 
 	if (event & EV_READ) {
@@ -303,7 +304,9 @@ frontend_dispatch_main(int fd, short event, void *bula)
 		if (n == 0)	/* No more messages. */
 			break;
 
-		switch (imsg.hdr.type) {
+		type = imsg_get_type(&imsg);
+
+		switch (type) {
 		case IMSG_SOCKET_IPC:
 			/*
 			 * Setup pipe and event handler to the engine
@@ -335,10 +338,10 @@ frontend_dispatch_main(int fd, short event, void *bula)
 				fatalx("%s: expected to receive imsg "
 				    "ICMPv6 fd but didn't receive any",
 				    __func__);
-			if (IMSG_DATA_SIZE(imsg) != sizeof(rdomain))
-				fatalx("%s: IMSG_ICMP6SOCK wrong length: "
-				    "%lu", __func__, IMSG_DATA_SIZE(imsg));
-			memcpy(&rdomain, imsg.data, sizeof(rdomain));
+			if (imsg_get_data(&imsg, &rdomain,
+			    sizeof(rdomain)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			set_icmp6sock(icmp6sock, rdomain);
 			break;
 		case IMSG_ROUTESOCK:
@@ -346,6 +349,7 @@ frontend_dispatch_main(int fd, short event, void *bula)
 				fatalx("%s: expected to receive imsg "
 				    "routesocket fd but didn't receive any",
 				    __func__);
+
 			event_set(&ev_route, fd, EV_READ | EV_PERSIST,
 			    route_receive, NULL);
 			break;
@@ -358,6 +362,7 @@ frontend_dispatch_main(int fd, short event, void *bula)
 				fatalx("%s: expected to receive imsg "
 				    "control fd but didn't receive any",
 				    __func__);
+
 			/* Listen on control socket. */
 			control_listen(fd);
 			break;
@@ -366,8 +371,7 @@ frontend_dispatch_main(int fd, short event, void *bula)
 			break;
 #endif	/* SMALL */
 		default:
-			log_debug("%s: error handling imsg %d", __func__,
-			    imsg.hdr.type);
+			log_debug("%s: error handling imsg %d", __func__, type);
 			break;
 		}
 		imsg_free(&imsg);
@@ -389,7 +393,7 @@ frontend_dispatch_engine(int fd, short event, void *bula)
 	struct imsg		 imsg;
 	ssize_t			 n;
 	int			 shut = 0;
-	uint32_t		 if_index;
+	uint32_t		 if_index, type;
 
 	if (event & EV_READ) {
 		if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN)
@@ -410,7 +414,9 @@ frontend_dispatch_engine(int fd, short event, void *bula)
 		if (n == 0)	/* No more messages. */
 			break;
 
-		switch (imsg.hdr.type) {
+		type = imsg_get_type(&imsg);
+
+		switch (type) {
 #ifndef	SMALL
 		case IMSG_CTL_END:
 		case IMSG_CTL_SHOW_INTERFACE_INFO:
@@ -427,16 +433,14 @@ frontend_dispatch_engine(int fd, short event, void *bula)
 			break;
 #endif	/* SMALL */
 		case IMSG_CTL_SEND_SOLICITATION:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(if_index))
-				fatalx("%s: IMSG_CTL_SEND_SOLICITATION wrong "
-				    "length: %lu", __func__,
-				    IMSG_DATA_SIZE(imsg));
-			if_index = *((uint32_t *)imsg.data);
+			if (imsg_get_data(&imsg, &if_index,
+			    sizeof(if_index)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			send_solicitation(if_index);
 			break;
 		default:
-			log_debug("%s: error handling imsg %d", __func__,
-			    imsg.hdr.type);
+			log_debug("%s: error handling imsg %d", __func__, type);
 			break;
 		}
 		imsg_free(&imsg);
diff --git slaacd.c slaacd.c
index c6c84d25e2c..43245529543 100644
--- slaacd.c
+++ slaacd.c
@@ -381,6 +381,7 @@ main_dispatch_frontend(int fd, short event, void *bula)
 	struct imsg		 imsg;
 	struct imsg_ifinfo	 imsg_ifinfo;
 	ssize_t			 n;
+	uint32_t		 type;
 	int			 shut = 0;
 	int			 rdomain;
 #ifndef	SMALL
@@ -408,29 +409,30 @@ main_dispatch_frontend(int fd, short event, void *bula)
 		if (n == 0)	/* No more messages. */
 			break;
 
-		switch (imsg.hdr.type) {
+		type = imsg_get_type(&imsg);
+
+		switch (type) {
 		case IMSG_OPEN_ICMP6SOCK:
-			log_debug("IMSG_OPEN_ICMP6SOCK");
-			if (IMSG_DATA_SIZE(imsg) != sizeof(rdomain))
-				fatalx("%s: IMSG_OPEN_ICMP6SOCK wrong length: "
-				    "%lu", __func__, IMSG_DATA_SIZE(imsg));
-			memcpy(&rdomain, imsg.data, sizeof(rdomain));
+			if (imsg_get_data(&imsg, &rdomain,
+			    sizeof(rdomain)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			open_icmp6sock(rdomain);
 			break;
 #ifndef	SMALL
 		case IMSG_CTL_LOG_VERBOSE:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(verbose))
-				fatalx("%s: IMSG_CTL_LOG_VERBOSE wrong length: "
-				    "%lu", __func__, IMSG_DATA_SIZE(imsg));
-			memcpy(&verbose, imsg.data, sizeof(verbose));
+			if (imsg_get_data(&imsg, &verbose,
+			    sizeof(verbose)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			log_setverbose(verbose);
 			break;
 #endif	/* SMALL */
 		case IMSG_UPDATE_IF:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(imsg_ifinfo))
-				fatalx("%s: IMSG_UPDATE_IF wrong length: %lu",
-				    __func__, IMSG_DATA_SIZE(imsg));
-			memcpy(&imsg_ifinfo, imsg.data, sizeof(imsg_ifinfo));
+			if (imsg_get_data(&imsg, &imsg_ifinfo,
+			    sizeof(imsg_ifinfo)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			if (get_soiikey(imsg_ifinfo.soiikey) == -1)
 				log_warn("get_soiikey");
 			else
@@ -438,8 +440,7 @@ main_dispatch_frontend(int fd, short event, void *bula)
 				    &imsg_ifinfo, sizeof(imsg_ifinfo));
 			break;
 		default:
-			log_debug("%s: error handling imsg %d", __func__,
-			    imsg.hdr.type);
+			log_debug("%s: error handling imsg %d", __func__, type);
 			break;
 		}
 		imsg_free(&imsg);
@@ -463,6 +464,7 @@ main_dispatch_engine(int fd, short event, void *bula)
 	struct imsg_configure_dfr	 dfr;
 	struct imsg_propose_rdns	 rdns;
 	ssize_t				 n;
+	uint32_t			 type;
 	int				 shut = 0;
 
 	ibuf = &iev->ibuf;
@@ -486,54 +488,47 @@ main_dispatch_engine(int fd, short event, void *bula)
 		if (n == 0)	/* No more messages. */
 			break;
 
-		switch (imsg.hdr.type) {
+		type = imsg_get_type(&imsg);
+
+		switch (type) {
 		case IMSG_CONFIGURE_ADDRESS:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(address))
-				fatalx("%s: IMSG_CONFIGURE_ADDRESS wrong "
-				    "length: %lu", __func__,
-				    IMSG_DATA_SIZE(imsg));
-			memcpy(&address, imsg.data, sizeof(address));
+			if (imsg_get_data(&imsg, &address,
+			    sizeof(address)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			configure_interface(&address);
 			break;
 		case IMSG_WITHDRAW_ADDRESS:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(address))
-				fatalx("%s: IMSG_WITHDRAW_ADDRESS wrong "
-				    "length: %lu", __func__,
-				    IMSG_DATA_SIZE(imsg));
-			memcpy(&address, imsg.data, sizeof(address));
+			if (imsg_get_data(&imsg, &address,
+			    sizeof(address)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			delete_address(&address);
 			break;
 		case IMSG_CONFIGURE_DFR:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(dfr))
-				fatalx("%s: IMSG_CONFIGURE_DFR wrong "
-				    "length: %lu", __func__,
-				    IMSG_DATA_SIZE(imsg));
-			memcpy(&dfr, imsg.data, sizeof(dfr));
+			if (imsg_get_data(&imsg, &dfr, sizeof(dfr)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			add_gateway(&dfr);
 			break;
 		case IMSG_WITHDRAW_DFR:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(dfr))
-				fatalx("%s: IMSG_WITHDRAW_DFR wrong "
-				    "length: %lu", __func__,
-				    IMSG_DATA_SIZE(imsg));
-			memcpy(&dfr, imsg.data, sizeof(dfr));
+			if (imsg_get_data(&imsg, &dfr, sizeof(dfr)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
+
 			delete_gateway(&dfr);
 			break;
 		case IMSG_PROPOSE_RDNS:
-			if (IMSG_DATA_SIZE(imsg) != sizeof(rdns))
-				fatalx("%s: IMSG_PROPOSE_RDNS wrong "
-				    "length: %lu", __func__,
-				    IMSG_DATA_SIZE(imsg));
-			memcpy(&rdns, imsg.data, sizeof(rdns));
+			if (imsg_get_data(&imsg, &rdns, sizeof(rdns)) == -1)
+				fatalx("%s: invalid %s", __func__, i2s(type));
 			if ((2 + rdns.rdns_count * sizeof(struct in6_addr)) >
 			    sizeof(struct sockaddr_rtdns))
 				fatalx("%s: rdns_count too big: %d", __func__,
 				    rdns.rdns_count);
+
 			send_rdns_proposal(&rdns);
 			break;
 		default:
-			log_debug("%s: error handling imsg %d", __func__,
-			    imsg.hdr.type);
+			log_debug("%s: error handling imsg %d", __func__, type);
 			break;
 		}
 		imsg_free(&imsg);

-- 
In my defence, I have been left unsupervised.