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 <root@cubeyond.net>

diff --git a/src/control.c b/src/control.c
index 24d7480ca01da916fc74180dfd469ac310eb1b2c..ad006c203bf2ada0a2353f67a5291eaa5c749e19 100644 (file)
--- 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 dd696d20ac0b6829c2ee98cd461454978201c950..9e966f7766a105c57d362b709028fc5432ca1e05 100644 (file)
--- 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 10768a3221f95e7785d45e6c30ac9edd8de72d4a..3737d07dbd3f3eab42fd36afd00467eadfd217fb 100644 (file)
--- 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;