From 45c5e5ad7fe04b5a32b6a8aefc3d958045aa1a50 Mon Sep 17 00:00:00 2001 From: florian Date: Sun, 25 Aug 2024 09:53:53 +0000 Subject: [PATCH] Do not peek inside of struct imsg. input & OK tb --- sbin/dhcpleased/control.c | 52 +++++++----- sbin/dhcpleased/dhcpleased.c | 153 +++++++++++++++++++++++----------- sbin/dhcpleased/dhcpleased.h | 12 +-- sbin/dhcpleased/engine.c | 155 +++++++++++++++++------------------ sbin/dhcpleased/frontend.c | 125 ++++++++++++++-------------- 5 files changed, 282 insertions(+), 215 deletions(-) diff --git a/sbin/dhcpleased/control.c b/sbin/dhcpleased/control.c index d29e3fb1639..b6043f58088 100644 --- a/sbin/dhcpleased/control.c +++ b/sbin/dhcpleased/control.c @@ -1,4 +1,4 @@ -/* $OpenBSD: control.c,v 1.4 2021/08/01 09:07:03 florian Exp $ */ +/* $OpenBSD: control.c,v 1.5 2024/08/25 09:53:53 florian Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -224,6 +224,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); @@ -252,40 +254,47 @@ 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_RELOAD: - frontend_imsg_compose_main(imsg.hdr.type, 0, NULL, 0); + frontend_imsg_compose_main(type, 0, NULL, 0); break; case IMSG_CTL_LOG_VERBOSE: - if (IMSG_DATA_SIZE(imsg) != sizeof(verbose)) + if (imsg_get_data(&imsg, &verbose, + sizeof(verbose)) == -1) break; - c->iev.ibuf.pid = imsg.hdr.pid; + + c->iev.ibuf.pid = pid; /* 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)) + 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; case IMSG_CTL_SEND_REQUEST: - 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; + + c->iev.ibuf.pid = pid; frontend_imsg_compose_engine(IMSG_REQUEST_REBOOT, 0, - imsg.hdr.pid, imsg.data, IMSG_DATA_SIZE(imsg)); + 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); @@ -299,9 +308,8 @@ control_imsg_relay(struct imsg *imsg) { struct ctl_conn *c; - if ((c = control_connbypid(imsg->hdr.pid)) == NULL) + if ((c = control_connbypid(imsg_get_pid(imsg))) == NULL) return (0); - return (imsg_compose_event(&c->iev, imsg->hdr.type, 0, imsg->hdr.pid, - -1, imsg->data, IMSG_DATA_SIZE(*imsg))); + return (imsg_forward_event(&c->iev, imsg)); } diff --git a/sbin/dhcpleased/dhcpleased.c b/sbin/dhcpleased/dhcpleased.c index b5f65046894..0a6a7e5946a 100644 --- a/sbin/dhcpleased/dhcpleased.c +++ b/sbin/dhcpleased/dhcpleased.c @@ -1,4 +1,4 @@ -/* $OpenBSD: dhcpleased.c,v 1.30 2023/10/10 16:09:53 florian Exp $ */ +/* $OpenBSD: dhcpleased.c,v 1.31 2024/08/25 09:53:53 florian Exp $ */ /* * Copyright (c) 2017, 2021 Florian Obser @@ -436,7 +436,7 @@ main_dispatch_frontend(int fd, short event, void *bula) struct imsg_ifinfo imsg_ifinfo; ssize_t n; int shut = 0; - uint32_t if_index; + uint32_t if_index, type; #ifndef SMALL int verbose; #endif /* SMALL */ @@ -462,12 +462,14 @@ 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_BPFSOCK: - if (IMSG_DATA_SIZE(imsg) != sizeof(if_index)) - fatalx("%s: IMSG_OPEN_BPFSOCK 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)); + open_bpfsock(if_index); break; #ifndef SMALL @@ -478,25 +480,24 @@ main_dispatch_frontend(int fd, short event, void *bula) log_warnx("configuration reloaded"); break; 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)); + read_lease_file(&imsg_ifinfo); main_imsg_compose_engine(IMSG_UPDATE_IF, -1, &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); @@ -517,6 +518,7 @@ main_dispatch_engine(int fd, short event, void *bula) struct imsgbuf *ibuf; struct imsg imsg; ssize_t n; + uint32_t type; int shut = 0; ibuf = &iev->ibuf; @@ -540,15 +542,16 @@ 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_INTERFACE: { struct imsg_configure_interface imsg_interface; - if (IMSG_DATA_SIZE(imsg) != sizeof(imsg_interface)) - fatalx("%s: IMSG_CONFIGURE_INTERFACE wrong " - "length: %lu", __func__, - IMSG_DATA_SIZE(imsg)); - memcpy(&imsg_interface, imsg.data, - sizeof(imsg_interface)); + + if (imsg_get_data(&imsg, &imsg_interface, + sizeof(imsg_interface)) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); + if (imsg_interface.routes_len >= MAX_DHCP_ROUTES) fatalx("%s: too many routes in imsg", __func__); configure_interface(&imsg_interface); @@ -556,14 +559,13 @@ main_dispatch_engine(int fd, short event, void *bula) } case IMSG_DECONFIGURE_INTERFACE: { struct imsg_configure_interface imsg_interface; - if (IMSG_DATA_SIZE(imsg) != sizeof(imsg_interface)) - fatalx("%s: IMSG_CONFIGURE_INTERFACE wrong " - "length: %lu", __func__, - IMSG_DATA_SIZE(imsg)); - memcpy(&imsg_interface, imsg.data, - sizeof(imsg_interface)); + + if (imsg_get_data(&imsg, &imsg_interface, + sizeof(imsg_interface)) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); if (imsg_interface.routes_len >= MAX_DHCP_ROUTES) fatalx("%s: too many routes in imsg", __func__); + deconfigure_interface(&imsg_interface); main_imsg_compose_frontend(IMSG_CLOSE_UDPSOCK, -1, &imsg_interface.if_index, @@ -572,48 +574,44 @@ main_dispatch_engine(int fd, short event, void *bula) } case IMSG_WITHDRAW_ROUTES: { struct imsg_configure_interface imsg_interface; - if (IMSG_DATA_SIZE(imsg) != sizeof(imsg_interface)) - fatalx("%s: IMSG_CONFIGURE_INTERFACE wrong " - "length: %lu", __func__, - IMSG_DATA_SIZE(imsg)); - memcpy(&imsg_interface, imsg.data, - sizeof(imsg_interface)); + + if (imsg_get_data(&imsg, &imsg_interface, + sizeof(imsg_interface)) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); if (imsg_interface.routes_len >= MAX_DHCP_ROUTES) fatalx("%s: too many routes in imsg", __func__); + if (imsg_interface.routes_len > 0) configure_routes(RTM_DELETE, &imsg_interface); break; } case IMSG_PROPOSE_RDNS: { struct imsg_propose_rdns 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 in_addr)) > sizeof(struct sockaddr_rtdns)) fatalx("%s: rdns_count too big: %d", __func__, rdns.rdns_count); + propose_rdns(&rdns); break; } case IMSG_WITHDRAW_RDNS: { struct imsg_propose_rdns rdns; - if (IMSG_DATA_SIZE(imsg) != sizeof(rdns)) - fatalx("%s: IMSG_WITHDRAW_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 (rdns.rdns_count != 0) fatalx("%s: expected rdns_count == 0: %d", __func__, rdns.rdns_count); + propose_rdns(&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); @@ -672,6 +670,17 @@ imsg_compose_event(struct imsgev *iev, uint16_t type, uint32_t peerid, return (ret); } +int +imsg_forward_event(struct imsgev *iev, struct imsg *imsg) +{ + int ret; + + if ((ret = imsg_forward(&iev->ibuf, imsg)) != -1) + imsg_event_add(iev); + + return (ret); +} + static int main_imsg_send_ipc_sockets(struct imsgbuf *frontend_buf, struct imsgbuf *engine_buf) @@ -1293,4 +1302,52 @@ config_clear(struct dhcpleased_conf *conf) free(conf); } + +#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_SEND_REQUEST); + I2S(IMSG_CTL_RELOAD); + I2S(IMSG_CTL_END); + I2S(IMSG_RECONF_CONF); + I2S(IMSG_RECONF_IFACE); + I2S(IMSG_RECONF_VC_ID); + I2S(IMSG_RECONF_C_ID); + I2S(IMSG_RECONF_H_NAME); + I2S(IMSG_RECONF_END); + I2S(IMSG_SEND_DISCOVER); + I2S(IMSG_SEND_REQUEST); + I2S(IMSG_SOCKET_IPC); + I2S(IMSG_OPEN_BPFSOCK); + I2S(IMSG_BPFSOCK); + I2S(IMSG_UDPSOCK); + I2S(IMSG_CLOSE_UDPSOCK); + I2S(IMSG_ROUTESOCK); + I2S(IMSG_CONTROLFD); + I2S(IMSG_STARTUP); + I2S(IMSG_UPDATE_IF); + I2S(IMSG_REMOVE_IF); + I2S(IMSG_DHCP); + I2S(IMSG_CONFIGURE_INTERFACE); + I2S(IMSG_DECONFIGURE_INTERFACE); + I2S(IMSG_PROPOSE_RDNS); + I2S(IMSG_WITHDRAW_RDNS); + I2S(IMSG_WITHDRAW_ROUTES); + I2S(IMSG_REPROPOSE_RDNS); + I2S(IMSG_REQUEST_REBOOT); + default: + snprintf(unknown, sizeof(unknown), "IMSG_%u", type); + return unknown; + } +} +#undef I2S + #endif /* SMALL */ diff --git a/sbin/dhcpleased/dhcpleased.h b/sbin/dhcpleased/dhcpleased.h index 41749fc5031..d3d79b61c68 100644 --- a/sbin/dhcpleased/dhcpleased.h +++ b/sbin/dhcpleased/dhcpleased.h @@ -1,4 +1,4 @@ -/* $OpenBSD: dhcpleased.h,v 1.16 2024/01/26 21:14:08 jan Exp $ */ +/* $OpenBSD: dhcpleased.h,v 1.17 2024/08/25 09:53:53 florian Exp $ */ /* * Copyright (c) 2017, 2021 Florian Obser @@ -158,7 +158,6 @@ #define MAX_SERVERS 16 /* max servers that can be ignored per if */ -#define IMSG_DATA_SIZE(imsg) ((imsg).hdr.len - IMSG_HEADER_SIZE) #define DHCP_SNAME_LEN 64 #define DHCP_FILE_LEN 128 @@ -252,9 +251,9 @@ struct iface_conf { SIMPLEQ_ENTRY(iface_conf) entry; char name[IF_NAMESIZE]; uint8_t *vc_id; - int vc_id_len; + size_t vc_id_len; uint8_t *c_id; - int c_id_len; + size_t c_id_len; char *h_name; int ignore; struct in_addr ignore_servers[MAX_SERVERS]; @@ -304,12 +303,14 @@ struct imsg_req_dhcp { void imsg_event_add(struct imsgev *); int imsg_compose_event(struct imsgev *, uint16_t, uint32_t, pid_t, int, void *, uint16_t); +int imsg_forward_event(struct imsgev *, struct imsg *); #ifndef SMALL void config_clear(struct dhcpleased_conf *); struct dhcpleased_conf *config_new_empty(void); void merge_config(struct dhcpleased_conf *, struct dhcpleased_conf *); const char *sin_to_str(struct sockaddr_in *); +const char *i2s(uint32_t); /* frontend.c */ struct iface_conf *find_iface_conf(struct iface_conf_head *, char *); @@ -323,6 +324,7 @@ void print_config(struct dhcpleased_conf *); struct dhcpleased_conf *parse_config(const char *); int cmdline_symset(char *); #else -#define sin_to_str(x...) "" +#define sin_to_str(x) "" +#define i2s(x) "" #endif /* SMALL */ diff --git a/sbin/dhcpleased/engine.c b/sbin/dhcpleased/engine.c index 289dffdf20a..cdb06545b20 100644 --- a/sbin/dhcpleased/engine.c +++ b/sbin/dhcpleased/engine.c @@ -1,4 +1,4 @@ -/* $OpenBSD: engine.c,v 1.45 2024/06/03 17:58:33 deraadt Exp $ */ +/* $OpenBSD: engine.c,v 1.46 2024/08/25 09:53:53 florian Exp $ */ /* * Copyright (c) 2017, 2021 Florian Obser @@ -127,7 +127,7 @@ void engine_dispatch_frontend(int, short, void *); void engine_dispatch_main(int, short, void *); #ifndef SMALL void send_interface_info(struct dhcpleased_iface *, pid_t); -void engine_showinfo_ctl(struct imsg *, uint32_t); +void engine_showinfo_ctl(pid_t, uint32_t); #endif /* SMALL */ void engine_update_iface(struct imsg_ifinfo *); struct dhcpleased_iface *get_dhcpleased_iface_by_id(uint32_t); @@ -286,7 +286,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) @@ -307,29 +307,29 @@ 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)); - engine_showinfo_ctl(&imsg, if_index); + if (imsg_get_data(&imsg, &if_index, + sizeof(if_index)) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); + + engine_showinfo_ctl(imsg_get_pid(&imsg), if_index); break; case IMSG_REQUEST_REBOOT: - if (IMSG_DATA_SIZE(imsg) != sizeof(if_index)) - fatalx("%s: IMSG_CTL_SEND_DISCOVER 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_dhcpleased_iface_by_id(if_index); if (iface != NULL) { switch (iface->state) { @@ -351,18 +351,19 @@ engine_dispatch_frontend(int fd, short event, void *bula) 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_dhcpleased_iface(if_index); break; case IMSG_DHCP: { struct imsg_dhcp imsg_dhcp; - if (IMSG_DATA_SIZE(imsg) != sizeof(imsg_dhcp)) - fatalx("%s: IMSG_DHCP wrong length: %lu", - __func__, IMSG_DATA_SIZE(imsg)); - memcpy(&imsg_dhcp, imsg.data, sizeof(imsg_dhcp)); + + if (imsg_get_data(&imsg, &imsg_dhcp, + sizeof(imsg_dhcp)) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); + iface = get_dhcpleased_iface_by_id(imsg_dhcp.if_index); if (iface != NULL) parse_dhcp(iface, &imsg_dhcp); @@ -373,8 +374,7 @@ engine_dispatch_frontend(int fd, short event, void *bula) send_rdns_proposal(iface); break; default: - log_debug("%s: unexpected imsg %d", __func__, - imsg.hdr.type); + log_debug("%s: unexpected imsg %d", __func__, type); break; } imsg_free(&imsg); @@ -400,6 +400,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) { @@ -421,7 +422,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 @@ -453,12 +456,12 @@ engine_dispatch_main(int fd, short event, void *bula) 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)); if (imsg_ifinfo.lease[LEASE_SIZE - 1] != '\0') fatalx("Invalid lease"); + engine_update_iface(&imsg_ifinfo); break; #ifndef SMALL @@ -472,15 +475,14 @@ engine_dispatch_main(int fd, short event, void *bula) SIMPLEQ_INIT(&nconf->iface_list); break; case IMSG_RECONF_IFACE: - if (IMSG_DATA_SIZE(imsg) != sizeof(struct - iface_conf)) - fatalx("%s: IMSG_RECONF_IFACE wrong length: " - "%lu", __func__, IMSG_DATA_SIZE(imsg)); if ((iface_conf = malloc(sizeof(struct iface_conf))) == NULL) fatal(NULL); - memcpy(iface_conf, imsg.data, sizeof(struct - iface_conf)); + + if (imsg_get_data(&imsg, iface_conf, + sizeof(struct iface_conf)) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); + iface_conf->vc_id = NULL; iface_conf->vc_id_len = 0; iface_conf->c_id = NULL; @@ -491,44 +493,48 @@ engine_dispatch_main(int fd, short event, void *bula) break; case IMSG_RECONF_VC_ID: if (iface_conf == NULL) - fatal("IMSG_RECONF_VC_ID without " + fatalx("IMSG_RECONF_VC_ID without " "IMSG_RECONF_IFACE"); - if (IMSG_DATA_SIZE(imsg) > 255 + 2) - fatalx("%s: IMSG_RECONF_VC_ID wrong length: " - "%lu", __func__, IMSG_DATA_SIZE(imsg)); - if ((iface_conf->vc_id = malloc(IMSG_DATA_SIZE(imsg))) + if ((iface_conf->vc_id_len = imsg_get_len(&imsg)) + > 255 + 2 || iface_conf->vc_id_len == 0) + fatalx("%s: invalid %s", __func__, i2s(type)); + if ((iface_conf->vc_id = malloc(iface_conf->vc_id_len)) == NULL) fatal(NULL); - memcpy(iface_conf->vc_id, imsg.data, - IMSG_DATA_SIZE(imsg)); - iface_conf->vc_id_len = IMSG_DATA_SIZE(imsg); + if (imsg_get_data(&imsg, iface_conf->vc_id, + iface_conf->vc_id_len) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); break; case IMSG_RECONF_C_ID: if (iface_conf == NULL) - fatal("IMSG_RECONF_C_ID without " + fatalx("IMSG_RECONF_C_ID without " "IMSG_RECONF_IFACE"); - if (IMSG_DATA_SIZE(imsg) > 255 + 2) - fatalx("%s: IMSG_RECONF_C_ID wrong length: " - "%lu", __func__, IMSG_DATA_SIZE(imsg)); - if ((iface_conf->c_id = malloc(IMSG_DATA_SIZE(imsg))) + if ((iface_conf->c_id_len = imsg_get_len(&imsg)) + > 255 + 2 || iface_conf->c_id_len == 0) + fatalx("%s: invalid %s", __func__, i2s(type)); + if ((iface_conf->c_id = malloc(iface_conf->c_id_len)) == NULL) fatal(NULL); - memcpy(iface_conf->c_id, imsg.data, - IMSG_DATA_SIZE(imsg)); - iface_conf->c_id_len = IMSG_DATA_SIZE(imsg); + if (imsg_get_data(&imsg, iface_conf->c_id, + iface_conf->c_id_len) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); break; - case IMSG_RECONF_H_NAME: + case IMSG_RECONF_H_NAME: { + size_t len; + if (iface_conf == NULL) - fatal("IMSG_RECONF_H_NAME without " + fatalx("IMSG_RECONF_H_NAME without " "IMSG_RECONF_IFACE"); - if (((char *)imsg.data)[IMSG_DATA_SIZE(imsg) - 1] != - '\0') - fatalx("Invalid hostname"); - if (IMSG_DATA_SIZE(imsg) > 256) - fatalx("Invalid hostname"); - if ((iface_conf->h_name = strdup(imsg.data)) == NULL) + if ((len = imsg_get_len(&imsg)) > 256 || len == 0) + fatalx("%s: invalid %s", __func__, i2s(type)); + if ((iface_conf->h_name = malloc(len)) == NULL) fatal(NULL); + if (imsg_get_data(&imsg, iface_conf->h_name, len) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); + if (iface_conf->h_name[len - 1] != '\0') + fatalx("Invalid hostname"); break; + } case IMSG_RECONF_END: { struct dhcpleased_iface *iface; int *ifaces; @@ -562,8 +568,7 @@ engine_dispatch_main(int fd, short event, void *bula) } #endif /* SMALL */ default: - log_debug("%s: unexpected imsg %d", __func__, - imsg.hdr.type); + log_debug("%s: unexpected imsg %d", __func__, type); break; } imsg_free(&imsg); @@ -605,22 +610,14 @@ send_interface_info(struct dhcpleased_iface *iface, pid_t pid) } void -engine_showinfo_ctl(struct imsg *imsg, uint32_t if_index) +engine_showinfo_ctl(pid_t pid, uint32_t if_index) { struct dhcpleased_iface *iface; - switch (imsg->hdr.type) { - case IMSG_CTL_SHOW_INTERFACE_INFO: - if ((iface = get_dhcpleased_iface_by_id(if_index)) != NULL) - send_interface_info(iface, imsg->hdr.pid); - else - engine_imsg_compose_frontend(IMSG_CTL_END, - imsg->hdr.pid, NULL, 0); - break; - default: - log_debug("%s: error handling imsg", __func__); - break; - } + if ((iface = get_dhcpleased_iface_by_id(if_index)) != NULL) + send_interface_info(iface, pid); + else + engine_imsg_compose_frontend(IMSG_CTL_END, pid, NULL, 0); } #endif /* SMALL */ diff --git a/sbin/dhcpleased/frontend.c b/sbin/dhcpleased/frontend.c index fdc4ae8ca16..ddb72eb4b4f 100644 --- a/sbin/dhcpleased/frontend.c +++ b/sbin/dhcpleased/frontend.c @@ -1,4 +1,4 @@ -/* $OpenBSD: frontend.c,v 1.35 2024/06/27 14:53:06 florian Exp $ */ +/* $OpenBSD: frontend.c,v 1.36 2024/08/25 09:53:53 florian Exp $ */ /* * Copyright (c) 2017, 2021 Florian Obser @@ -238,6 +238,7 @@ frontend_dispatch_main(int fd, short event, void *bula) struct imsgbuf *ibuf = &iev->ibuf; struct iface *iface; ssize_t n; + uint32_t type; int shut = 0, bpfsock, if_index, udpsock; if (event & EV_READ) { @@ -259,7 +260,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 @@ -291,10 +294,10 @@ frontend_dispatch_main(int fd, short event, void *bula) fatalx("%s: expected to receive imsg " "bpf fd but didn't receive any", __func__); - if (IMSG_DATA_SIZE(imsg) != sizeof(if_index)) - fatalx("%s: IMSG_BPFSOCK 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)); + set_bpfsock(bpfsock, if_index); break; case IMSG_UDPSOCK: @@ -302,10 +305,10 @@ frontend_dispatch_main(int fd, short event, void *bula) fatalx("%s: expected to receive imsg " "udpsocket fd but didn't receive any", __func__); - if (IMSG_DATA_SIZE(imsg) != sizeof(if_index)) - fatalx("%s: IMSG_UDPSOCK 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)); + if ((iface = get_iface_by_id(if_index)) == NULL) { close(fd); break; @@ -316,10 +319,10 @@ frontend_dispatch_main(int fd, short event, void *bula) iface->udpsock = udpsock; break; case IMSG_CLOSE_UDPSOCK: - if (IMSG_DATA_SIZE(imsg) != sizeof(if_index)) - fatalx("%s: IMSG_UDPSOCK 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)); + if ((iface = get_iface_by_id(if_index)) != NULL && iface->udpsock != -1) { close(iface->udpsock); @@ -348,15 +351,14 @@ frontend_dispatch_main(int fd, short event, void *bula) SIMPLEQ_INIT(&nconf->iface_list); break; case IMSG_RECONF_IFACE: - if (IMSG_DATA_SIZE(imsg) != sizeof(struct - iface_conf)) - fatalx("%s: IMSG_RECONF_IFACE wrong length: " - "%lu", __func__, IMSG_DATA_SIZE(imsg)); if ((iface_conf = malloc(sizeof(struct iface_conf))) == NULL) fatal(NULL); - memcpy(iface_conf, imsg.data, sizeof(struct - iface_conf)); + + if (imsg_get_data(&imsg, iface_conf, + sizeof(struct iface_conf)) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); + iface_conf->vc_id = NULL; iface_conf->vc_id_len = 0; iface_conf->c_id = NULL; @@ -367,44 +369,48 @@ frontend_dispatch_main(int fd, short event, void *bula) break; case IMSG_RECONF_VC_ID: if (iface_conf == NULL) - fatal("IMSG_RECONF_VC_ID without " + fatalx("IMSG_RECONF_VC_ID without " "IMSG_RECONF_IFACE"); - if (IMSG_DATA_SIZE(imsg) > 255 + 2) - fatalx("%s: IMSG_RECONF_VC_ID wrong length: " - "%lu", __func__, IMSG_DATA_SIZE(imsg)); - if ((iface_conf->vc_id = malloc(IMSG_DATA_SIZE(imsg))) + if ((iface_conf->vc_id_len = imsg_get_len(&imsg)) + > 255 + 2 || iface_conf->vc_id_len == 0) + fatalx("%s: invalid %s", __func__, i2s(type)); + if ((iface_conf->vc_id = malloc(iface_conf->vc_id_len)) == NULL) fatal(NULL); - memcpy(iface_conf->vc_id, imsg.data, - IMSG_DATA_SIZE(imsg)); - iface_conf->vc_id_len = IMSG_DATA_SIZE(imsg); + if (imsg_get_data(&imsg, iface_conf->vc_id, + iface_conf->vc_id_len) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); break; case IMSG_RECONF_C_ID: if (iface_conf == NULL) - fatal("IMSG_RECONF_C_ID without " + fatalx("IMSG_RECONF_C_ID without " "IMSG_RECONF_IFACE"); - if (IMSG_DATA_SIZE(imsg) > 255 + 2) - fatalx("%s: IMSG_RECONF_C_ID wrong length: " - "%lu", __func__, IMSG_DATA_SIZE(imsg)); - if ((iface_conf->c_id = malloc(IMSG_DATA_SIZE(imsg))) + if ((iface_conf->c_id_len = imsg_get_len(&imsg)) + > 255 + 2 || iface_conf->c_id_len == 0) + fatalx("%s: invalid %s", __func__, i2s(type)); + if ((iface_conf->c_id = malloc(iface_conf->c_id_len)) == NULL) fatal(NULL); - memcpy(iface_conf->c_id, imsg.data, - IMSG_DATA_SIZE(imsg)); - iface_conf->c_id_len = IMSG_DATA_SIZE(imsg); + if (imsg_get_data(&imsg, iface_conf->c_id, + iface_conf->c_id_len) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); break; - case IMSG_RECONF_H_NAME: + case IMSG_RECONF_H_NAME: { + size_t len; + if (iface_conf == NULL) - fatal("IMSG_RECONF_H_NAME without " + fatalx("IMSG_RECONF_H_NAME without " "IMSG_RECONF_IFACE"); - if (((char *)imsg.data)[IMSG_DATA_SIZE(imsg) - 1] != - '\0') - fatalx("Invalid hostname"); - if (IMSG_DATA_SIZE(imsg) > 256) - fatalx("Invalid hostname"); - if ((iface_conf->h_name = strdup(imsg.data)) == NULL) + if ((len = imsg_get_len(&imsg)) > 256 || len == 0) + fatalx("%s: invalid %s", __func__, i2s(type)); + if ((iface_conf->h_name = malloc(len)) == NULL) fatal(NULL); + if (imsg_get_data(&imsg, iface_conf->h_name, len) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); + if (iface_conf->h_name[len - 1] != '\0') + fatalx("Invalid hostname"); break; + } case IMSG_RECONF_END: { int i; int *ifaces; @@ -442,8 +448,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); @@ -465,6 +470,7 @@ frontend_dispatch_engine(int fd, short event, void *bula) struct imsg imsg; struct iface *iface; ssize_t n; + uint32_t type; int shut = 0; if (event & EV_READ) { @@ -486,7 +492,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: @@ -495,12 +503,10 @@ frontend_dispatch_engine(int fd, short event, void *bula) #endif /* SMALL */ case IMSG_SEND_DISCOVER: { struct imsg_req_dhcp imsg_req_dhcp; - if (IMSG_DATA_SIZE(imsg) != sizeof(imsg_req_dhcp)) - fatalx("%s: IMSG_SEND_DISCOVER wrong " - "length: %lu", __func__, - IMSG_DATA_SIZE(imsg)); - memcpy(&imsg_req_dhcp, imsg.data, - sizeof(imsg_req_dhcp)); + + if (imsg_get_data(&imsg, &imsg_req_dhcp, + sizeof(imsg_req_dhcp)) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); iface = get_iface_by_id(imsg_req_dhcp.if_index); @@ -513,12 +519,10 @@ frontend_dispatch_engine(int fd, short event, void *bula) } case IMSG_SEND_REQUEST: { struct imsg_req_dhcp imsg_req_dhcp; - if (IMSG_DATA_SIZE(imsg) != sizeof(imsg_req_dhcp)) - fatalx("%s: IMSG_SEND_REQUEST wrong " - "length: %lu", __func__, - IMSG_DATA_SIZE(imsg)); - memcpy(&imsg_req_dhcp, imsg.data, - sizeof(imsg_req_dhcp)); + + if (imsg_get_data(&imsg, &imsg_req_dhcp, + sizeof(imsg_req_dhcp)) == -1) + fatalx("%s: invalid %s", __func__, i2s(type)); iface = get_iface_by_id(imsg_req_dhcp.if_index); @@ -530,8 +534,7 @@ frontend_dispatch_engine(int fd, short event, void *bula) 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