tree-wide: use recvmsg_safe() at various places

Let's be extra careful whenever we return from recvmsg() and see
MSG_CTRUNC set. This generally means we ran into a programming error, as
we didn't size the control buffer large enough. It's an error condition
we should at least log about, or propagate up. Hence do that.

This is particularly important when receiving fds, since for those the
control data can be of any size. In particular on stream sockets that's
nasty, because if we miss an fd because of control data truncation we
cannot recover, we might not even realize that we are one off.

(Also, when failing early, if there's any chance the socket might be
AF_UNIX let's close all received fds, all the time. We got this right
most of the time, but there were a few cases missing. God, UNIX is hard
to use)

diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c
index 2d0564e66f04623cd06838e6ad4ac4ded640ae11..b797a52180f959d3bc34ef6235314f54f7502d97 100644 (file)
--- a/src/basic/socket-util.c
+++ b/src/basic/socket-util.c
@@ -901,9 +901,9 @@ ssize_t receive_one_fd_iov(
          * combination with send_one_fd().
          */
 
-        k = recvmsg(transport_fd, &mh, MSG_CMSG_CLOEXEC | flags);
+        k = recvmsg_safe(transport_fd, &mh, MSG_CMSG_CLOEXEC | flags);
         if (k < 0)
-                return (ssize_t) -errno;
+                return k;
 
         CMSG_FOREACH(cmsg, &mh) {
                 if (cmsg->cmsg_level == SOL_SOCKET &&
@@ -915,12 +915,13 @@ ssize_t receive_one_fd_iov(
                 }
         }
 
-        if (!found)
+        if (!found) {
                 cmsg_close_all(&mh);
 
-        /* If didn't receive an FD or any data, return an error. */
-        if (k == 0 && !found)
-                return -EIO;
+                /* If didn't receive an FD or any data, return an error. */
+                if (k == 0)
+                        return -EIO;
+        }
 
         if (found)
                 *ret_fd = *(int*) CMSG_DATA(found);

diff --git a/src/core/manager.c b/src/core/manager.c
index 7536a9ca84de5f186bd18195aa1ce720bf6c2983..43b8a02e488ab79f7a3a10b9b98bc9130f6b7634 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -2360,20 +2360,20 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
                 return 0;
         }
 
-        n = recvmsg(m->notify_fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC|MSG_TRUNC);
-        if (n < 0) {
-                if (IN_SET(errno, EAGAIN, EINTR))
-                        return 0; /* Spurious wakeup, try again */
-
-                /* If this is any other, real error, then let's stop processing this socket. This of course means we
-                 * won't take notification messages anymore, but that's still better than busy looping around this:
-                 * being woken up over and over again but being unable to actually read the message off the socket. */
-                return log_error_errno(errno, "Failed to receive notification message: %m");
-        }
+        n = recvmsg_safe(m->notify_fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC|MSG_TRUNC);
+        if (IN_SET(n, -EAGAIN, -EINTR))
+                return 0; /* Spurious wakeup, try again */
+        if (n < 0)
+                /* If this is any other, real error, then let's stop processing this socket. This of course
+                 * means we won't take notification messages anymore, but that's still better than busy
+                 * looping around this: being woken up over and over again but being unable to actually read
+                 * the message off the socket. */
+                return log_error_errno(n, "Failed to receive notification message: %m");
 
         CMSG_FOREACH(cmsg, &msghdr) {
                 if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) {
 
+                        assert(!fd_array);
                         fd_array = (int*) CMSG_DATA(cmsg);
                         n_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int);
 
@@ -2381,6 +2381,7 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
                            cmsg->cmsg_type == SCM_CREDENTIALS &&
                            cmsg->cmsg_len == CMSG_LEN(sizeof(struct ucred))) {
 
+                        assert(!ucred);
                         ucred = (struct ucred*) CMSG_DATA(cmsg);
                 }
         }

diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c
index ee4268b96572a423b6f7a02f36132da70609388d..e4266baa62cc67856d2d35ecade4e5e9ab0fa1aa 100644 (file)
--- a/src/coredump/coredump.c
+++ b/src/coredump/coredump.c
@@ -911,10 +911,10 @@ static int process_socket(int fd) {
 
                 mh.msg_iov = &iovec;
 
-                n = recvmsg(fd, &mh, MSG_CMSG_CLOEXEC);
+                n = recvmsg_safe(fd, &mh, MSG_CMSG_CLOEXEC);
                 if (n < 0)  {
                         free(iovec.iov_base);
-                        r = log_error_errno(errno, "Failed to receive datagram: %m");
+                        r = log_error_errno(n, "Failed to receive datagram: %m");
                         goto finish;
                 }
 
@@ -935,15 +935,17 @@ static int process_socket(int fd) {
                         }
 
                         if (!found) {
-                                log_error("Coredump file descriptor missing.");
-                                r = -EBADMSG;
+                                cmsg_close_all(&mh);
+                                r = log_error_errno(SYNTHETIC_ERRNO(EBADMSG),
+                                                    "Coredump file descriptor missing.");
                                 goto finish;
                         }
 
                         assert(input_fd < 0);
                         input_fd = *(int*) CMSG_DATA(found);
                         break;
-                }
+                } else
+                        cmsg_close_all(&mh);
 
                 /* Add trailing NUL byte, in case these are strings */
                 ((char*) iovec.iov_base)[n] = 0;
@@ -952,8 +954,6 @@ static int process_socket(int fd) {
                 r = iovw_put(&iovw, iovec.iov_base, iovec.iov_len);
                 if (r < 0)
                         goto finish;
-
-                cmsg_close_all(&mh);
         }
 
         /* Make sure we got all data we really need */

diff --git a/src/home/homed-manager.c b/src/home/homed-manager.c
index ed2a54615c66c5fdab99410f10bf6ab1899eb66b..5ce7be4fac02b202c46fd8a3b76bb68281a0c4b5 100644 (file)
--- a/src/home/homed-manager.c
+++ b/src/home/homed-manager.c
@@ -981,9 +981,9 @@ static ssize_t read_datagram(int fd, struct ucred *ret_sender, void **ret) {
                         .msg_controllen = sizeof(control),
                 };
 
