ubusd/libubus-io: fix socket descriptor passing

In commit 5d7ca8309d0a ("ubusd/libubus-io: fix variable sized struct
position warning") the position of cmsghdr struct has been changed in
order to fix clang-9 compiler warning, but it has introduced regression
in at least `logread` which hanged indefinitely.

So this patch reworks the socket descriptor passing in a way recommended
in the `cmsg(3)` manual page.

Ref: http://lists.infradead.org/pipermail/openwrt-devel/2019-December/020840.html
Fixes: 5d7ca8309d0a ("ubusd/libubus-io: fix variable sized struct position warning")
Reported-by: Hannu Nyman <hannu.nyman@welho.com>
Signed-off-by: Petr Štetiar <ynezz@true.cz>

diff --git a/libubus-io.c b/libubus-io.c
index ba1016d0fa09b5cd7081209545f6a34e7104c853..3561ac462eb960fbbd828d9100d0656f101131cb 100644 (file)
--- a/libubus-io.c
+++ b/libubus-io.c
@@ -59,23 +59,24 @@ static void wait_data(int fd, bool write)
 
 static int writev_retry(int fd, struct iovec *iov, int iov_len, int sock_fd)
 {
-       static struct {
-               int fd;
-               struct cmsghdr h;
-       } fd_buf = {
-               .h = {
-                       .cmsg_len = sizeof(fd_buf),
-                       .cmsg_level = SOL_SOCKET,
-                       .cmsg_type = SCM_RIGHTS,
-               }
-       };
-       struct msghdr msghdr = {
-               .msg_iov = iov,
-               .msg_iovlen = iov_len,
-               .msg_control = &fd_buf,
-               .msg_controllen = sizeof(fd_buf),
-       };
+       uint8_t fd_buf[CMSG_SPACE(sizeof(int))] = { 0 };
+       struct msghdr msghdr = { 0 };
+       struct cmsghdr *cmsg;
        int len = 0;
+       int *pfd;
+
+       msghdr.msg_iov = iov,
+       msghdr.msg_iovlen = iov_len,
+       msghdr.msg_control = fd_buf;
+       msghdr.msg_controllen = sizeof(fd_buf);
+
+       cmsg = CMSG_FIRSTHDR(&msghdr);
+       cmsg->cmsg_type = SCM_RIGHTS;
+       cmsg->cmsg_level = SOL_SOCKET;
+       cmsg->cmsg_len = CMSG_LEN(sizeof(int));
+
+       pfd = (int *) CMSG_DATA(cmsg);
+       msghdr.msg_controllen = cmsg->cmsg_len;
 
        do {
                ssize_t cur_len;
@@ -84,7 +85,7 @@ static int writev_retry(int fd, struct iovec *iov, int iov_len, int sock_fd)
                        msghdr.msg_control = NULL;
                        msghdr.msg_controllen = 0;
                } else {
-                       fd_buf.fd = sock_fd;
+                       *pfd = sock_fd;
                }
 
                cur_len = sendmsg(fd, &msghdr, 0);
@@ -156,33 +157,38 @@ int __hidden ubus_send_msg(struct ubus_context *ctx, uint32_t seq,
 
 static int recv_retry(struct ubus_context *ctx, struct iovec *iov, bool wait, int *recv_fd)
 {
-       int bytes, total = 0;
-       int fd = ctx->sock.fd;
-       static struct {
-               int fd;
-               struct cmsghdr h;
-       } fd_buf = {
-               .h = {
-                       .cmsg_type = SCM_RIGHTS,
-                       .cmsg_level = SOL_SOCKET,
-                       .cmsg_len = sizeof(fd_buf),
-               },
-       };
-       struct msghdr msghdr = {
-               .msg_iov = iov,
-               .msg_iovlen = 1,
-       };
+       uint8_t fd_buf[CMSG_SPACE(sizeof(int))] = { 0 };
+       struct msghdr msghdr = { 0 };
+       struct cmsghdr *cmsg;
+       int total = 0;
+       int bytes;
+       int *pfd;
+       int fd;
+
+       fd = ctx->sock.fd;
+
+       msghdr.msg_iov = iov,
+       msghdr.msg_iovlen = 1,
+       msghdr.msg_control = fd_buf;
+       msghdr.msg_controllen = sizeof(fd_buf);
+
+       cmsg = CMSG_FIRSTHDR(&msghdr);
+       cmsg->cmsg_type = SCM_RIGHTS;
+       cmsg->cmsg_level = SOL_SOCKET;
+       cmsg->cmsg_len = CMSG_LEN(sizeof(int));
+
+       pfd = (int *) CMSG_DATA(cmsg);
 
        while (iov->iov_len > 0) {
                if (recv_fd) {
-                       msghdr.msg_control = &fd_buf;
-                       msghdr.msg_controllen = sizeof(fd_buf);
+                       msghdr.msg_control = fd_buf;
+                       msghdr.msg_controllen = cmsg->cmsg_len;
                } else {
                        msghdr.msg_control = NULL;
                        msghdr.msg_controllen = 0;
                }
 
-               fd_buf.fd = -1;
+               *pfd = -1;
                bytes = recvmsg(fd, &msghdr, 0);
                if (!bytes)
                        return -1;
@@ -199,7 +205,7 @@ static int recv_retry(struct ubus_context *ctx, struct iovec *iov, bool wait, in
                        return 0;
 
                if (recv_fd)
-                       *recv_fd = fd_buf.fd;
+                       *recv_fd = *pfd;
 
                recv_fd = NULL;
 

diff --git a/ubusd.c b/ubusd.c
index 0d43977c0bde920077b806f850c6607fecd5b958..5993653785e01cf6767d6d3f5a50e86e33ec660d 100644 (file)
--- a/ubusd.c
+++ b/ubusd.c
@@ -82,27 +82,28 @@ void ubus_msg_free(struct ubus_msg_buf *ub)
 
 ssize_t ubus_msg_writev(int fd, struct ubus_msg_buf *ub, size_t offset)
 {
+       uint8_t fd_buf[CMSG_SPACE(sizeof(int))] = { 0 };
        static struct iovec iov[2];
-       static struct {
-               int fd;
-               struct cmsghdr h;
-       } fd_buf = {
-               .h = {
-                       .cmsg_len = sizeof(fd_buf),
-                       .cmsg_level = SOL_SOCKET,
-                       .cmsg_type = SCM_RIGHTS,
-               },
-       };
-       struct msghdr msghdr = {
-               .msg_iov = iov,
-               .msg_iovlen = ARRAY_SIZE(iov),
-               .msg_control = &fd_buf,
-               .msg_controllen = sizeof(fd_buf),
-       };
+       struct msghdr msghdr = { 0 };
        struct ubus_msghdr hdr;
+       struct cmsghdr *cmsg;
        ssize_t ret;
+       int *pfd;
 
-       fd_buf.fd = ub->fd;
+       msghdr.msg_iov = iov;
+       msghdr.msg_iovlen = ARRAY_SIZE(iov);
+       msghdr.msg_control = fd_buf;
+       msghdr.msg_controllen = sizeof(fd_buf);
+
+       cmsg = CMSG_FIRSTHDR(&msghdr);
+       cmsg->cmsg_type = SCM_RIGHTS;
+       cmsg->cmsg_level = SOL_SOCKET;
+       cmsg->cmsg_len = CMSG_LEN(sizeof(int));
+
+       pfd = (int *) CMSG_DATA(cmsg);
+       msghdr.msg_controllen = cmsg->cmsg_len;
+
+       *pfd = ub->fd;
        if (ub->fd < 0 || offset) {
                msghdr.msg_control = NULL;
                msghdr.msg_controllen = 0;

diff --git a/ubusd_main.c b/ubusd_main.c
index 81868c1482bcc7e6029000e7437d7bb9dd9b7705..f977ff30d12d50c561f4a73ccb1463cd75a403d9 100644 (file)
--- a/ubusd_main.c
+++ b/ubusd_main.c
@@ -50,22 +50,25 @@ static void handle_client_disconnect(struct ubus_client *cl)
 static void client_cb(struct uloop_fd *sock, unsigned int events)
 {
        struct ubus_client *cl = container_of(sock, struct ubus_client, sock);
+       uint8_t fd_buf[CMSG_SPACE(sizeof(int))] = { 0 };
+       struct msghdr msghdr = { 0 };
        struct ubus_msg_buf *ub;
        static struct iovec iov;
-       static struct {
-               int fd;
-               struct cmsghdr h;
-       } fd_buf = {
-               .h = {
-                       .cmsg_type = SCM_RIGHTS,
-                       .cmsg_level = SOL_SOCKET,
-                       .cmsg_len = sizeof(fd_buf),
-               }
-       };
-       struct msghdr msghdr = {
-               .msg_iov = &iov,
-               .msg_iovlen = 1,
-       };
+       struct cmsghdr *cmsg;
+       int *pfd;
+
+       msghdr.msg_iov = &iov,
+       msghdr.msg_iovlen = 1,
+       msghdr.msg_control = fd_buf;
+       msghdr.msg_controllen = sizeof(fd_buf);
+
+       cmsg = CMSG_FIRSTHDR(&msghdr);
+       cmsg->cmsg_type = SCM_RIGHTS;
+       cmsg->cmsg_level = SOL_SOCKET;
+       cmsg->cmsg_len = CMSG_LEN(sizeof(int));
+
+       pfd = (int *) CMSG_DATA(cmsg);
+       msghdr.msg_controllen = cmsg->cmsg_len;
 
        /* first try to tx more pending data */
        while ((ub = ubus_msg_head(cl))) {
@@ -100,14 +103,14 @@ retry:
                int offset = cl->pending_msg_offset;
                int bytes;
 
-               fd_buf.fd = -1;
+               *pfd = -1;
 
                iov.iov_base = ((char *) &cl->hdrbuf) + offset;
                iov.iov_len = sizeof(cl->hdrbuf) - offset;
 
                if (cl->pending_msg_fd < 0) {
-                       msghdr.msg_control = &fd_buf;
-                       msghdr.msg_controllen = sizeof(fd_buf);
+                       msghdr.msg_control = fd_buf;
+                       msghdr.msg_controllen = cmsg->cmsg_len;
                } else {
                        msghdr.msg_control = NULL;
                        msghdr.msg_controllen = 0;
@@ -117,8 +120,8 @@ retry:
                if (bytes < 0)
                        goto out;
 
-               if (fd_buf.fd >= 0)
-                       cl->pending_msg_fd = fd_buf.fd;
+               if (*pfd >= 0)
+                       cl->pending_msg_fd = *pfd;
 
                cl->pending_msg_offset += bytes;
                if (cl->pending_msg_offset < (int) sizeof(cl->hdrbuf))