From 78ea09ed1633a583dbcde6e7bab9df4639ec8a34 Mon Sep 17 00:00:00 2001 From: Roy Marples Date: Mon, 22 Jun 2026 23:41:53 +0100 Subject: [PATCH] control: Avoid hangup in the recvdata path Instead return an error and bubble it up where it can be hangup / freed more cleanly. Reported-by: CuB3y0nd --- src/control.c | 47 ++++++++++++++++++++++++------------------- src/control.h | 2 +- src/privsep-control.c | 7 ++++++- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/control.c b/src/control.c index 24d7480c..ad006c20 100644 --- a/src/control.c +++ b/src/control.c @@ -113,10 +113,8 @@ control_handle_read(struct fd_list *fd) bytes = read(fd->fd, buffer, sizeof(buffer) - 1); if (bytes == -1) logerr(__func__); - if (bytes == -1 || bytes == 0) { - control_hangup(fd); - return -1; - } + if (bytes == -1 || bytes == 0) + return (int)bytes; #ifdef PRIVSEP if (IN_PRIVSEP(fd->ctx)) { @@ -132,15 +130,13 @@ control_handle_read(struct fd_list *fd) if (err == 1 && ps_ctl_sendargs(fd, buffer, (size_t)bytes) == -1) { logerr(__func__); - control_free(fd); return -1; } - return 0; + return 1; } #endif - control_recvdata(fd, buffer, (size_t)bytes); - return 0; + return control_recvdata(fd, buffer, (size_t)bytes); } static int @@ -203,23 +199,31 @@ static void control_handle_data(void *arg, unsigned short events) { struct fd_list *fd = arg; + int err; if (!(events & (ELE_READ | ELE_WRITE | ELE_HANGUP))) logerrx("%s: unexpected event 0x%04x", __func__, events); if (events & ELE_WRITE && !(events & ELE_HANGUP)) { - if (control_handle_write(fd) == -1) - return; + err = control_handle_write(fd); + if (err == -1) + goto hangup; } if (events & ELE_READ) { - if (control_handle_read(fd) == -1) - return; + err = control_handle_read(fd); + if (err == -1 || err == 0) + goto hangup; } if (events & ELE_HANGUP) - control_hangup(fd); + goto hangup; + + return; + +hangup: + control_hangup(fd); } -void +int control_recvdata(struct fd_list *fd, char *data, size_t len) { char *p = data, *e; @@ -241,12 +245,13 @@ control_recvdata(struct fd_list *fd, char *data, size_t len) if (e == NULL) { errno = EINVAL; logerrx("%s: no terminator", __func__); - return; + return -1; } - if ((size_t)argc >= sizeof(argvp) / sizeof(argvp[0])) { + if ((size_t)argc + 1 >= + sizeof(argvp) / sizeof(argvp[0])) { errno = ENOBUFS; logerrx("%s: no arg buffer", __func__); - return; + return -1; } *ap++ = p; argc++; @@ -266,12 +271,12 @@ control_recvdata(struct fd_list *fd, char *data, size_t len) *ap = NULL; if (dhcpcd_handleargs(fd->ctx, fd, argc, argvp) == -1) { logerr(__func__); - if (errno != EINTR && errno != EAGAIN) { - control_free(fd); - return; - } + if (errno != EINTR && errno != EAGAIN) + return -1; } } + + return 1; } struct fd_list * diff --git a/src/control.h b/src/control.h index dd696d20..9e966f77 100644 --- a/src/control.h +++ b/src/control.h @@ -77,5 +77,5 @@ struct fd_list *control_new(struct dhcpcd_ctx *, int, unsigned int); void control_free(struct fd_list *); void control_delete(struct fd_list *); int control_queue(struct fd_list *, void *, size_t); -void control_recvdata(struct fd_list *fd, char *, size_t); +int control_recvdata(struct fd_list *fd, char *, size_t); #endif diff --git a/src/privsep-control.c b/src/privsep-control.c index 10768a32..3737d07d 100644 --- a/src/privsep-control.c +++ b/src/privsep-control.c @@ -109,6 +109,7 @@ ps_ctl_dispatch(void *arg, struct ps_msghdr *psm, struct msghdr *msg) struct iovec *iov = msg->msg_iov; struct fd_list *fd; unsigned int fd_flags = FD_SENDLEN; + int err; switch (psm->ps_flags) { case PS_CTL_PRIV: @@ -132,7 +133,11 @@ ps_ctl_dispatch(void *arg, struct ps_msghdr *psm, struct msghdr *msg) if (fd == NULL) return -1; ctx->ps_control_client = fd; - control_recvdata(fd, iov->iov_base, iov->iov_len); + err = control_recvdata(fd, iov->iov_base, iov->iov_len); + if (err == -1 || err == 0) { + control_free(fd); + ctx->ps_control_client = NULL; + } break; case PS_CTL_EOF: ctx->ps_control_client = NULL; -- 2.47.3