From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 23 Apr 2020 17:47:38 +0000 (+0200)
Subject: netlink: port to recvmsg_safe()
X-Git-Tag: v246-rc1~388
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9e45fb09bfbbf79b340b3303f810834b33cde109;p=thirdparty%2Fsystemd.git

netlink: port to recvmsg_safe()

This also makes sure the control buffer is properly aligned. This
matters, as otherwise the control buffer might not be aligned and the
cmsg buffer counting might be off. The incorrect alignment is becoming
visible by using recvmsg_safe() as we suddenly notice the MSG_CTRUNC bit
set because of this.

That said, apparently this isn't enough to make this work on all
kernels. Since I couldn't figure this out, we now add 1K to the buffer
to be sure. We do this once already, also for a pktinfo structure
(though an IPv4/IPv6) one. I am puzzled by this, but this shouldn't
matter much. it works locally just fine, except for those ubuntu CI
kernels...

While we are at it, make some other changes too, to simplify and
modernize the function.
---

diff --git a/src/libsystemd/sd-netlink/netlink-socket.c b/src/libsystemd/sd-netlink/netlink-socket.c
index 8eff8d99244..71b3d1e2f1f 100644
--- a/src/libsystemd/sd-netlink/netlink-socket.c
+++ b/src/libsystemd/sd-netlink/netlink-socket.c
@@ -16,6 +16,10 @@
 #include "socket-util.h"
 #include "util.h"
 
+/* For some reason we need some extra cmsg space on some kernels. It's not clear why, and one of those days
+ * we need to track this down. See: https://github.com/systemd/systemd/pull/15457 */
+#define EXTRA_CMSG_SPACE 1024
+
 int socket_open(int family) {
         int fd;
 
@@ -240,30 +244,27 @@ int socket_write_message(sd_netlink *nl, sd_netlink_message *m) {
 
 static int socket_recv_message(int fd, struct iovec *iov, uint32_t *ret_mcast_group, bool peek) {
         union sockaddr_union sender;
-        uint8_t cmsg_buffer[CMSG_SPACE(sizeof(struct nl_pktinfo))];
+        CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct nl_pktinfo)) + EXTRA_CMSG_SPACE) control;
         struct msghdr msg = {
                 .msg_iov = iov,
                 .msg_iovlen = 1,
                 .msg_name = &sender,
                 .msg_namelen = sizeof(sender),
-                .msg_control = cmsg_buffer,
-                .msg_controllen = sizeof(cmsg_buffer),
+                .msg_control = &control,
+                .msg_controllen = sizeof(control),
         };
         ssize_t n;
 
         assert(fd >= 0);
         assert(iov);
 
-        n = recvmsg(fd, &msg, MSG_TRUNC | (peek ? MSG_PEEK : 0));
-        if (n < 0) {
-                /* no data */
-                if (errno == ENOBUFS)
-                        log_debug("rtnl: kernel receive buffer overrun");
-                else if (errno == EAGAIN)
-                        log_debug("rtnl: no data in socket");
-
-                return IN_SET(errno, EAGAIN, EINTR) ? 0 : -errno;
-        }
+        n = recvmsg_safe(fd, &msg, MSG_TRUNC | (peek ? MSG_PEEK : 0));
+        if (n == -ENOBUFS)
+                return log_debug_errno(n, "rtnl: kernel receive buffer overrun");
+        if (IN_SET(n, -EAGAIN, -EINTR))
+                return 0;
+        if (n < 0)
+                return (int) n;
 
         if (sender.nl.nl_pid != 0) {
                 /* not from the kernel, ignore */
@@ -271,20 +272,20 @@ static int socket_recv_message(int fd, struct iovec *iov, uint32_t *ret_mcast_gr
 
                 if (peek) {
                         /* drop the message */
-                        n = recvmsg(fd, &msg, 0);
+                        n = recvmsg_safe(fd, &msg, 0);
                         if (n < 0)
-                                return IN_SET(errno, EAGAIN, EINTR) ? 0 : -errno;
+                                return (int) n;
                 }
 
                 return 0;
         }
 
         if (ret_mcast_group) {
-                struct cmsghdr *cmsg;
+                struct nl_pktinfo *pi;
 
-                cmsg = cmsg_find(&msg, SOL_NETLINK, NETLINK_PKTINFO, CMSG_LEN(sizeof(struct nl_pktinfo)));
-                if (cmsg)
-                        *ret_mcast_group = ((struct nl_pktinfo*) CMSG_DATA(cmsg))->group;
+                pi = CMSG_FIND_DATA(&msg, SOL_NETLINK, NETLINK_PKTINFO, struct nl_pktinfo);
+                if (pi)
+                        *ret_mcast_group = pi->group;
                 else
                         *ret_mcast_group = 0;
         }
diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h
index 8e029b86d90..cbad1dce605 100644
--- a/src/resolve/resolved-manager.h
+++ b/src/resolve/resolved-manager.h
@@ -165,6 +165,7 @@ void manager_verify_all(Manager *m);
 
 DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
 
+/* For some reason we need some extra cmsg space on some kernels/archs. One of those days we ned to figure out why */
 #define EXTRA_CMSG_SPACE 1024
 
 int manager_is_own_hostname(Manager *m, const char *name);