Merge pull request #1331 from dvdhrm/misc-cleanup

util, nspawn, machined: random cleanups

5 files changed, 96 insertions, 73 deletions

diff --git a/src/basic/util.c b/src/basic/util.c
index e3b2af8e02..40d9e34f85 100644
--- a/src/basic/util.c
+++ b/src/basic/util.c
@@ -6775,3 +6775,72 @@ int fgetxattr_malloc(int fd, const char *name, char **value) {
                         return -errno;
         }
 }
+
+int send_one_fd(int transport_fd, int fd) {
+        union {
+                struct cmsghdr cmsghdr;
+                uint8_t buf[CMSG_SPACE(sizeof(int))];
+        } control = {};
+        struct msghdr mh = {
+                .msg_control = &control,
+                .msg_controllen = sizeof(control),
+        };
+        struct cmsghdr *cmsg;
+        ssize_t k;
+
+        assert(transport_fd >= 0);
+        assert(fd >= 0);
+
+        cmsg = CMSG_FIRSTHDR(&mh);
+        cmsg->cmsg_level = SOL_SOCKET;
+        cmsg->cmsg_type = SCM_RIGHTS;
+        cmsg->cmsg_len = CMSG_LEN(sizeof(int));
+        memcpy(CMSG_DATA(cmsg), &fd, sizeof(int));
+
+        mh.msg_controllen = CMSG_SPACE(sizeof(int));
+        k = sendmsg(transport_fd, &mh, MSG_NOSIGNAL);
+        if (k < 0)
+                return -errno;
+
+        return 0;
+}
+
+int receive_one_fd(int transport_fd) {
+        union {
+                struct cmsghdr cmsghdr;
+                uint8_t buf[CMSG_SPACE(sizeof(int))];
+        } control = {};
+        struct msghdr mh = {
+                .msg_control = &control,
+                .msg_controllen = sizeof(control),
+        };
+        struct cmsghdr *cmsg;
+        ssize_t k;
+
+        assert(transport_fd >= 0);
+
+        /*
+         * Receive a single FD via @transport_fd. We don't care for the
+         * transport-type, but the caller must assure that no other CMSG types
+         * than SCM_RIGHTS is enabled. We also retrieve a single FD at most, so
+         * for packet-based transports, the caller must ensure to send only a
+         * single FD per packet.
+         * This is best used in combination with send_one_fd().
+         */
+
+        k = recvmsg(transport_fd, &mh, MSG_NOSIGNAL | MSG_CMSG_CLOEXEC);
+        if (k < 0)
+                return -errno;
+
+        cmsg = CMSG_FIRSTHDR(&mh);
+        if (!cmsg || CMSG_NXTHDR(&mh, cmsg) ||
+            cmsg->cmsg_level != SOL_SOCKET ||
+            cmsg->cmsg_type != SCM_RIGHTS ||
+            cmsg->cmsg_len != CMSG_LEN(sizeof(int)) ||
+            *(const int *)CMSG_DATA(cmsg) < 0) {
+                cmsg_close_all(&mh);
+                return -EIO;
+        }
+
+        return *(const int *)CMSG_DATA(cmsg);
+}
diff --git a/src/basic/util.h b/src/basic/util.h
index 8abaa740b2..905f375263 100644
--- a/src/basic/util.h
+++ b/src/basic/util.h
@@ -938,3 +938,6 @@ int reset_uid_gid(void);
 
 int getxattr_malloc(const char *path, const char *name, char **value, bool allow_symlink);
 int fgetxattr_malloc(int fd, const char *name, char **value);
+
+int send_one_fd(int transport_fd, int fd);
+int receive_one_fd(int transport_fd);
diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c
index 6aaaa8aa31..b010c90989 100644
--- a/src/machine/machine-dbus.c
+++ b/src/machine/machine-dbus.c
@@ -353,9 +353,9 @@ int bus_machine_method_get_addresses(sd_bus_message *message, void *userdata, sd
 
                 r = wait_for_terminate(child, &si);
                 if (r < 0)
-                        return sd_bus_error_set_errnof(error, r, "Failed to wait for client: %m");
+                        return sd_bus_error_set_errnof(error, r, "Failed to wait for child: %m");
                 if (si.si_code != CLD_EXITED || si.si_status != EXIT_SUCCESS)
-                        return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Client died abnormally.");
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Child died abnormally.");
                 break;
         }
 
