From 041c9d1c052bb4936fd03240f7d0dd64aedda972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0tetiar?= Date: Fri, 27 Dec 2019 14:48:32 +0100 Subject: [PATCH] ubusd/libubus-io: fix socket descriptor passing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Petr Štetiar --- libubus-io.c | 80 ++++++++++++++++++++++++++++------------------------ ubusd.c | 35 ++++++++++++----------- ubusd_main.c | 41 ++++++++++++++------------- 3 files changed, 83 insertions(+), 73 deletions(-) diff --git a/libubus-io.c b/libubus-io.c index ba1016d..3561ac4 100644 --- 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 0d43977..5993653 100644 --- 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 81868c1..f977ff3 100644 --- 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))