ubusd/libubus-io: fix socket descriptor passing

In commit 5d7ca8309d ("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: 5d7ca8309d ("ubusd/libubus-io: fix variable sized struct position warning")
Reported-by: Hannu Nyman <hannu.nyman@welho.com>
Signed-off-by: Petr Štetiar <ynezz@true.cz>
This commit is contained in:
Petr Štetiar 2019-12-27 14:48:32 +01:00
parent 8f2292478c
commit 041c9d1c05
3 changed files with 83 additions and 73 deletions

View file

@ -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 int writev_retry(int fd, struct iovec *iov, int iov_len, int sock_fd)
{ {
static struct { uint8_t fd_buf[CMSG_SPACE(sizeof(int))] = { 0 };
int fd; struct msghdr msghdr = { 0 };
struct cmsghdr h; struct cmsghdr *cmsg;
} 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),
};
int len = 0; 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 { do {
ssize_t cur_len; 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_control = NULL;
msghdr.msg_controllen = 0; msghdr.msg_controllen = 0;
} else { } else {
fd_buf.fd = sock_fd; *pfd = sock_fd;
} }
cur_len = sendmsg(fd, &msghdr, 0); 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) static int recv_retry(struct ubus_context *ctx, struct iovec *iov, bool wait, int *recv_fd)
{ {
int bytes, total = 0; uint8_t fd_buf[CMSG_SPACE(sizeof(int))] = { 0 };
int fd = ctx->sock.fd; struct msghdr msghdr = { 0 };
static struct { struct cmsghdr *cmsg;
int fd; int total = 0;
struct cmsghdr h; int bytes;
} fd_buf = { int *pfd;
.h = { int fd;
.cmsg_type = SCM_RIGHTS,
.cmsg_level = SOL_SOCKET, fd = ctx->sock.fd;
.cmsg_len = sizeof(fd_buf),
}, msghdr.msg_iov = iov,
}; msghdr.msg_iovlen = 1,
struct msghdr msghdr = { msghdr.msg_control = fd_buf;
.msg_iov = iov, msghdr.msg_controllen = sizeof(fd_buf);
.msg_iovlen = 1,
}; 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) { while (iov->iov_len > 0) {
if (recv_fd) { if (recv_fd) {
msghdr.msg_control = &fd_buf; msghdr.msg_control = fd_buf;
msghdr.msg_controllen = sizeof(fd_buf); msghdr.msg_controllen = cmsg->cmsg_len;
} else { } else {
msghdr.msg_control = NULL; msghdr.msg_control = NULL;
msghdr.msg_controllen = 0; msghdr.msg_controllen = 0;
} }
fd_buf.fd = -1; *pfd = -1;
bytes = recvmsg(fd, &msghdr, 0); bytes = recvmsg(fd, &msghdr, 0);
if (!bytes) if (!bytes)
return -1; return -1;
@ -199,7 +205,7 @@ static int recv_retry(struct ubus_context *ctx, struct iovec *iov, bool wait, in
return 0; return 0;
if (recv_fd) if (recv_fd)
*recv_fd = fd_buf.fd; *recv_fd = *pfd;
recv_fd = NULL; recv_fd = NULL;

35
ubusd.c
View file

@ -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) 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 iovec iov[2];
static struct { struct msghdr msghdr = { 0 };
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 ubus_msghdr hdr; struct ubus_msghdr hdr;
struct cmsghdr *cmsg;
ssize_t ret; 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) { if (ub->fd < 0 || offset) {
msghdr.msg_control = NULL; msghdr.msg_control = NULL;
msghdr.msg_controllen = 0; msghdr.msg_controllen = 0;

View file

@ -50,22 +50,25 @@ static void handle_client_disconnect(struct ubus_client *cl)
static void client_cb(struct uloop_fd *sock, unsigned int events) static void client_cb(struct uloop_fd *sock, unsigned int events)
{ {
struct ubus_client *cl = container_of(sock, struct ubus_client, sock); 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; struct ubus_msg_buf *ub;
static struct iovec iov; static struct iovec iov;
static struct { struct cmsghdr *cmsg;
int fd; int *pfd;
struct cmsghdr h;
} fd_buf = { msghdr.msg_iov = &iov,
.h = { msghdr.msg_iovlen = 1,
.cmsg_type = SCM_RIGHTS, msghdr.msg_control = fd_buf;
.cmsg_level = SOL_SOCKET, msghdr.msg_controllen = sizeof(fd_buf);
.cmsg_len = sizeof(fd_buf),
} cmsg = CMSG_FIRSTHDR(&msghdr);
}; cmsg->cmsg_type = SCM_RIGHTS;
struct msghdr msghdr = { cmsg->cmsg_level = SOL_SOCKET;
.msg_iov = &iov, cmsg->cmsg_len = CMSG_LEN(sizeof(int));
.msg_iovlen = 1,
}; pfd = (int *) CMSG_DATA(cmsg);
msghdr.msg_controllen = cmsg->cmsg_len;
/* first try to tx more pending data */ /* first try to tx more pending data */
while ((ub = ubus_msg_head(cl))) { while ((ub = ubus_msg_head(cl))) {
@ -100,14 +103,14 @@ retry:
int offset = cl->pending_msg_offset; int offset = cl->pending_msg_offset;
int bytes; int bytes;
fd_buf.fd = -1; *pfd = -1;
iov.iov_base = ((char *) &cl->hdrbuf) + offset; iov.iov_base = ((char *) &cl->hdrbuf) + offset;
iov.iov_len = sizeof(cl->hdrbuf) - offset; iov.iov_len = sizeof(cl->hdrbuf) - offset;
if (cl->pending_msg_fd < 0) { if (cl->pending_msg_fd < 0) {
msghdr.msg_control = &fd_buf; msghdr.msg_control = fd_buf;
msghdr.msg_controllen = sizeof(fd_buf); msghdr.msg_controllen = cmsg->cmsg_len;
} else { } else {
msghdr.msg_control = NULL; msghdr.msg_control = NULL;
msghdr.msg_controllen = 0; msghdr.msg_controllen = 0;
@ -117,8 +120,8 @@ retry:
if (bytes < 0) if (bytes < 0)
goto out; goto out;
if (fd_buf.fd >= 0) if (*pfd >= 0)
cl->pending_msg_fd = fd_buf.fd; cl->pending_msg_fd = *pfd;
cl->pending_msg_offset += bytes; cl->pending_msg_offset += bytes;
if (cl->pending_msg_offset < (int) sizeof(cl->hdrbuf)) if (cl->pending_msg_offset < (int) sizeof(cl->hdrbuf))