From 6ea17d709e1f2689b762f4f3cfca951c1c2446a3 Mon Sep 17 00:00:00 2001 From: Roy Marples Date: Tue, 5 May 2026 21:02:16 +0100 Subject: [PATCH 1/3] privsep: Change IPC to use SOCK_STREAM macOS does not support SOCK_SEQPACKET. All our messages use a fixed header which includes the lengths of all parts sent. We can use these limits with MSG_WAITALL on blocking sockets in place of MSG_EOR to get the same effect. Start of the work for #524. --- src/dhcpcd.c | 8 ++--- src/logerr.c | 31 ++++++++++++----- src/logerr.h | 2 +- src/privsep-bpf.c | 2 +- src/privsep-control.c | 2 +- src/privsep-inet.c | 2 +- src/privsep-root.c | 80 +++++++++++++++++-------------------------- src/privsep.c | 36 +++++++++++++------ 8 files changed, 88 insertions(+), 75 deletions(-) diff --git a/src/dhcpcd.c b/src/dhcpcd.c index 3f5fedda..7b62be0b 100644 --- a/src/dhcpcd.c +++ b/src/dhcpcd.c @@ -384,7 +384,7 @@ dhcpcd_daemonise(struct dhcpcd_ctx *ctx) eloop_event_delete(ctx->eloop, ctx->fork_fd); exit_code = EXIT_SUCCESS; - if (send(ctx->fork_fd, &exit_code, sizeof(exit_code), MSG_EOR) == -1) + if (send(ctx->fork_fd, &exit_code, sizeof(exit_code), 0) == -1) logerr(__func__); close(ctx->fork_fd); ctx->fork_fd = -1; @@ -1498,7 +1498,7 @@ dhcpcd_signal_cb(int sig, void *arg) if (sig != SIGCHLD && ctx->options & DHCPCD_FORKED) { if (ctx->fork_fd != -1 && sig != SIGHUP && - send(ctx->fork_fd, &sig, sizeof(sig), MSG_EOR) == -1) + send(ctx->fork_fd, &sig, sizeof(sig), 0) == -1) logerr("%s: send", __func__); return; } @@ -1905,7 +1905,7 @@ dhcpcd_fork_cb(void *arg, unsigned short events) if (!(events & ELE_READ)) logerrx("%s: unexpected event 0x%04x", __func__, events); - len = read(ctx->fork_fd, &exit_code, sizeof(exit_code)); + len = recv(ctx->fork_fd, &exit_code, sizeof(exit_code), MSG_WAITALL); if (len == -1) { logerr(__func__); eloop_exit(ctx->eloop, EXIT_FAILURE); @@ -2469,7 +2469,7 @@ main(int argc, char **argv, char **envp) if (!(ctx.options & DHCPCD_DAEMONISE)) goto start_manager; - if (xsocketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CXNB, 0, fork_fd) == + if (xsocketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, fork_fd) == -1) { logerr("socketpair"); goto exit_failure; diff --git a/src/logerr.c b/src/logerr.c index af13b249..88d085ef 100644 --- a/src/logerr.c +++ b/src/logerr.c @@ -218,17 +218,20 @@ __printflike(2, 0) static int vlogmessage(int pri, const char *fmt, if (ctx->log_fd != -1) { pid_t pid = getpid(); char buf[LOGERR_SYSLOGBUF]; + size_t mlen = 0; struct iovec iov[] = { { .iov_base = &pri, .iov_len = sizeof(pri) }, { .iov_base = &pid, .iov_len = sizeof(pid) }, + { .iov_base = &mlen, .iov_len = sizeof(mlen) }, { .iov_base = buf }, }; len = vsnprintf(buf, sizeof(buf), fmt, args); if (len != -1) { - if ((size_t)len >= sizeof(buf)) - len = (int)sizeof(buf) - 1; - iov[2].iov_len = (size_t)(len + 1); + mlen = (size_t)len + 1; + if (mlen > sizeof(buf)) + mlen = sizeof(buf); + iov[3].iov_len = mlen; struct msghdr msg = { .msg_iov = iov, .msg_iovlen = sizeof(iov) / sizeof(iov[0]), @@ -399,10 +402,11 @@ logreadfd(int fd) int pri; pid_t pid; char buf[LOGERR_SYSLOGBUF] = { '\0' }; + size_t mlen; struct iovec iov[] = { { .iov_base = &pri, .iov_len = sizeof(pri) }, { .iov_base = &pid, .iov_len = sizeof(pid) }, - { .iov_base = buf, .iov_len = sizeof(buf) }, + { .iov_base = &mlen, .iov_len = sizeof(mlen) }, }; struct msghdr msg = { .msg_iov = iov, .msg_iovlen = sizeof(iov) / sizeof(iov[0]) }; @@ -411,14 +415,25 @@ logreadfd(int fd) len = recvmsg(fd, &msg, MSG_WAITALL); if (len == -1 || len == 0) return len; - /* Ensure we received the minimum and at least one character to log */ - if ((size_t)len < sizeof(pri) + sizeof(pid) + 1 || - msg.msg_flags & MSG_TRUNC) { + if ((size_t)len != sizeof(pri) + sizeof(pid) + sizeof(mlen)) { errno = EMSGSIZE; return -1; } + if (mlen > sizeof(buf)) { + errno = ENOBUFS; + return -1; + } + + len = recv(fd, buf, mlen, MSG_WAITALL); + if (len == -1) + return -1; + if ((size_t)len != mlen) { + errno = EINVAL; + return -1; + } + /* Ensure what we receive is NUL terminated */ - buf[(size_t)len - (sizeof(pri) + sizeof(pid)) - 1] = '\0'; + buf[mlen] = '\0'; ctx->log_pid = pid; logmessage(pri, "%s", buf); diff --git a/src/logerr.h b/src/logerr.h index 5d3a0149..f5196a62 100644 --- a/src/logerr.h +++ b/src/logerr.h @@ -76,7 +76,7 @@ __printflike(2, 3) void logerrmessage(int pri, const char *fmt, ...); #define logerr(...) log_err(__VA_ARGS__) #define logerrx(...) log_errx(__VA_ARGS__) -/* For logging in a chroot using SOCK_SEQPACKET */ +/* For logging in a chroot using SOCK_STREAM */ int loggetfd(void); void logsetfd(int); ssize_t logreadfd(int); diff --git a/src/privsep-bpf.c b/src/privsep-bpf.c index 4277d574..dab7b802 100644 --- a/src/privsep-bpf.c +++ b/src/privsep-bpf.c @@ -53,7 +53,7 @@ #include "logerr.h" #include "privsep.h" -/* We expect to have open 3 SEQPACKET and one RAW fd */ +/* We expect to have open 3 SOCK_STREAM and one RAW fd */ static void ps_bpf_recvbpf(void *arg, unsigned short events) diff --git a/src/privsep-control.c b/src/privsep-control.c index 24591324..76f6df79 100644 --- a/src/privsep-control.c +++ b/src/privsep-control.c @@ -36,7 +36,7 @@ #include "logerr.h" #include "privsep.h" -/* We expect to have open 2 SEQPACKET, 2 STREAM and 2 file STREAM fds */ +/* We expect to have open 2 privsep STREAM, 2 STREAM and 2 file STREAM fds */ static int ps_ctl_startcb(struct ps_process *psp) diff --git a/src/privsep-inet.c b/src/privsep-inet.c index 590e770b..4ff48218 100644 --- a/src/privsep-inet.c +++ b/src/privsep-inet.c @@ -47,7 +47,7 @@ #include "logerr.h" #include "privsep.h" -/* We expect to have open 2 SEQPACKET, 1 udp, 1 udp6 and 1 raw6 fds */ +/* We expect to have open 2 SOCK_STREAM, 1 udp, 1 udp6 and 1 raw6 fds */ #ifdef INET static void diff --git a/src/privsep-root.c b/src/privsep-root.c index 0b1bdd4b..fbf62819 100644 --- a/src/privsep-root.c +++ b/src/privsep-root.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -79,11 +80,6 @@ ps_root_readerrorcb(struct psr_ctx *pc) struct dhcpcd_ctx *ctx = pc->psr_ctx; int fd = PS_ROOT_FD(ctx); struct psr_error *psr_error = &pc->psr_error; - struct iovec iov[] = { - { .iov_base = psr_error, .iov_len = sizeof(*psr_error) }, - { .iov_base = pc->psr_data, .iov_len = pc->psr_datalen }, - }; - struct msghdr msg = { .msg_iov = iov, .msg_iovlen = __arraycount(iov) }; ssize_t len; #define PSR_ERROR(e) \ @@ -95,45 +91,23 @@ ps_root_readerrorcb(struct psr_ctx *pc) if (eloop_waitfd(fd) == -1) PSR_ERROR(errno); - if (!pc->psr_mallocdata) - goto recv; - - /* We peek at the psr_error structure to tell us how much of a buffer - * we need to read the whole packet. */ - msg.msg_iovlen--; - len = recvmsg(fd, &msg, MSG_PEEK | MSG_WAITALL); + len = recv(fd, psr_error, sizeof(*psr_error), MSG_WAITALL); if (len == -1) PSR_ERROR(errno); - /* After this point, we MUST do another recvmsg even on a failure - * to remove the message after peeking. */ - if ((size_t)len < sizeof(*psr_error)) { - /* We can't use the header to work out buffers, so - * remove the message and bail. */ - (void)recvmsg(fd, &msg, MSG_WAITALL); + if ((size_t)len < sizeof(*psr_error)) PSR_ERROR(EINVAL); - } - /* No data to read? Unlikely but ... */ if (psr_error->psr_datalen == 0) - goto recv; + return len; - pc->psr_data = malloc(psr_error->psr_datalen); - if (pc->psr_data != NULL) { - iov[1].iov_base = pc->psr_data; - iov[1].iov_len = psr_error->psr_datalen; - msg.msg_iovlen++; - } + if (pc->psr_mallocdata) + pc->psr_data = malloc(psr_error->psr_datalen); -recv: - len = recvmsg(fd, &msg, MSG_WAITALL); + len = recv(fd, pc->psr_data, psr_error->psr_datalen, MSG_WAITALL); if (len == -1) PSR_ERROR(errno); - else if ((size_t)len < sizeof(*psr_error)) - PSR_ERROR(EINVAL); - else if (msg.msg_flags & MSG_TRUNC) - PSR_ERROR(ENOBUFS); - else if ((size_t)len != sizeof(*psr_error) + psr_error->psr_datalen) { + else if ((size_t)len != psr_error->psr_datalen) { #ifdef PRIVSEP_DEBUG logerrx("%s: recvmsg returned %zd, expecting %zu", __func__, len, sizeof(*psr_error) + psr_error->psr_datalen); @@ -204,7 +178,7 @@ ps_root_writeerror(struct dhcpcd_ctx *ctx, ssize_t result, void *data, if (len == 0) msg.msg_iovlen = 1; - err = sendmsg(fd, &msg, MSG_EOR); + err = sendmsg(fd, &msg, 0); /* Error sending the message? Try sending the error of sending. */ if (err == -1 && errno != EPIPE) { @@ -214,7 +188,7 @@ ps_root_writeerror(struct dhcpcd_ctx *ctx, ssize_t result, void *data, psr.psr_errno = errno; psr.psr_datalen = 0; msg.msg_iovlen = 1; - err = sendmsg(fd, &msg, MSG_EOR); + err = sendmsg(fd, &msg, 0); } return err; @@ -857,14 +831,14 @@ ps_root_start(struct dhcpcd_ctx *ctx) int logfd[2] = { -1, -1 }, datafd[2] = { -1, -1 }; pid_t pid; - if (xsocketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CXNB, 0, logfd) == -1) + if (xsocketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, logfd) == -1) return -1; #ifdef PRIVSEP_RIGHTS if (ps_rights_limit_fdpair(logfd) == -1) return -1; #endif - if (xsocketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CXNB, 0, datafd) == -1) + if (xsocketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, datafd) == -1) return -1; if (ps_setbuf_fdpair(datafd) == -1) @@ -950,20 +924,30 @@ ps_root_stop(struct dhcpcd_ctx *ctx) /* drain the log */ if (ctx->ps_log_root_fd != -1) { ssize_t loglen; - -#ifdef __linux__ - /* Seems to help to get the last parts, - * sched_yield(2) does not. */ - sleep(0); -#endif - do { + struct pollfd pfd = { + .fd = ctx->ps_log_root_fd, + .events = POLLIN + }; + int n; + + /* the socket is blocking and we may not be able to + * change it to non blocking, so poll for data */ + for (;;) { + n = poll(&pfd, 1, 1); + if (n == -1 || n == 0) + break; loglen = logreadfd(ctx->ps_log_root_fd); - } while (loglen != 0 && loglen != -1); - close(ctx->ps_log_root_fd); - ctx->ps_log_root_fd = -1; + if (loglen == -1 || loglen == 0) + break; + } } } + if (ctx->ps_log_root_fd != -1) { + close(ctx->ps_log_root_fd); + ctx->ps_log_root_fd = -1; + } + if (ctx->ps_data_fd != -1) { eloop_event_delete(ctx->eloop, ctx->ps_data_fd); close(ctx->ps_data_fd); diff --git a/src/privsep.c b/src/privsep.c index d59902fd..48e71a9f 100644 --- a/src/privsep.c +++ b/src/privsep.c @@ -350,7 +350,7 @@ ps_startprocess(struct ps_process *psp, int fd[2]; pid_t pid; - if (xsocketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CXNB, 0, fd) == -1) { + if (xsocketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, fd) == -1) { logerr("%s: socketpair", __func__); return -1; } @@ -881,6 +881,7 @@ ps_sendpsmmsg(struct dhcpcd_ctx *ctx, int fd, struct ps_msghdr *psm, psm->ps_namelen = msg->msg_namelen; psm->ps_controllen = (socklen_t)msg->msg_controllen; + psm->ps_datalen = 0; iovp->iov_base = msg->msg_name; iovp->iov_len = msg->msg_namelen; @@ -908,10 +909,11 @@ ps_sendpsmmsg(struct dhcpcd_ctx *ctx, int fd, struct ps_msghdr *psm, iovp->iov_base = msg->msg_iov[i].iov_base; iovp->iov_len = msg->msg_iov[i].iov_len; iovp++; + psm->ps_datalen += msg->msg_iov[i].iov_len; } } - len = sendmsg(fd, &m, MSG_EOR); + len = sendmsg(fd, &m, 0); if (len == -1 && ctx != NULL) { if (ctx->options & DHCPCD_FORKED && @@ -1064,12 +1066,14 @@ ps_recvpsmsg(struct dhcpcd_ctx *ctx, int fd, unsigned short events, ssize_t (*callback)(void *, struct ps_msghdr *, struct msghdr *), void *cbctx) { - struct ps_msg psm; + struct ps_msghdr psm; + uint8_t ps_data[PS_BUFLEN]; ssize_t len; size_t dlen; struct iovec iov[1]; struct msghdr msg = { .msg_iov = iov, .msg_iovlen = 1 }; bool stop = false; + socklen_t cmsg_padlen; if (events & ELE_HANGUP) { len = 0; @@ -1078,24 +1082,24 @@ ps_recvpsmsg(struct dhcpcd_ctx *ctx, int fd, unsigned short events, if (!(events & ELE_READ)) logerrx("%s: unexpected event 0x%04x", __func__, events); - len = read(fd, &psm, sizeof(psm)); + len = recv(fd, &psm, sizeof(psm), MSG_WAITALL); #ifdef PRIVSEP_DEBUG - logdebugx("%s: fd=%d %zd", __func__, fd, len); + logdebugx("%s: pid=%d fd=%d len=%zd", __func__, getpid(), fd, len); #endif if (len == -1 || len == 0) stop = true; else { dlen = (size_t)len; - if (dlen < sizeof(psm.psm_hdr)) { + if (dlen < sizeof(psm)) { errno = EINVAL; return -1; } - if (psm.psm_hdr.ps_cmd == PS_STOP) { + if (psm.ps_cmd == PS_STOP) { stop = true; len = 0; - } else if (psm.psm_hdr.ps_cmd == PS_DAEMONISED) { + } else if (psm.ps_cmd == PS_DAEMONISED) { ps_daemonised(ctx); return 0; } @@ -1111,16 +1115,26 @@ ps_recvpsmsg(struct dhcpcd_ctx *ctx, int fd, unsigned short events, eloop_exit(ctx->eloop, len != -1 ? EXIT_SUCCESS : EXIT_FAILURE); return len; } - dlen -= sizeof(psm.psm_hdr); - if (ps_unrollmsg(&msg, &psm.psm_hdr, psm.psm_data, dlen) == -1) + cmsg_padlen = CALC_CMSG_PADLEN(psm.ps_controllen, + psm.ps_namelen); + dlen = psm.ps_namelen + psm.ps_controllen + cmsg_padlen + psm.ps_datalen; + if (dlen != 0) { + len = recv(fd, ps_data, dlen, MSG_WAITALL); + if ((size_t)len != dlen) { + errno = EINVAL; + return -1; + } + } + + if (ps_unrollmsg(&msg, &psm, ps_data, dlen) == -1) return -1; if (callback == NULL) return 0; errno = 0; - return callback(cbctx, &psm.psm_hdr, &msg); + return callback(cbctx, &psm, &msg); } struct ps_process * From 1a95498a540f4214ae96df375cd64ae7276dc20a Mon Sep 17 00:00:00 2001 From: Roy Marples Date: Tue, 5 May 2026 23:07:50 +0100 Subject: [PATCH 2/3] Address review comments. --- src/dhcpcd.c | 2 +- src/logerr.c | 4 ++-- src/privsep-root.c | 6 +++++- src/privsep.c | 4 ++++ 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/dhcpcd.c b/src/dhcpcd.c index 7b62be0b..b5c5ae28 100644 --- a/src/dhcpcd.c +++ b/src/dhcpcd.c @@ -2785,7 +2785,7 @@ main(int argc, char **argv, char **envp) #ifdef USE_SIGNALS /* If still attached, detach from the launcher */ if (ctx.options & DHCPCD_STARTED && ctx.fork_fd != -1) { - if (send(ctx.fork_fd, &i, sizeof(i), MSG_EOR) == -1) + if (send(ctx.fork_fd, &i, sizeof(i), 0) == -1) logerr("%s: send", __func__); } #endif diff --git a/src/logerr.c b/src/logerr.c index 88d085ef..763c8da4 100644 --- a/src/logerr.c +++ b/src/logerr.c @@ -236,7 +236,7 @@ __printflike(2, 0) static int vlogmessage(int pri, const char *fmt, .msg_iov = iov, .msg_iovlen = sizeof(iov) / sizeof(iov[0]), }; - len = (int)sendmsg(ctx->log_fd, &msg, MSG_EOR); + len = (int)sendmsg(ctx->log_fd, &msg, 0); } return len; } @@ -419,7 +419,7 @@ logreadfd(int fd) errno = EMSGSIZE; return -1; } - if (mlen > sizeof(buf)) { + if (mlen >= sizeof(buf)) { errno = ENOBUFS; return -1; } diff --git a/src/privsep-root.c b/src/privsep-root.c index fbf62819..96e1e054 100644 --- a/src/privsep-root.c +++ b/src/privsep-root.c @@ -101,8 +101,12 @@ ps_root_readerrorcb(struct psr_ctx *pc) if (psr_error->psr_datalen == 0) return len; - if (pc->psr_mallocdata) + if (pc->psr_mallocdata) { pc->psr_data = malloc(psr_error->psr_datalen); + if (pc->psr_data == NULL) + PSR_ERROR(errno); + } else if (psr_error->psr_datalen > pc->psr_datalen) + PSR_ERROR(EMSGSIZE); len = recv(fd, pc->psr_data, psr_error->psr_datalen, MSG_WAITALL); if (len == -1) diff --git a/src/privsep.c b/src/privsep.c index 48e71a9f..5d1e4899 100644 --- a/src/privsep.c +++ b/src/privsep.c @@ -1120,6 +1120,10 @@ ps_recvpsmsg(struct dhcpcd_ctx *ctx, int fd, unsigned short events, psm.ps_namelen); dlen = psm.ps_namelen + psm.ps_controllen + cmsg_padlen + psm.ps_datalen; if (dlen != 0) { + if (dlen > sizeof(ps_data)) { + errno = EMSGSIZE; + return -1; + } len = recv(fd, ps_data, dlen, MSG_WAITALL); if ((size_t)len != dlen) { errno = EINVAL; From 691ee9e237cd6e3b334d330bb6e3397c2e5bba2e Mon Sep 17 00:00:00 2001 From: Roy Marples Date: Tue, 5 May 2026 23:37:45 +0100 Subject: [PATCH 3/3] Address more review comments. --- src/logerr.c | 4 ++-- src/privsep.c | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/logerr.c b/src/logerr.c index 763c8da4..c2d318f9 100644 --- a/src/logerr.c +++ b/src/logerr.c @@ -419,7 +419,7 @@ logreadfd(int fd) errno = EMSGSIZE; return -1; } - if (mlen >= sizeof(buf)) { + if (mlen > sizeof(buf)) { errno = ENOBUFS; return -1; } @@ -433,7 +433,7 @@ logreadfd(int fd) } /* Ensure what we receive is NUL terminated */ - buf[mlen] = '\0'; + buf[mlen == sizeof(buf) ? mlen - 1 : mlen] = '\0'; ctx->log_pid = pid; logmessage(pri, "%s", buf); diff --git a/src/privsep.c b/src/privsep.c index 5d1e4899..56265432 100644 --- a/src/privsep.c +++ b/src/privsep.c @@ -902,10 +902,11 @@ ps_sendpsmmsg(struct dhcpcd_ctx *ctx, int fd, struct ps_msghdr *psm, m.msg_iovlen++; for (i = 0; i < (int)msg->msg_iovlen; i++) { - if ((size_t)(m.msg_iovlen++) > __arraycount(iov)) { + if ((size_t)m.msg_iovlen >= __arraycount(iov)) { errno = ENOBUFS; return -1; } + m.msg_iovlen++; iovp->iov_base = msg->msg_iov[i].iov_base; iovp->iov_len = msg->msg_iov[i].iov_len; iovp++;