Re: git: f5ea3dce2cbe - main - libnv: switch fd_wait() from select(2) to poll(2)
Date: Fri, 01 May 2026 08:11:17 UTC
On Wednesday, April 29, 2026, Mark Johnston <markj@freebsd.org> wrote:
> The branch main has been updated by markj:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=
> f5ea3dce2cbe1ee2068c5e5c11bb066f5789685b
>
> commit f5ea3dce2cbe1ee2068c5e5c11bb066f5789685b
> Author: Mariusz Zaborski <oshogbo@FreeBSD.org>
> AuthorDate: 2026-04-28 14:35:10 +0000
> Commit: Mark Johnston <markj@FreeBSD.org>
> CommitDate: 2026-04-29 14:39:28 +0000
>
> libnv: switch fd_wait() from select(2) to poll(2)
>
> The previous implementation used FD_SET() on a stack-allocated fd_set,
> which is an out-of-bounds write whenever the socket fd is >= FD_SETSIZE
> (1024).
This problem seems like a more generic problem, after looking into
sys/select.h
What about adding a check for the FD_SET about the variable is on the stack
or not? And enforce the FD_SETSIZE limit almost the same way as it is
already done with fortify source.
Maybe the https://lists.llvm.org/pipermail/llvm-dev/2013-October/066294.html
can
be used (or abused) to implement this check.
Feel free to correct me, if I'm wrong.
> Approved by: so
> Security: FreeBSD-SA-26:16.libnv
> Security: CVE-2026-39457
> Reported by: Joshua Rogers of AISLE Research Team (
> https://aisle.com/)
> Reviewed by: markj
> Differential Revision: https://reviews.freebsd.org/D56689
> ---
> lib/libnv/msgio.c | 12 +++----
> lib/libnv/tests/nvlist_send_recv_test.c | 56
> +++++++++++++++++++++++++++++++++
> 2 files changed, 62 insertions(+), 6 deletions(-)
>
> diff --git a/lib/libnv/msgio.c b/lib/libnv/msgio.c
> index f6f75241ff06..de2994e47fef 100644
> --- a/lib/libnv/msgio.c
> +++ b/lib/libnv/msgio.c
> @@ -32,10 +32,10 @@
>
> #include <sys/param.h>
> #include <sys/socket.h>
> -#include <sys/select.h>
>
> #include <errno.h>
> #include <fcntl.h>
> +#include <poll.h>
> #include <stdbool.h>
> #include <stdint.h>
> #include <stdlib.h>
> @@ -86,14 +86,14 @@ msghdr_add_fd(struct cmsghdr *cmsg, int fd)
> static void
> fd_wait(int fd, bool doread)
> {
> - fd_set fds;
> + struct pollfd pfd;
>
> PJDLOG_ASSERT(fd >= 0);
>
> - FD_ZERO(&fds);
> - FD_SET(fd, &fds);
> - (void)select(fd + 1, doread ? &fds : NULL, doread ? NULL : &fds,
> - NULL, NULL);
> + pfd.fd = fd;
> + pfd.events = doread ? POLLIN : POLLOUT;
> + pfd.revents = 0;
> + (void)poll(&pfd, 1, -1);
> }
>
> static int
> diff --git a/lib/libnv/tests/nvlist_send_recv_test.c
> b/lib/libnv/tests/nvlist_send_recv_test.c
> index 4a5c10df656d..d655a26a7362 100644
> --- a/lib/libnv/tests/nvlist_send_recv_test.c
> +++ b/lib/libnv/tests/nvlist_send_recv_test.c
> @@ -27,6 +27,8 @@
> */
>
> #include <sys/param.h>
> +#include <sys/resource.h>
> +#include <sys/select.h>
> #include <sys/socket.h>
> #include <sys/sysctl.h>
> #include <sys/wait.h>
> @@ -531,6 +533,59 @@ ATF_TC_BODY(nvlist_send_recv__send_nvlist__stream,
> tc)
> nvlist_send_recv__send_nvlist(SOCK_STREAM);
> }
>
> +/*
> + * Regression test for fd_wait(): the previous select(2)-based
> implementation
> + * called FD_SET() unconditionally, which is an out-of-bounds stack write
> when
> + * the socket fd is >= FD_SETSIZE. Force the socketpair fds above
> FD_SETSIZE
> + * and verify a full nvlist round-trip still works.
> + */
> +ATF_TC_WITHOUT_HEAD(nvlist_send_recv__highfd);
> +ATF_TC_BODY(nvlist_send_recv__highfd, tc)
> +{
> + struct rlimit rl;
> + nvlist_t *nvl;
> + int socks[2], hi_send, hi_recv, status;
> + pid_t pid;
> +
> + hi_send = FD_SETSIZE + 5;
> + hi_recv = FD_SETSIZE + 6;
> +
> + rl.rlim_cur = rl.rlim_max = hi_recv + 1;
> + if (setrlimit(RLIMIT_NOFILE, &rl) != 0)
> + atf_tc_skip("cannot raise RLIMIT_NOFILE: %s",
> strerror(errno));
> +
> + ATF_REQUIRE(socketpair(PF_UNIX, SOCK_STREAM, 0, socks) == 0);
> + ATF_REQUIRE(dup2(socks[0], hi_recv) == hi_recv);
> + ATF_REQUIRE(dup2(socks[1], hi_send) == hi_send);
> + (void)close(socks[0]);
> + (void)close(socks[1]);
> +
> + pid = fork();
> + ATF_REQUIRE(pid >= 0);
> + if (pid == 0) {
> + /* Child: send. */
> + (void)close(hi_recv);
> + nvl = nvlist_create(0);
> + nvlist_add_string(nvl, "key", "value");
> + if (nvlist_send(hi_send, nvl) != 0)
> + err(EXIT_FAILURE, "nvlist_send");
> + nvlist_destroy(nvl);
> + _exit(0);
> + }
> +
> + (void)close(hi_send);
> + nvl = nvlist_recv(hi_recv, 0);
> + ATF_REQUIRE(nvl != NULL);
> + ATF_REQUIRE(nvlist_error(nvl) == 0);
> + ATF_REQUIRE(nvlist_exists_string(nvl, "key"));
> + ATF_REQUIRE(strcmp(nvlist_get_string(nvl, "key"), "value") == 0);
> + nvlist_destroy(nvl);
> +
> + ATF_REQUIRE(waitpid(pid, &status, 0) == pid);
> + ATF_REQUIRE(status == 0);
> + (void)close(hi_recv);
> +}
> +
> ATF_TC_WITHOUT_HEAD(nvlist_send_recv__send_closed_fd__dgram);
> ATF_TC_BODY(nvlist_send_recv__send_closed_fd__dgram, tc)
> {
> @@ -734,6 +789,7 @@ ATF_TP_ADD_TCS(tp)
>
> ATF_TP_ADD_TC(tp, nvlist_send_recv__send_nvlist__dgram);
> ATF_TP_ADD_TC(tp, nvlist_send_recv__send_nvlist__stream);
> + ATF_TP_ADD_TC(tp, nvlist_send_recv__highfd);
> ATF_TP_ADD_TC(tp, nvlist_send_recv__send_closed_fd__dgram);
> ATF_TP_ADD_TC(tp, nvlist_send_recv__send_closed_fd__stream);
> ATF_TP_ADD_TC(tp, nvlist_send_recv__send_many_fds__dgram);
>
>