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 <[email protected]> Signed-off-by: Petr Štetiar <[email protected]> --- libubus-io.c | 96 ++++++++++++++++++++++++++++------------------------ ubusd.c | 43 +++++++++++------------ ubusd_main.c | 47 +++++++++++++------------ 3 files changed, 98 insertions(+), 88 deletions(-) diff --git a/libubus-io.c b/libubus-io.c index ba1016d0fa09..7668dc52e8d3 100644 --- a/libubus-io.c +++ b/libubus-io.c @@ -59,35 +59,36 @@ 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 msg = { 0 }; + struct cmsghdr *cmsg; int len = 0; + int *pfd; + + msg.msg_iov = iov, + msg.msg_iovlen = iov_len, + msg.msg_control = fd_buf; + msg.msg_controllen = sizeof(fd_buf); + + cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_len = CMSG_LEN(sizeof(int)); + + pfd = (int *) CMSG_DATA(cmsg); + msg.msg_controllen = cmsg->cmsg_len; do { ssize_t cur_len; if (sock_fd < 0) { - msghdr.msg_control = NULL; - msghdr.msg_controllen = 0; + msg.msg_control = NULL; + msg.msg_controllen = 0; } else { - fd_buf.fd = sock_fd; + *pfd = sock_fd; } - cur_len = sendmsg(fd, &msghdr, 0); + cur_len = sendmsg(fd, &msg, 0); if (cur_len < 0) { switch(errno) { case EAGAIN: @@ -114,8 +115,8 @@ static int writev_retry(int fd, struct iovec *iov, int iov_len, int sock_fd) } iov->iov_base += cur_len; iov->iov_len -= cur_len; - msghdr.msg_iov = iov; - msghdr.msg_iovlen = iov_len; + msg.msg_iov = iov; + msg.msg_iovlen = iov_len; } while (1); /* Should never reach here */ @@ -156,34 +157,39 @@ 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 msg = { 0 }; + struct cmsghdr *cmsg; + int total = 0; + int bytes; + int *pfd; + int fd; + + fd = ctx->sock.fd; + + msg.msg_iov = iov, + msg.msg_iovlen = 1, + msg.msg_control = fd_buf; + msg.msg_controllen = sizeof(fd_buf); + + cmsg = CMSG_FIRSTHDR(&msg); + 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); + msg.msg_control = fd_buf; + msg.msg_controllen = cmsg->cmsg_len; } else { - msghdr.msg_control = NULL; - msghdr.msg_controllen = 0; + msg.msg_control = NULL; + msg.msg_controllen = 0; } - fd_buf.fd = -1; - bytes = recvmsg(fd, &msghdr, 0); + *pfd = -1; + bytes = recvmsg(fd, &msg, 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 0d43977c0bde..6f3a280cdf57 100644 --- a/ubusd.c +++ b/ubusd.c @@ -82,30 +82,31 @@ 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 msg = { 0 }; struct ubus_msghdr hdr; + struct cmsghdr *cmsg; ssize_t ret; + int *pfd; - fd_buf.fd = ub->fd; + msg.msg_iov = iov; + msg.msg_iovlen = ARRAY_SIZE(iov); + msg.msg_control = fd_buf; + msg.msg_controllen = sizeof(fd_buf); + + cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_len = CMSG_LEN(sizeof(int)); + + pfd = (int *) CMSG_DATA(cmsg); + msg.msg_controllen = cmsg->cmsg_len; + + *pfd = ub->fd; if (ub->fd < 0 || offset) { - msghdr.msg_control = NULL; - msghdr.msg_controllen = 0; + msg.msg_control = NULL; + msg.msg_controllen = 0; } if (offset < sizeof(ub->hdr)) { @@ -122,11 +123,11 @@ ssize_t ubus_msg_writev(int fd, struct ubus_msg_buf *ub, size_t offset) offset -= sizeof(ub->hdr); iov[0].iov_base = ((char *) ub->data) + offset; iov[0].iov_len = ub->len - offset; - msghdr.msg_iovlen = 1; + msg.msg_iovlen = 1; } do { - ret = sendmsg(fd, &msghdr, 0); + ret = sendmsg(fd, &msg, 0); } while (ret < 0 && errno == EINTR); return ret; diff --git a/ubusd_main.c b/ubusd_main.c index 81868c1482bc..11cb2f99f7b6 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 msg = { 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; + + msg.msg_iov = &iov, + msg.msg_iovlen = 1, + msg.msg_control = fd_buf; + msg.msg_controllen = sizeof(fd_buf); + + cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_len = CMSG_LEN(sizeof(int)); + + pfd = (int *) CMSG_DATA(cmsg); + msg.msg_controllen = cmsg->cmsg_len; /* first try to tx more pending data */ while ((ub = ubus_msg_head(cl))) { @@ -100,25 +103,25 @@ 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); + msg.msg_control = fd_buf; + msg.msg_controllen = cmsg->cmsg_len; } else { - msghdr.msg_control = NULL; - msghdr.msg_controllen = 0; + msg.msg_control = NULL; + msg.msg_controllen = 0; } - bytes = recvmsg(sock->fd, &msghdr, 0); + bytes = recvmsg(sock->fd, &msg, 0); 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)) _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