@@ -444,9 +444,9 @@ int bus_machine_method_get_os_release(sd_bus_message *message, void *userdata, s
 
                 r = wait_for_terminate(child, &si);
                 if (r < 0)
-                        return sd_bus_error_set_errnof(error, r, "Failed to wait for client: %m");
+                        return sd_bus_error_set_errnof(error, r, "Failed to wait for child: %m");
                 if (si.si_code != CLD_EXITED || si.si_status != EXIT_SUCCESS)
-                        return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Client died abnormally.");
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Child died abnormally.");
 
                 break;
         }
@@ -1040,11 +1040,11 @@ int bus_machine_method_bind_mount(sd_bus_message *message, void *userdata, sd_bu
 
         r = wait_for_terminate(child, &si);
         if (r < 0) {
-                r = sd_bus_error_set_errnof(error, r, "Failed to wait for client: %m");
+                r = sd_bus_error_set_errnof(error, r, "Failed to wait for child: %m");
                 goto finish;
         }
         if (si.si_code != CLD_EXITED) {
-                r = sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Client died abnormally.");
+                r = sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Child died abnormally.");
                 goto finish;
         }
         if (si.si_status != EXIT_SUCCESS) {
@@ -1052,7 +1052,7 @@ int bus_machine_method_bind_mount(sd_bus_message *message, void *userdata, sd_bu
                 if (read(errno_pipe_fd[0], &r, sizeof(r)) == sizeof(r))
                         r = sd_bus_error_set_errnof(error, r, "Failed to mount: %m");
                 else
-                        r = sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Client failed.");
+                        r = sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Child failed.");
                 goto finish;
         }
 
@@ -1088,7 +1088,7 @@ static int machine_operation_done(sd_event_source *s, const siginfo_t *si, void
         o->pid = 0;
 
         if (si->si_code != CLD_EXITED) {
-                r = sd_bus_error_setf(&error, SD_BUS_ERROR_FAILED, "Client died abnormally.");
+                r = sd_bus_error_setf(&error, SD_BUS_ERROR_FAILED, "Child died abnormally.");
                 goto fail;
         }
 
@@ -1096,7 +1096,7 @@ static int machine_operation_done(sd_event_source *s, const siginfo_t *si, void
                 if (read(o->errno_fd, &r, sizeof(r)) == sizeof(r))
                         r = sd_bus_error_set_errnof(&error, r, "%m");
                 else
-                        r = sd_bus_error_setf(&error, SD_BUS_ERROR_FAILED, "Client failed.");
+                        r = sd_bus_error_setf(&error, SD_BUS_ERROR_FAILED, "Child failed.");
 
                 goto fail;
         }
diff --git a/src/nspawn/nspawn-expose-ports.c b/src/nspawn/nspawn-expose-ports.c
index 38250b6e02..9e63d88b69 100644
--- a/src/nspawn/nspawn-expose-ports.c
+++ b/src/nspawn/nspawn-expose-ports.c
@@ -183,17 +183,8 @@ int expose_port_execute(sd_netlink *rtnl, ExposePort *l, union in_addr_union *ex
 }
 
 int expose_port_send_rtnl(int send_fd) {
-        union {
-                struct cmsghdr cmsghdr;
-                uint8_t buf[CMSG_SPACE(sizeof(int))];
-        } control = {};
-        struct msghdr mh = {
-                .msg_control = &control,
-                .msg_controllen = sizeof(control),
-        };
-        struct cmsghdr *cmsg;
         _cleanup_close_ int fd = -1;
-        ssize_t k;
+        int r;
 
         assert(send_fd >= 0);
 
@@ -201,19 +192,11 @@ int expose_port_send_rtnl(int send_fd) {
         if (fd < 0)
                 return log_error_errno(errno, "Failed to allocate container netlink: %m");
 
-        cmsg = CMSG_FIRSTHDR(&mh);
-        cmsg->cmsg_level = SOL_SOCKET;
-        cmsg->cmsg_type = SCM_RIGHTS;
-        cmsg->cmsg_len = CMSG_LEN(sizeof(int));
-        memcpy(CMSG_DATA(cmsg), &fd, sizeof(int));
-
-        mh.msg_controllen = cmsg->cmsg_len;
-
         /* Store away the fd in the socket, so that it stays open as
          * long as we run the child */
-        k = sendmsg(send_fd, &mh, MSG_NOSIGNAL);
-        if (k < 0)
-                return log_error_errno(errno, "Failed to send netlink fd: %m");
+        r = send_one_fd(send_fd, fd);
+        if (r < 0)
+                return log_error_errno(r, "Failed to send netlink fd: %m");
 
         return 0;
 }
@@ -224,33 +207,16 @@ int expose_port_watch_rtnl(
                 sd_netlink_message_handler_t handler,
                 union in_addr_union *exposed,
                 sd_netlink **ret) {
-
-        union {
-                struct cmsghdr cmsghdr;
-                uint8_t buf[CMSG_SPACE(sizeof(int))];
-        } control = {};
-        struct msghdr mh = {
-                .msg_control = &control,
-                .msg_controllen = sizeof(control),
-        };
-        struct cmsghdr *cmsg;
         _cleanup_netlink_unref_ sd_netlink *rtnl = NULL;
         int fd, r;
-        ssize_t k;
 
         assert(event);
         assert(recv_fd >= 0);
         assert(ret);
 
-        k = recvmsg(recv_fd, &mh, MSG_NOSIGNAL);
-        if (k < 0)
-                return log_error_errno(errno, "Failed to recv netlink fd: %m");
-
-        cmsg = CMSG_FIRSTHDR(&mh);
-        assert(cmsg->cmsg_level == SOL_SOCKET);
-        assert(cmsg->cmsg_type == SCM_RIGHTS);
-        assert(cmsg->cmsg_len == CMSG_LEN(sizeof(int)));
-        memcpy(&fd, CMSG_DATA(cmsg), sizeof(int));
+        fd = receive_one_fd(recv_fd);
+        if (fd < 0)
+                return log_error_errno(fd, "Failed to recv netlink fd: %m");
 
         r = sd_netlink_open_fd(&rtnl, fd);
         if (r < 0) {
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 5702df8ab4..7451c2bf64 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1264,16 +1264,7 @@ static int setup_dev_console(const char *dest, const char *console) {
 static int setup_kmsg(const char *dest, int kmsg_socket) {
         const char *from, *to;
         _cleanup_umask_ mode_t u;
-        int fd, k;
-        union {
-                struct cmsghdr cmsghdr;
-                uint8_t buf[CMSG_SPACE(sizeof(int))];
-        } control = {};
-        struct msghdr mh = {
-                .msg_control = &control,
-                .msg_controllen = sizeof(control),
-        };
-        struct cmsghdr *cmsg;
+        int fd, r;
 
         assert(kmsg_socket >= 0);
 
@@ -1298,21 +1289,13 @@ static int setup_kmsg(const char *dest, int kmsg_socket) {
         if (fd < 0)
                 return log_error_errno(errno, "Failed to open fifo: %m");
 
-        cmsg = CMSG_FIRSTHDR(&mh);
-        cmsg->cmsg_level = SOL_SOCKET;
-        cmsg->cmsg_type = SCM_RIGHTS;
-        cmsg->cmsg_len = CMSG_LEN(sizeof(int));
-        memcpy(CMSG_DATA(cmsg), &fd, sizeof(int));
-
-        mh.msg_controllen = cmsg->cmsg_len;
-
         /* Store away the fd in the socket, so that it stays open as
          * long as we run the child */
-        k = sendmsg(kmsg_socket, &mh, MSG_NOSIGNAL);
+        r = send_one_fd(kmsg_socket, fd);
         safe_close(fd);
 
-        if (k < 0)
-                return log_error_errno(errno, "Failed to send FIFO fd: %m");
+        if (r < 0)
+                return log_error_errno(r, "Failed to send FIFO fd: %m");
 
         /* And now make the FIFO unavailable as /run/kmsg... */
         (void) unlink(from);
@@ -2804,6 +2787,8 @@ static int outer_child(
         }
 
         pid_socket = safe_close(pid_socket);
+        kmsg_socket = safe_close(kmsg_socket);
+        rtnl_socket = safe_close(rtnl_socket);
 
         return 0;
 }
@@ -3489,8 +3474,8 @@ int main(int argc, char *argv[]) {
                 }
 
                 /* Let the child know that we are ready and wait that the child is completely ready now. */
-                if (!barrier_place_and_sync(&barrier)) { /* #5 */
-                        log_error("Client died too early.");
+                if (!barrier_place_and_sync(&barrier)) { /* #4 */
+                        log_error("Child died too early.");
                         r = -ESRCH;
                         goto finish;
                 }