From 33d2acb6252095c2288251910767e7a74a54f912 Mon Sep 17 00:00:00 2001 From: florian Date: Sat, 24 Aug 2024 09:44:41 +0000 Subject: [PATCH] Do not peek inside of struct imsg. While here use i2s helper function for error logging. OK tb --- sbin/slaacd/control.c | 39 +++++++++--------- sbin/slaacd/engine.c | 92 +++++++++++++++++++++--------------------- sbin/slaacd/frontend.c | 38 +++++++++-------- sbin/slaacd/slaacd.c | 85 ++++++++++++++++++-------------------- 4 files changed, 126 insertions(+), 128 deletions(-) diff --git a/sbin/slaacd/control.c b/sbin/slaacd/control.c index bd3f2290f95..a5fb37fb4bc 100644 --- a/sbin/slaacd/control.c +++ b/sbin/slaacd/control.c @@ -1,4 +1,4 @@ -/* $OpenBSD: control.c,v 1.9 2021/03/20 16:46:03 kn Exp $ */ +/* $OpenBSD: control.c,v 1.10 2024/08/24 09:44:41 florian Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -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 a/sbin/slaacd/engine.c b/sbin/slaacd/engine.c index f50fd6a821e..de88b89026b 100644 --- a/sbin/slaacd/engine.c +++ b/sbin/slaacd/engine.c @@ -1,4 +1,4 @@ -/* $OpenBSD: engine.c,v 1.91 2024/07/13 16:06:34 florian Exp $ */ +/* $OpenBSD: engine.c,v 1.92 2024/08/24 09:44:41 florian Exp $ */ /* * Copyright (c) 2017 Florian Obser @@ -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 a/sbin/slaacd/frontend.c b/sbin/slaacd/frontend.c index 7e5024a5c84..388fb82d6b2 100644 --- a/sbin/slaacd/frontend.c +++ b/sbin/slaacd/frontend.c @@ -1,4 +1,4 @@ -/* $OpenBSD: frontend.c,v 1.67 2024/06/03 17:58:33 deraadt Exp $ */ +/* $OpenBSD: frontend.c,v 1.68 2024/08/24 09:44:41 florian Exp $ */ /* * Copyright (c) 2017 Florian Obser @@ -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 a/sbin/slaacd/slaacd.c b/sbin/slaacd/slaacd.c index 8d3f183d716..c76bae07ed3 100644 --- a/sbin/slaacd/slaacd.c +++ b/sbin/slaacd/slaacd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: slaacd.c,v 1.70 2024/08/24 09:42:40 florian Exp $ */ +/* $OpenBSD: slaacd.c,v 1.71 2024/08/24 09:44:41 florian Exp $ */ /* * Copyright (c) 2017 Florian Obser @@ -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); -- 2.20.1