[OpenWrt-Devel] [PATCH ubus] ubusd/libubus-io: fix socket descriptor passing
Petr Štetiar
ynezz at true.cz
Fri Dec 27 08:59:22 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>
---
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
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list