From 44e61dab7effb739f543b8b4eee43f577bf2db49 Mon Sep 17 00:00:00 2001 From: Andy Pan Date: Tue, 10 Sep 2024 02:22:15 +0800 Subject: [PATCH] kqueue: disallow ill-suited file descriptor kinds (#4513) Follows up on https://github.com/libuv/libuv/pull/659. Signed-off-by: Andy Pan --- src/unix/kqueue.c | 33 +++++++++++++++++++++++++++ test/test-poll.c | 57 +++++++++++++++++++++++++++++++++++++---------- 2 files changed, 78 insertions(+), 12 deletions(-) diff --git a/src/unix/kqueue.c b/src/unix/kqueue.c index 1762165e..876b7170 100644 --- a/src/unix/kqueue.c +++ b/src/unix/kqueue.c @@ -99,6 +99,39 @@ int uv__io_fork(uv_loop_t* loop) { int uv__io_check_fd(uv_loop_t* loop, int fd) { struct kevent ev; int rc; + struct stat sb; +#ifdef __APPLE__ + char path[MAXPATHLEN]; +#endif + + if (uv__fstat(fd, &sb)) + return UV__ERR(errno); + + /* On FreeBSD, kqueue only supports EVFILT_READ notification for regular files + * and always reports ready events for writing, resulting in busy-looping. + * + * On Darwin, DragonFlyBSD, NetBSD and OpenBSD, kqueue reports ready events for + * regular files as readable and writable only once, acting like an EV_ONESHOT. + * + * Neither of the above cases should be added to the kqueue. + */ + if (S_ISREG(sb.st_mode) || S_ISDIR(sb.st_mode)) + return UV_EINVAL; + +#ifdef __APPLE__ + /* On Darwin (both macOS and iOS), in addition to regular files, FIFOs also don't + * work properly with kqueue: the disconnection from the last writer won't trigger + * an event for kqueue in spite of what the man pages say. Thus, we also disallow + * the case of S_IFIFO. */ + if (S_ISFIFO(sb.st_mode)) { + /* File descriptors of FIFO, pipe and kqueue share the same type of file, + * therefore there is no way to tell them apart via stat.st_mode&S_IFMT. + * Fortunately, FIFO is the only one that has a persisted file on filesystem, + * from which we're able to make the distinction for it. */ + if (!fcntl(fd, F_GETPATH, path)) + return UV_EINVAL; + } +#endif rc = 0; EV_SET(&ev, fd, EVFILT_READ, EV_ADD, 0, 0, 0); diff --git a/test/test-poll.c b/test/test-poll.c index fcd644f2..5161de25 100644 --- a/test/test-poll.c +++ b/test/test-poll.c @@ -626,26 +626,59 @@ TEST_IMPL(poll_unidirectional) { /* Windows won't let you open a directory so we open a file instead. - * OS X lets you poll a file so open the $PWD instead. Both fail - * on Linux so it doesn't matter which one we pick. Both succeed - * on FreeBSD, Solaris and AIX so skip the test on those platforms. + * OS X lets you poll a file so open the $PWD instead. Both fail + * on Linux so it doesn't matter which one we pick. Both succeed + * on Solaris and AIX so skip the test on those platforms. + * On *BSD/Darwin, we disallow polling of regular files, directories. + * In addition to regular files, we also disallow FIFOs on Darwin. */ +#ifdef __APPLE__ +#define TEST_POLL_FIFO_PATH "/tmp/uv-test-poll-fifo" +#endif TEST_IMPL(poll_bad_fdtype) { -#if !defined(__DragonFly__) && !defined(__FreeBSD__) && !defined(__sun) && \ +#if !defined(__sun) && \ !defined(_AIX) && !defined(__MVS__) && \ - !defined(__OpenBSD__) && !defined(__CYGWIN__) && !defined(__MSYS__) && \ - !defined(__NetBSD__) + !defined(__CYGWIN__) && !defined(__MSYS__) uv_poll_t poll_handle; - int fd; + int fd[2]; #if defined(_WIN32) - fd = _open("test/fixtures/empty_file", UV_FS_O_RDONLY); + fd[0] = _open("test/fixtures/empty_file", UV_FS_O_RDONLY); #else - fd = open(".", UV_FS_O_RDONLY); + fd[0] = open(".", UV_FS_O_RDONLY); +#endif + ASSERT_NE(fd[0], -1); + ASSERT_NE(0, uv_poll_init(uv_default_loop(), &poll_handle, fd[0])); + ASSERT_OK(close(fd[0])); +#if defined(__APPLE__) || \ + defined(__DragonFly__) || \ + defined(__FreeBSD__) || \ + defined(__OpenBSD__) || \ + defined(__NetBSD__) + fd[0] = open("test/fixtures/empty_file", UV_FS_O_RDONLY); + ASSERT_NE(fd[0], -1); + /* Regular files should be banned from kqueue. */ + ASSERT_NE(0, uv_poll_init(uv_default_loop(), &poll_handle, fd[0])); + ASSERT_OK(close(fd[0])); +#ifdef __APPLE__ + ASSERT_OK(pipe(fd)); + /* Pipes should be permitted in kqueue. */ + ASSERT_EQ(0, uv_poll_init(uv_default_loop(), &poll_handle, fd[0])); + ASSERT_OK(close(fd[0])); + ASSERT_OK(close(fd[1])); + + ASSERT_OK(mkfifo(TEST_POLL_FIFO_PATH, 0600)); + fd[0] = open(TEST_POLL_FIFO_PATH, O_RDONLY | O_NONBLOCK); + ASSERT_NE(fd[0], -1); + fd[1] = open(TEST_POLL_FIFO_PATH, O_WRONLY | O_NONBLOCK); + ASSERT_NE(fd[1], -1); + /* FIFOs should be banned from kqueue. */ + ASSERT_NE(0, uv_poll_init(uv_default_loop(), &poll_handle, fd[0])); + ASSERT_OK(close(fd[0])); + ASSERT_OK(close(fd[1])); + unlink(TEST_POLL_FIFO_PATH); +#endif #endif - ASSERT_NE(fd, -1); - ASSERT_NE(0, uv_poll_init(uv_default_loop(), &poll_handle, fd)); - ASSERT_OK(close(fd)); #endif MAKE_VALGRIND_HAPPY(uv_default_loop());