From fb29cdbef21f1d66e2ebe600601897b6578d3959 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 24 Apr 2020 23:54:25 +0200 Subject: tree-wide: make sure our control buffers are properly aligned We always need to make them unions with a "struct cmsghdr" in them, so that things properly aligned. Otherwise we might end up at an unaligned address and the counting goes all wrong, possibly making the kernel refuse our buffers. Also, let's make sure we initialize the control buffers to zero when sending, but leave them uninitialized when reading. Both the alignment and the initialization thing is mentioned in the cmsg(3) man page. --- src/basic/socket-util.c | 10 ++-------- src/basic/socket-util.h | 11 +++++++++++ src/core/manager.c | 7 ++----- src/coredump/coredump.c | 5 +---- src/home/homed-manager.c | 5 +---- src/import/importd.c | 7 ++----- src/journal/journald-server.c | 23 ++++++++--------------- src/journal/journald-stream.c | 6 +++--- src/journal/journald-syslog.c | 5 +---- src/libsystemd-network/icmp6-util.c | 8 +++----- src/libsystemd-network/sd-dhcp-client.c | 6 +++--- src/libsystemd-network/sd-dhcp-server.c | 12 ++++++------ src/libsystemd/sd-bus/bus-socket.c | 10 ++-------- src/libsystemd/sd-device/device-monitor.c | 6 +++--- src/nspawn/nspawn.c | 7 ++----- src/portable/portable.c | 10 ++-------- src/resolve/resolved-dns-stream.c | 7 ++----- src/resolve/resolved-manager.c | 20 ++++++-------------- src/shared/ask-password-api.c | 6 +----- src/timesync/timesyncd-manager.c | 5 +---- src/udev/udev-ctrl.c | 6 +++--- src/udev/udevd.c | 5 +---- 22 files changed, 66 insertions(+), 121 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index a23dd6cbed..0be098df2e 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -818,10 +818,7 @@ ssize_t send_one_fd_iov_sa( const struct sockaddr *sa, socklen_t len, int flags) { - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(int))]; - } control = {}; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int))) control = {}; struct msghdr mh = { .msg_name = (struct sockaddr*) sa, .msg_namelen = len, @@ -875,10 +872,7 @@ ssize_t receive_one_fd_iov( int flags, int *ret_fd) { - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(int))]; - } control = {}; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int))) control; struct msghdr mh = { .msg_control = &control, .msg_controllen = sizeof(control), diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index fba4efef81..6f366c0429 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -166,6 +166,17 @@ struct cmsghdr* cmsg_find(struct msghdr *mh, int level, int type, socklen_t leng (ctype*) (_found ? CMSG_DATA(_found) : NULL); \ }) +/* Resolves to a type that can carry cmsghdr structures. Make sure things are properly aligned, i.e. the type + * itself is placed properly in memory and the size is also aligned to what's appropriate for "cmsghdr" + * structures. */ +#define CMSG_BUFFER_TYPE(size) \ + union { \ + struct cmsghdr cmsghdr; \ + uint8_t buf[size]; \ + uint8_t align_check[(size) >= CMSG_SPACE(0) && \ + (size) == CMSG_ALIGN(size) ? 1 : -1]; \ + } + /* * Certain hardware address types (e.g Infiniband) do not fit into sll_addr * (8 bytes) and run over the structure. This macro returns the correct size that diff --git a/src/core/manager.c b/src/core/manager.c index 7c207bed6e..09dc92d599 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2349,11 +2349,8 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t .iov_base = buf, .iov_len = sizeof(buf)-1, }; - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(struct ucred)) + - CMSG_SPACE(sizeof(int) * NOTIFY_FD_MAX)]; - } control = {}; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) + + CMSG_SPACE(sizeof(int) * NOTIFY_FD_MAX)) control; struct msghdr msghdr = { .msg_iov = &iovec, .msg_iovlen = 1, diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index ff4238c91d..42231dbd6b 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -884,10 +884,7 @@ static int process_socket(int fd) { log_debug("Processing coredump received on stdin..."); for (;;) { - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(int))]; - } control = {}; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int))) control; struct msghdr mh = { .msg_control = &control, .msg_controllen = sizeof(control), diff --git a/src/home/homed-manager.c b/src/home/homed-manager.c index 8ea93120d0..70673fc84d 100644 --- a/src/home/homed-manager.c +++ b/src/home/homed-manager.c @@ -946,10 +946,7 @@ static ssize_t read_datagram(int fd, struct ucred *ret_sender, void **ret) { return -ENOMEM; if (ret_sender) { - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(struct ucred))]; - } control; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred))) control; bool found_ucred = false; struct cmsghdr *cmsg; struct msghdr mh; diff --git a/src/import/importd.c b/src/import/importd.c index 9408edece1..d799a10f94 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -550,11 +550,8 @@ static int manager_on_notify(sd_event_source *s, int fd, uint32_t revents, void .iov_base = buf, .iov_len = sizeof(buf)-1, }; - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(struct ucred)) + - CMSG_SPACE(sizeof(int) * NOTIFY_FD_MAX)]; - } control = {}; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) + + CMSG_SPACE(sizeof(int) * NOTIFY_FD_MAX)) control; struct msghdr msghdr = { .msg_iov = &iovec, .msg_iovlen = 1, diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index 981a569453..9efa65a294 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -1268,21 +1268,14 @@ int server_process_datagram( int *fds = NULL, v = 0; size_t n_fds = 0; - union { - struct cmsghdr cmsghdr; - - /* We use NAME_MAX space for the SELinux label - * here. The kernel currently enforces no - * limit, but according to suggestions from - * the SELinux people this will change and it - * will probably be identical to NAME_MAX. For - * now we use that, but this should be updated - * one day when the final limit is known. */ - uint8_t buf[CMSG_SPACE(sizeof(struct ucred)) + - CMSG_SPACE(sizeof(struct timeval)) + - CMSG_SPACE(sizeof(int)) + /* fd */ - CMSG_SPACE(NAME_MAX)]; /* selinux label */ - } control = {}; + /* We use NAME_MAX space for the SELinux label here. The kernel currently enforces no limit, but + * according to suggestions from the SELinux people this will change and it will probably be + * identical to NAME_MAX. For now we use that, but this should be updated one day when the final + * limit is known. */ + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) + + CMSG_SPACE(sizeof(struct timeval)) + + CMSG_SPACE(sizeof(int)) + /* fd */ + CMSG_SPACE(NAME_MAX) /* selinux label */) control; union sockaddr_union sa = {}; diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c index 202ac3cda2..4dadbd62be 100644 --- a/src/journal/journald-stream.c +++ b/src/journal/journald-stream.c @@ -489,7 +489,7 @@ static int stdout_stream_scan(StdoutStream *s, bool force_flush) { } static int stdout_stream_process(sd_event_source *es, int fd, uint32_t revents, void *userdata) { - uint8_t buf[CMSG_SPACE(sizeof(struct ucred))]; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred))) control; StdoutStream *s = userdata; struct ucred *ucred; struct iovec iovec; @@ -500,8 +500,8 @@ static int stdout_stream_process(sd_event_source *es, int fd, uint32_t revents, struct msghdr msghdr = { .msg_iov = &iovec, .msg_iovlen = 1, - .msg_control = buf, - .msg_controllen = sizeof(buf), + .msg_control = &control, + .msg_controllen = sizeof(control), }; assert(s); diff --git a/src/journal/journald-syslog.c b/src/journal/journald-syslog.c index 1325b56385..46013c3878 100644 --- a/src/journal/journald-syslog.c +++ b/src/journal/journald-syslog.c @@ -39,10 +39,7 @@ static void forward_syslog_iovec( .msg_iovlen = n_iovec, }; struct cmsghdr *cmsg; - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(struct ucred))]; - } control; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred))) control; const char *j; int r; diff --git a/src/libsystemd-network/icmp6-util.c b/src/libsystemd-network/icmp6-util.c index 1a51c22a8d..bdd94867ac 100644 --- a/src/libsystemd-network/icmp6-util.c +++ b/src/libsystemd-network/icmp6-util.c @@ -147,11 +147,9 @@ int icmp6_send_router_solicitation(int s, const struct ether_addr *ether_addr) { int icmp6_receive(int fd, void *buffer, size_t size, struct in6_addr *dst, triple_timestamp *timestamp) { - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(int)) + /* ttl */ - CMSG_SPACE(sizeof(struct timeval))]; - } control = {}; + + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int)) + /* ttl */ + CMSG_SPACE(sizeof(struct timeval))) control; struct iovec iov = {}; union sockaddr_union sa = {}; struct msghdr msg = { diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index 47f60d4a2a..ee0e63e113 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -1899,13 +1899,13 @@ static int client_receive_message_raw( sd_dhcp_client *client = userdata; _cleanup_free_ DHCPPacket *packet = NULL; - uint8_t cmsgbuf[CMSG_SPACE(sizeof(struct tpacket_auxdata))]; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct tpacket_auxdata))) control; struct iovec iov = {}; struct msghdr msg = { .msg_iov = &iov, .msg_iovlen = 1, - .msg_control = cmsgbuf, - .msg_controllen = sizeof(cmsgbuf), + .msg_control = &control, + .msg_controllen = sizeof(control), }; struct cmsghdr *cmsg; bool checksum = true; diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index 59792af686..14f2bcb4fb 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -267,14 +267,14 @@ static int dhcp_server_send_udp(sd_dhcp_server *server, be32_t destination, .iov_base = message, .iov_len = len, }; - uint8_t cmsgbuf[CMSG_SPACE(sizeof(struct in_pktinfo))] = {}; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct in_pktinfo))) control = {}; struct msghdr msg = { .msg_name = &dest, .msg_namelen = sizeof(dest.in), .msg_iov = &iov, .msg_iovlen = 1, - .msg_control = cmsgbuf, - .msg_controllen = sizeof(cmsgbuf), + .msg_control = &control, + .msg_controllen = sizeof(control), }; struct cmsghdr *cmsg; struct in_pktinfo *pktinfo; @@ -970,14 +970,14 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, static int server_receive_message(sd_event_source *s, int fd, uint32_t revents, void *userdata) { _cleanup_free_ DHCPMessage *message = NULL; - uint8_t cmsgbuf[CMSG_SPACE(sizeof(struct in_pktinfo))]; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct in_pktinfo))) control; sd_dhcp_server *server = userdata; struct iovec iov = {}; struct msghdr msg = { .msg_iov = &iov, .msg_iovlen = 1, - .msg_control = cmsgbuf, - .msg_controllen = sizeof(cmsgbuf), + .msg_control = &control, + .msg_controllen = sizeof(control), }; struct cmsghdr *cmsg; ssize_t buflen, len; diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index e336dbb62e..acc313d164 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -518,10 +518,7 @@ static int bus_socket_read_auth(sd_bus *b) { ssize_t k; int r; void *p; - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(int) * BUS_FDS_MAX)]; - } control; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int) * BUS_FDS_MAX)) control; bool handle_cmsg = false; assert(b); @@ -1169,10 +1166,7 @@ int bus_socket_read_message(sd_bus *bus) { size_t need; int r; void *b; - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(int) * BUS_FDS_MAX)]; - } control; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int) * BUS_FDS_MAX)) control; bool handle_cmsg = false; assert(bus); diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c index 42753abe08..20a422c77c 100644 --- a/src/libsystemd/sd-device/device-monitor.c +++ b/src/libsystemd/sd-device/device-monitor.c @@ -361,13 +361,13 @@ int device_monitor_receive_device(sd_device_monitor *m, sd_device **ret) { .iov_base = &buf, .iov_len = sizeof(buf) }; - char cred_msg[CMSG_SPACE(sizeof(struct ucred))]; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred))) control; union sockaddr_union snl; struct msghdr smsg = { .msg_iov = &iov, .msg_iovlen = 1, - .msg_control = cred_msg, - .msg_controllen = sizeof(cred_msg), + .msg_control = &control, + .msg_controllen = sizeof(control), .msg_name = &snl, .msg_namelen = sizeof(snl), }; diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 43c6b6845e..8e8881749a 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3687,11 +3687,8 @@ static int nspawn_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t r .iov_base = buf, .iov_len = sizeof(buf)-1, }; - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(struct ucred)) + - CMSG_SPACE(sizeof(int) * NOTIFY_FD_MAX)]; - } control = {}; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) + + CMSG_SPACE(sizeof(int) * NOTIFY_FD_MAX)) control; struct msghdr msghdr = { .msg_iov = &iovec, .msg_iovlen = 1, diff --git a/src/portable/portable.c b/src/portable/portable.c index 4a881460db..f47a3329fb 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -127,10 +127,7 @@ static int send_item( const char *name, int fd) { - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(int))]; - } control = {}; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int))) control = {}; struct iovec iovec; struct msghdr mh = { .msg_control = &control, @@ -168,10 +165,7 @@ static int recv_item( char **ret_name, int *ret_fd) { - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(int))]; - } control = {}; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int))) control; char buffer[PATH_MAX+2]; struct iovec iov = IOVEC_INIT(buffer, sizeof(buffer)-1); struct msghdr mh = { diff --git a/src/resolve/resolved-dns-stream.c b/src/resolve/resolved-dns-stream.c index 9c47cd60e4..bbaffc165a 100644 --- a/src/resolve/resolved-dns-stream.c +++ b/src/resolve/resolved-dns-stream.c @@ -87,11 +87,8 @@ static int dns_stream_complete(DnsStream *s, int error) { } static int dns_stream_identify(DnsStream *s) { - union { - struct cmsghdr header; /* For alignment */ - uint8_t buffer[CMSG_SPACE(MAXSIZE(struct in_pktinfo, struct in6_pktinfo)) - + EXTRA_CMSG_SPACE /* kernel appears to require extra space */]; - } control; + CMSG_BUFFER_TYPE(CMSG_SPACE(MAXSIZE(struct in_pktinfo, struct in6_pktinfo)) + + EXTRA_CMSG_SPACE /* kernel appears to require extra space */) control; struct msghdr mh = {}; struct cmsghdr *cmsg; socklen_t sl; diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 69630cf49d..1da590b68a 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -741,12 +741,9 @@ Manager *manager_free(Manager *m) { int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) { _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL; - union { - struct cmsghdr header; /* For alignment */ - uint8_t buffer[CMSG_SPACE(MAXSIZE(struct in_pktinfo, struct in6_pktinfo)) - + CMSG_SPACE(int) /* ttl/hoplimit */ - + EXTRA_CMSG_SPACE /* kernel appears to require extra buffer space */]; - } control; + CMSG_BUFFER_TYPE(CMSG_SPACE(MAXSIZE(struct in_pktinfo, struct in6_pktinfo)) + + CMSG_SPACE(int) /* ttl/hoplimit */ + + EXTRA_CMSG_SPACE /* kernel appears to require extra buffer space */) control; union sockaddr_union sa; struct iovec iov; struct msghdr mh = { @@ -930,10 +927,8 @@ static int manager_ipv4_send( uint16_t port, const struct in_addr *source, DnsPacket *p) { - union { - struct cmsghdr header; /* For alignment */ - uint8_t buffer[CMSG_SPACE(sizeof(struct in_pktinfo))]; - } control = {}; + + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct in_pktinfo))) control = {}; union sockaddr_union sa; struct iovec iov; struct msghdr mh = { @@ -988,10 +983,7 @@ static int manager_ipv6_send( const struct in6_addr *source, DnsPacket *p) { - union { - struct cmsghdr header; /* For alignment */ - uint8_t buffer[CMSG_SPACE(sizeof(struct in6_pktinfo))]; - } control = {}; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct in6_pktinfo))) control = {}; union sockaddr_union sa; struct iovec iov; struct msghdr mh = { diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index e690b0fa6e..4c5781a11a 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -859,13 +859,10 @@ int ask_password_agent( pollfd[FD_INOTIFY].events = POLLIN; for (;;) { + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred))) control; char passphrase[LINE_MAX+1]; struct iovec iovec; struct ucred *ucred; - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(struct ucred))]; - } control; ssize_t n; int k; usec_t t; @@ -917,7 +914,6 @@ int ask_password_agent( iovec = IOVEC_MAKE(passphrase, sizeof(passphrase)); - zero(control); struct msghdr msghdr = { .msg_iov = &iovec, .msg_iovlen = 1, diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c index 2980f79b16..dadf213a80 100644 --- a/src/timesync/timesyncd-manager.c +++ b/src/timesync/timesyncd-manager.c @@ -407,10 +407,7 @@ static int manager_receive_response(sd_event_source *source, int fd, uint32_t re .iov_base = &ntpmsg, .iov_len = sizeof(ntpmsg), }; - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(struct timeval))]; - } control; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct timeval))) control; union sockaddr_union server_addr; struct msghdr msghdr = { .msg_iov = &iov, diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c index d067279f3e..1e51f22e26 100644 --- a/src/udev/udev-ctrl.c +++ b/src/udev/udev-ctrl.c @@ -189,12 +189,12 @@ static int udev_ctrl_connection_event_handler(sd_event_source *s, int fd, uint32 _cleanup_(udev_ctrl_disconnect_and_listen_againp) struct udev_ctrl *uctrl = NULL; struct udev_ctrl_msg_wire msg_wire; struct iovec iov = IOVEC_MAKE(&msg_wire, sizeof(struct udev_ctrl_msg_wire)); - char cred_msg[CMSG_SPACE(sizeof(struct ucred))]; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred))) control; struct msghdr smsg = { .msg_iov = &iov, .msg_iovlen = 1, - .msg_control = cred_msg, - .msg_controllen = sizeof(cred_msg), + .msg_control = &control, + .msg_controllen = sizeof(control), }; struct cmsghdr *cmsg; struct ucred *cred; diff --git a/src/udev/udevd.c b/src/udev/udevd.c index cfda47f849..d3f9d532e1 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -906,10 +906,7 @@ static int on_worker(sd_event_source *s, int fd, uint32_t revents, void *userdat .iov_base = &msg, .iov_len = sizeof(msg), }; - union { - struct cmsghdr cmsghdr; - uint8_t buf[CMSG_SPACE(sizeof(struct ucred))]; - } control = {}; + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred))) control; struct msghdr msghdr = { .msg_iov = &iovec, .msg_iovlen = 1, -- cgit v1.2.3