Do not peek inside of struct imsg.
authorflorian <florian@openbsd.org>
Sat, 24 Aug 2024 09:44:41 +0000 (09:44 +0000)
committerflorian <florian@openbsd.org>
Sat, 24 Aug 2024 09:44:41 +0000 (09:44 +0000)
While here use i2s helper function for error logging.

OK tb

sbin/slaacd/control.c
sbin/slaacd/engine.c
sbin/slaacd/frontend.c
sbin/slaacd/slaacd.c

index bd3f229..a5fb37f 100644 (file)
@@ -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 <henning@openbsd.org>
@@ -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);
index f50fd6a..de88b89 100644 (file)
@@ -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 <florian@openbsd.org>
@@ -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);
index 7e5024a..388fb82 100644 (file)
@@ -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 <florian@openbsd.org>
@@ -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);
index 8d3f183..c76bae0 100644 (file)
@@ -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 <florian@openbsd.org>
@@ -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);