-                m = recvmsg(fd, &mh, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
+                m = recvmsg_safe(fd, &mh, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
                 if (m < 0)
-                        return -errno;
+                        return m;
 
                 cmsg_close_all(&mh);
 

diff --git a/src/import/importd.c b/src/import/importd.c
index 8977ebd8352e2c54d6e78cf99ba127b36969d6e3..340b12ecd5c9044d2c6c776529ce8aa4b7241179 100644 (file)
--- a/src/import/importd.c
+++ b/src/import/importd.c
@@ -566,13 +566,11 @@ static int manager_on_notify(sd_event_source *s, int fd, uint32_t revents, void
         ssize_t n;
         int r;
 
-        n = recvmsg(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
-        if (n < 0) {
-                if (IN_SET(errno, EAGAIN, EINTR))
-                        return 0;
-
-                return -errno;
-        }
+        n = recvmsg_safe(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
+        if (IN_SET(n, -EAGAIN, -EINTR))
+                return 0;
+        if (n < 0)
+                return (int) n;
 
         cmsg_close_all(&msghdr);
 

diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index 64cb3279f64f6efcdbfc79a76eb277b24b22b79a..9616e161ccb60f24de4f3567d2a081739c9ce741 100644 (file)
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -1317,29 +1317,35 @@ int server_process_datagram(
 
         iovec = IOVEC_MAKE(s->buffer, s->buffer_size - 1); /* Leave room for trailing NUL we add later */
 
-        n = recvmsg(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
-        if (n < 0) {
-                if (IN_SET(errno, EINTR, EAGAIN))
-                        return 0;
-
-                return log_error_errno(errno, "recvmsg() failed: %m");
+        n = recvmsg_safe(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
+        if (IN_SET(n, -EINTR, -EAGAIN))
+                return 0;
+        if (n == -EXFULL) {
+                log_warning("Got message with truncated control data (too many fds sent?), ignoring.");
+                return 0;
         }
+        if (n < 0)
+                return log_error_errno(n, "recvmsg() failed: %m");
 
         CMSG_FOREACH(cmsg, &msghdr)
                 if (cmsg->cmsg_level == SOL_SOCKET &&
                     cmsg->cmsg_type == SCM_CREDENTIALS &&
-                    cmsg->cmsg_len == CMSG_LEN(sizeof(struct ucred)))
+                    cmsg->cmsg_len == CMSG_LEN(sizeof(struct ucred))) {
+                        assert(!ucred);
                         ucred = (struct ucred*) CMSG_DATA(cmsg);
-                else if (cmsg->cmsg_level == SOL_SOCKET &&
+                } else if (cmsg->cmsg_level == SOL_SOCKET &&
                          cmsg->cmsg_type == SCM_SECURITY) {
+                        assert(!label);
                         label = (char*) CMSG_DATA(cmsg);
                         label_len = cmsg->cmsg_len - CMSG_LEN(0);
                 } else if (cmsg->cmsg_level == SOL_SOCKET &&
                            cmsg->cmsg_type == SO_TIMESTAMP &&
-                           cmsg->cmsg_len == CMSG_LEN(sizeof(struct timeval)))
+                           cmsg->cmsg_len == CMSG_LEN(sizeof(struct timeval))) {
+                        assert(!tv);
                         tv = (struct timeval*) CMSG_DATA(cmsg);
-                else if (cmsg->cmsg_level == SOL_SOCKET &&
+                } else if (cmsg->cmsg_level == SOL_SOCKET &&
                          cmsg->cmsg_type == SCM_RIGHTS) {
+                        assert(!fds);
                         fds = (int*) CMSG_DATA(cmsg);
                         n_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int);
                 }

diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c
index 609af506a429d12ebca2e91bab88703485a940c7..ec6dad62e83942d592c53c6e572b35cc3478007d 100644 (file)
--- a/src/journal/journald-stream.c
+++ b/src/journal/journald-stream.c
@@ -545,6 +545,7 @@ static int stdout_stream_process(sd_event_source *es, int fd, uint32_t revents,
                 if (cmsg->cmsg_level == SOL_SOCKET &&
                     cmsg->cmsg_type == SCM_CREDENTIALS &&
                     cmsg->cmsg_len == CMSG_LEN(sizeof(struct ucred))) {
+                        assert(!ucred);
                         ucred = (struct ucred *)CMSG_DATA(cmsg);
                         break;
                 }

diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c
index 18d30d010a20439340e5315f90aca88cbbbf9477..f54a5d19763d195ebb8241da6422fd4229c3c3ac 100644 (file)
--- a/src/libsystemd/sd-bus/bus-socket.c
+++ b/src/libsystemd/sd-bus/bus-socket.c
@@ -557,17 +557,24 @@ static int bus_socket_read_auth(sd_bus *b) {
                 mh.msg_control = &control;
                 mh.msg_controllen = sizeof(control);
 
-                k = recvmsg(b->input_fd, &mh, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
-                if (k < 0 && errno == ENOTSOCK) {
+                k = recvmsg_safe(b->input_fd, &mh, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
+                if (k == -ENOTSOCK) {
                         b->prefer_readv = true;
                         k = readv(b->input_fd, &iov, 1);
+                        if (k < 0)
+                                k = -errno;
                 } else
                         handle_cmsg = true;
         }
+        if (k == -EAGAIN)
+                return 0;
         if (k < 0)
-                return errno == EAGAIN ? 0 : -errno;
-        if (k == 0)
+                return (int) k;
+        if (k == 0) {
+                if (handle_cmsg)
+                        cmsg_close_all(&mh); /* paranoia, we shouldn't have gotten any fds on EOF */
                 return -ECONNRESET;
+        }
 
         b->rbuffer_size += k;
 
@@ -1193,17 +1200,24 @@ int bus_socket_read_message(sd_bus *bus) {
                 mh.msg_control = &control;
                 mh.msg_controllen = sizeof(control);
 
-                k = recvmsg(bus->input_fd, &mh, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
-                if (k < 0 && errno == ENOTSOCK) {
+                k = recvmsg_safe(bus->input_fd, &mh, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
+                if (k == -ENOTSOCK) {
                         bus->prefer_readv = true;
                         k = readv(bus->input_fd, &iov, 1);
+                        if (k < 0)
+                                k = -errno;
                 } else
                         handle_cmsg = true;
         }
+        if (k == -EAGAIN)
+                return 0;
         if (k < 0)
-                return errno == EAGAIN ? 0 : -errno;
-        if (k == 0)
+                return (int) k;
+        if (k == 0) {
+                if (handle_cmsg)
+                        cmsg_close_all(&mh); /* On EOF we shouldn't have gotten an fd, but let's make sure */
                 return -ECONNRESET;
+        }
 
         bus->rbuffer_size += k;
 

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 9888c9e294bd2ee00d642e420b42b8f647b0c226..b0d2edac95a67e45c1f51a0ff70a1764a0614ad3 100644 (file)
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -3716,13 +3716,12 @@ static int nspawn_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t r
                 return 0;
         }
 
-        n = recvmsg(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
-        if (n < 0) {
-                if (IN_SET(errno, EAGAIN, EINTR))
-                        return 0;
+        n = recvmsg_safe(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
+        if (IN_SET(n, -EAGAIN, -EINTR))
+                return 0;
+        if (n < 0)
+                return log_warning_errno(n, "Couldn't read notification socket: %m");
 
-                return log_warning_errno(errno, "Couldn't read notification socket: %m");
-        }
         cmsg_close_all(&msghdr);
 
         CMSG_FOREACH(cmsg, &msghdr) {

diff --git a/src/portable/portable.c b/src/portable/portable.c
index 58e4a6670d8b483df0517a50268564a638436f75..49087179c87d69e0f52e317382306533113603e3 100644 (file)
--- a/src/portable/portable.c
+++ b/src/portable/portable.c
@@ -190,9 +190,9 @@ static int recv_item(
         assert(ret_name);
         assert(ret_fd);
 
-        n = recvmsg(socket_fd, &mh, MSG_CMSG_CLOEXEC);
+        n = recvmsg_safe(socket_fd, &mh, MSG_CMSG_CLOEXEC);
         if (n < 0)
-                return -errno;
+                return (int) n;
 
         CMSG_FOREACH(cmsg, &mh) {
                 if (cmsg->cmsg_level == SOL_SOCKET &&

diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index 94590e3038673ef03a798edd5fbff7c9a625451f..5055800b046dbc76caea18669baa55ee3357c769 100644 (file)
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -775,17 +775,14 @@ int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) {
 
         iov = IOVEC_MAKE(DNS_PACKET_DATA(p), p->allocated);
 
-        l = recvmsg(fd, &mh, 0);
-        if (l < 0) {
-                if (IN_SET(errno, EAGAIN, EINTR))
-                        return 0;
-
-                return -errno;
-        }
+        l = recvmsg_safe(fd, &mh, 0);
+        if (IN_SET(l, -EAGAIN, -EINTR))
+                return 0;
+        if (l < 0)
+                return l;
         if (l == 0)
                 return 0;
 
-        assert(!(mh.msg_flags & MSG_CTRUNC));
         assert(!(mh.msg_flags & MSG_TRUNC));
 
         p->size = (size_t) l;

diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c
index 9b74d909b1c62a21150b0bd4f1e8832185d3114a..b7b74260584a71e53ec17ce57b047ced130465e4 100644 (file)
--- a/src/shared/ask-password-api.c
+++ b/src/shared/ask-password-api.c
@@ -925,12 +925,11 @@ int ask_password_agent(
                 msghdr.msg_control = &control;
                 msghdr.msg_controllen = sizeof(control);
 
-                n = recvmsg(socket_fd, &msghdr, 0);
+                n = recvmsg_safe(socket_fd, &msghdr, 0);
+                if (IN_SET(n, -EAGAIN, -EINTR))
+                        continue;
                 if (n < 0) {
-                        if (IN_SET(errno, EAGAIN, EINTR))
-                                continue;
-
-                        r = -errno;
+                        r = (int) n;
                         goto finish;
                 }
 

diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c
index e18e1e6c04853597c03dc97cb73ce266ccd543f4..2980f79b16f8388b91c5ad22ffb59a01f076c161 100644 (file)
--- a/src/timesync/timesyncd-manager.c
+++ b/src/timesync/timesyncd-manager.c
@@ -438,12 +438,11 @@ static int manager_receive_response(sd_event_source *source, int fd, uint32_t re
                 return manager_connect(m);
         }
 
-        len = recvmsg(fd, &msghdr, MSG_DONTWAIT);
+        len = recvmsg_safe(fd, &msghdr, MSG_DONTWAIT);
+        if (len == -EAGAIN)
+                return 0;
         if (len < 0) {
-                if (errno == EAGAIN)
-                        return 0;
-
-                log_warning("Error receiving message. Disconnecting.");
+                log_warning_errno(len, "Error receiving message, disconnecting: %m");
                 return manager_connect(m);
         }
 

diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c
index b8c0d83a022c1729211d0c96fe5231d08502d746..d067279f3e1aea02c555a5a41a05af9607d18fcf 100644 (file)
--- a/src/udev/udev-ctrl.c
+++ b/src/udev/udev-ctrl.c
@@ -212,13 +212,11 @@ static int udev_ctrl_connection_event_handler(sd_event_source *s, int fd, uint32
         if (size == 0)
                 return 0; /* Client disconnects? */
 
-        size = recvmsg(fd, &smsg, 0);
-        if (size < 0) {
-                if (errno != EINTR)
-                        return log_error_errno(errno, "Failed to receive ctrl message: %m");
-
+        size = recvmsg_safe(fd, &smsg, 0);
+        if (size == -EINTR)
                 return 0;
-        }
+        if (size < 0)
+                return log_error_errno(size, "Failed to receive ctrl message: %m");
 
         cmsg_close_all(&smsg);
 

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 456bc8c479a39e0bd268e849748f1ef2c433b5be..4b15159c5b6e8f2511adc9564d1fc1f1cadb1103 100644 (file)
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -921,16 +921,18 @@ static int on_worker(sd_event_source *s, int fd, uint32_t revents, void *userdat
                 struct ucred *ucred = NULL;
                 struct worker *worker;
 
-                size = recvmsg(fd, &msghdr, MSG_DONTWAIT);
-                if (size < 0) {
-                        if (errno == EINTR)
-                                continue;
-                        else if (errno == EAGAIN)
-                                /* nothing more to read */
-                                break;
+                size = recvmsg_safe(fd, &msghdr, MSG_DONTWAIT);
+                if (size == -EINTR)
+                        continue;
+                if (size == -EAGAIN)
+                        /* nothing more to read */
+                        break;
+                if (size < 0)
+                        return log_error_errno(size, "Failed to receive message: %m");
+
+                cmsg_close_all(&msghdr);
 
-                        return log_error_errno(errno, "Failed to receive message: %m");
-                } else if (size != sizeof(struct worker_message)) {
+                if (size != sizeof(struct worker_message)) {
                         log_warning("Ignoring worker message with invalid size %zi bytes", size);
                         continue;
                 }