From patchwork Fri Dec 27 13:59:22 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Petr_=C5=A0tetiar?= X-Patchwork-Id: 1215719 X-Patchwork-Delegate: ynezz@true.cz Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:e::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=true.cz Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="fr/xQOoG"; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 47kpNc0mQHz9sPh for ; Sat, 28 Dec 2019 00:59:43 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Subject:In-Reply-To:MIME-Version: Message-Id:Date:To:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=oK65P2Zoh1OB2SFLloBvafPqNPEqoT5aOjROF4z14pE=; b=fr/xQOoG1U79++ wGIFEDpnf+c89yFzsh6cHUge9tqxoH60rxSNgiNKzM+70j4RcT5QNXqPHvwActob3XO88kk14j6HX sDh6lyTJqsfXugy+eIxSghYYhv/ZvVgsdLWxO9AXmkKahYuluX33C+W3yylTSgQMZyw9DmevpdD3m 177+pHdL73DZiImoyO0YofTktIAIl/J34S1aZr3tXCXNa6fKPv7fUHvB74LnmHtf3lZcam9y7Xwyt h1ZpWwW96uusMYo2W/2b3tqVRMPOo3yuxnK1e6l6xCM1avdAV6NCR5QkTm4UySlnBe1y39m1efCGJ wqYLYyAvDqxhgL9LO5QA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1ikq9Q-0007Z9-3t; Fri, 27 Dec 2019 13:59:36 +0000 Received: from smtp-out.xnet.cz ([178.217.244.18]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1ikq9M-0007Yd-0e for openwrt-devel@lists.openwrt.org; Fri, 27 Dec 2019 13:59:34 +0000 Received: from meh.true.cz (meh.true.cz [108.61.167.218]) (Authenticated sender: petr@true.cz) by smtp-out.xnet.cz (Postfix) with ESMTPSA id EC7FE38B8; Fri, 27 Dec 2019 14:59:27 +0100 (CET) Received: by meh.true.cz (OpenSMTPD) with ESMTP id 1021b207; Fri, 27 Dec 2019 14:59:17 +0100 (CET) From: =?utf-8?q?Petr_=C5=A0tetiar?= To: openwrt-devel@lists.openwrt.org Date: Fri, 27 Dec 2019 14:59:22 +0100 Message-Id: <20191227135922.13972-1-ynezz@true.cz> MIME-Version: 1.0 In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191227_055932_356484_547C81B1 X-CRM114-Status: GOOD ( 12.19 ) X-Spam-Score: 0.0 (/) X-Spam-Report: SpamAssassin version 3.4.2 on bombadil.infradead.org summary: Content analysis details: (0.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [178.217.244.18 listed in list.dnswl.org] 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 SPF_NONE SPF: sender does not publish an SPF Record Subject: [OpenWrt-Devel] [PATCH ubus] ubusd/libubus-io: fix socket descriptor passing X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Hannu Nyman , =?utf-8?q?Petr_=C5=A0tetiar?= Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org 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 | 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))