[OpenWrt-Devel] [PATCH v2 ubus] ubusd/libubus-io: fix socket descriptor passing

Petr Štetiar ynezz at true.cz
Fri Dec 27 09:15:19 EST 2019


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 at welho.com>
Signed-off-by: Petr Štetiar <ynezz at true.cz>
---

 changes since v1:

  - removed unnecessary rename of the `msghdr` variable to new `msg` name

 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 ba1016d0fa09..3561ac462eb9 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 0d43977c0bde..5993653785e0 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 81868c1482bc..f977ff30d12d 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))

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


More information about the openwrt-devel mailing list