From: Lennart Poettering Date: Tue, 9 Jun 2020 11:40:25 +0000 (+0200) Subject: tree-wide: check POLLNVAL everywhere X-Git-Tag: v246-rc1~178 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dad28bffd67692d6c22e0de98ddb0d217c034ec5;p=thirdparty%2Fsystemd.git tree-wide: check POLLNVAL everywhere poll() sets POLLNVAL inside of the poll structures if an invalid fd is passed. So far we generally didn't check for that, thus not taking notice of the error. Given that this specific kind of error is generally indication of a programming error, and given that our code is embedded into our projects via NSS or because people link against our library, let's explicitly check for this and convert it to EBADF. (I ran into a busy loop because of this missing check when some of my test code accidentally closed an fd it shouldn't close, so this is a real thing) --- diff --git a/src/basic/io-util.c b/src/basic/io-util.c index c906fc07417..4b3d1c814f5 100644 --- a/src/basic/io-util.c +++ b/src/basic/io-util.c @@ -33,10 +33,13 @@ int flush_fd(int fd) { continue; return -errno; - - } else if (r == 0) + } + if (r == 0) return count; + if (pollfd.revents & POLLNVAL) + return -EBADF; + l = read(fd, buf, sizeof(buf)); if (l < 0) { @@ -169,6 +172,9 @@ int pipe_eof(int fd) { if (r == 0) return 0; + if (pollfd.revents & POLLNVAL) + return -EBADF; + return pollfd.revents & POLLHUP; } @@ -188,6 +194,9 @@ int fd_wait_for_event(int fd, int event, usec_t t) { if (r == 0) return 0; + if (pollfd.revents & POLLNVAL) + return -EBADF; + return pollfd.revents; } diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index 05dd7e70014..2c990047fc6 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -1002,6 +1002,9 @@ int flush_accept(int fd) { if (r == 0) return 0; + if (pollfd.revents & POLLNVAL) + return -EBADF; + if (iteration >= MAX_FLUSH_ITERATIONS) return log_debug_errno(SYNTHETIC_ERRNO(EBUSY), "Failed to flush connections within " STRINGIFY(MAX_FLUSH_ITERATIONS) " iterations."); diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index a082995498f..859f4bbd44a 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -2093,10 +2093,13 @@ static int wait_for_change(sd_journal *j, int poll_fd) { return log_error_errno(errno, "Couldn't wait for journal event: %m"); } - if (pollfds[1].revents & (POLLHUP|POLLERR)) /* STDOUT has been closed? */ + if (pollfds[1].revents & (POLLHUP|POLLERR|POLLNVAL)) /* STDOUT has been closed? */ return log_debug_errno(SYNTHETIC_ERRNO(ECANCELED), "Standard output has been closed."); + if (pollfds[0].revents & POLLNVAL) + return log_debug_errno(SYNTHETIC_ERRNO(EBADF), "Change fd closed?"); + r = sd_journal_process(j); if (r < 0) return log_error_errno(r, "Failed to process journal events: %m"); diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index acc313d164b..778f80377f8 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -1275,6 +1275,8 @@ int bus_socket_process_opening(sd_bus *b) { r = poll(&p, 1, 0); if (r < 0) return -errno; + if (p.revents & POLLNVAL) + return -EBADF; if (!(p.revents & (POLLOUT|POLLERR|POLLHUP))) return 0; diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index cd1a79a256d..9de5e454a6b 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -3121,8 +3121,15 @@ static int bus_poll(sd_bus *bus, bool need_more, uint64_t timeout_usec) { r = ppoll(p, n, m == USEC_INFINITY ? NULL : timespec_store(&ts, m), NULL); if (r < 0) return -errno; + if (r == 0) + return 0; + + if (p[0].revents & POLLNVAL) + return -EBADF; + if (n >= 2 && (p[1].revents & POLLNVAL)) + return -EBADF; - return r > 0 ? 1 : 0; + return 1; } _public_ int sd_bus_wait(sd_bus *bus, uint64_t timeout_usec) { diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c index 587a1f2595b..0bc0fc16ae5 100644 --- a/src/libsystemd/sd-daemon/sd-daemon.c +++ b/src/libsystemd/sd-daemon/sd-daemon.c @@ -577,6 +577,8 @@ _public_ int sd_notify_barrier(int unset_environment, uint64_t timeout) { return -errno; if (r == 0) return -ETIMEDOUT; + if (pfd.revents & POLLNVAL) + return -EBADF; return 1; } diff --git a/src/libsystemd/sd-netlink/sd-netlink.c b/src/libsystemd/sd-netlink/sd-netlink.c index 5b7081089e4..ddeb4a79924 100644 --- a/src/libsystemd/sd-netlink/sd-netlink.c +++ b/src/libsystemd/sd-netlink/sd-netlink.c @@ -462,7 +462,6 @@ static usec_t calc_elapse(uint64_t usec) { } static int rtnl_poll(sd_netlink *rtnl, bool need_more, uint64_t timeout_usec) { - struct pollfd p[1] = {}; struct timespec ts; usec_t m = USEC_INFINITY; int r, e; @@ -495,14 +494,21 @@ static int rtnl_poll(sd_netlink *rtnl, bool need_more, uint64_t timeout_usec) { if (timeout_usec != (uint64_t) -1 && (m == (uint64_t) -1 || timeout_usec < m)) m = timeout_usec; - p[0].fd = rtnl->fd; - p[0].events = e; + struct pollfd p = { + .fd = rtnl->fd, + .events = e, + }; - r = ppoll(p, 1, m == (uint64_t) -1 ? NULL : timespec_store(&ts, m), NULL); + r = ppoll(&p, 1, m == (uint64_t) -1 ? NULL : timespec_store(&ts, m), NULL); if (r < 0) return -errno; + if (r == 0) + return 0; - return r > 0 ? 1 : 0; + if (p.revents & POLLNVAL) + return -EBADF; + + return 1; } int sd_netlink_wait(sd_netlink *nl, uint64_t timeout_usec) { diff --git a/src/libudev/libudev-monitor.c b/src/libudev/libudev-monitor.c index 5f780e0be95..bf23021b530 100644 --- a/src/libudev/libudev-monitor.c +++ b/src/libudev/libudev-monitor.c @@ -216,8 +216,11 @@ static int udev_monitor_receive_sd_device(struct udev_monitor *udev_monitor, sd_ continue; return -errno; - } else if (r == 0) + } + if (r == 0) return -EAGAIN; + if (pfd.revents & POLLNVAL) + return -EBADF; /* receive next message */ break; diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index 4c5781a11ad..3d0b9391069 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -305,6 +305,12 @@ int ask_password_plymouth( goto finish; } + if (pollfd[POLL_SOCKET].revents & POLLNVAL || + (notify >= 0 && pollfd[POLL_INOTIFY].revents & POLLNVAL)) { + r = -EBADF; + goto finish; + } + if (notify >= 0 && pollfd[POLL_INOTIFY].revents != 0) (void) flush_fd(notify); @@ -541,6 +547,12 @@ int ask_password_tty( goto finish; } + if ((pollfd[POLL_TTY].revents & POLLNVAL) || + (notify >= 0 && (pollfd[POLL_INOTIFY].revents & POLLNVAL))) { + r = -EBADF; + goto finish; + } + if (notify >= 0 && pollfd[POLL_INOTIFY].revents != 0 && keyname) { (void) flush_fd(notify); @@ -888,6 +900,13 @@ int ask_password_agent( goto finish; } + if (pollfd[FD_SOCKET].revents & POLLNVAL || + pollfd[FD_SIGNAL].revents & POLLNVAL || + (notify >= 0 && pollfd[FD_INOTIFY].revents & POLLNVAL)) { + r = -EBADF; + goto finish; + } + if (pollfd[FD_SIGNAL].revents & POLLIN) { r = -EINTR; goto finish; diff --git a/src/shared/barrier.c b/src/shared/barrier.c index bb5869dad40..80b597b5cad 100644 --- a/src/shared/barrier.c +++ b/src/shared/barrier.c @@ -218,10 +218,14 @@ static bool barrier_read(Barrier *b, int64_t comp) { uint64_t buf; int r; - r = poll(pfd, 2, -1); - if (r < 0 && IN_SET(errno, EAGAIN, EINTR)) - continue; - else if (r < 0) + r = poll(pfd, ELEMENTSOF(pfd), -1); + if (r < 0) { + if (IN_SET(errno, EAGAIN, EINTR)) + continue; + goto error; + } + if (pfd[0].revents & POLLNVAL || + pfd[1].revents & POLLNVAL) goto error; if (pfd[1].revents) { diff --git a/src/shared/utmp-wtmp.c b/src/shared/utmp-wtmp.c index fa4f32f3538..7d0d90247cc 100644 --- a/src/shared/utmp-wtmp.c +++ b/src/shared/utmp-wtmp.c @@ -334,9 +334,10 @@ static int write_to_terminal(const char *tty, const char *message) { k = poll(&pollfd, 1, (end - t) / USEC_PER_MSEC); if (k < 0) return -errno; - if (k == 0) return -ETIME; + if (pollfd.revents & POLLNVAL) + return -EBADF; n = write(fd, p, left); if (n < 0) { diff --git a/src/shared/varlink.c b/src/shared/varlink.c index ac111555e9a..c0b58742254 100644 --- a/src/shared/varlink.c +++ b/src/shared/varlink.c @@ -1048,10 +1048,14 @@ int varlink_wait(Varlink *v, usec_t timeout) { NULL); if (r < 0) return -errno; + if (r == 0) + return 0; - handle_revents(v, pfd.revents); + if (pfd.revents & POLLNVAL) + return -EBADF; - return r > 0 ? 1 : 0; + handle_revents(v, pfd.revents); + return 1; } int varlink_get_fd(Varlink *v) { @@ -1139,9 +1143,14 @@ int varlink_flush(Varlink *v) { .events = POLLOUT, }; - if (poll(&pfd, 1, -1) < 0) + r = poll(&pfd, 1, -1); + if (r < 0) return -errno; + assert(r > 0); + if (pfd.revents & POLLNVAL) + return -EBADF; + handle_revents(v, pfd.revents); } diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index fbfddc0262f..4f9c23099a9 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -271,6 +271,9 @@ static int execute_s2h(const SleepConfig *sleep_config) { tfd = safe_close(tfd); + if (FLAGS_SET(fds.revents, POLLNVAL)) + return log_error_errno(SYNTHETIC_ERRNO(EBADF), "Invalid timer fd to sleep on?"); + if (!FLAGS_SET(fds.revents, POLLIN)) /* We woke up before the alarm time, we are done. */ return 0; diff --git a/src/stdio-bridge/stdio-bridge.c b/src/stdio-bridge/stdio-bridge.c index 52b9ce45555..ca145aebf93 100644 --- a/src/stdio-bridge/stdio-bridge.c +++ b/src/stdio-bridge/stdio-bridge.c @@ -238,17 +238,19 @@ static int run(int argc, char *argv[]) { ts = timespec_store(&_ts, t); } - { - struct pollfd p[3] = { - {.fd = fd, .events = events_a }, - {.fd = STDIN_FILENO, .events = events_b & POLLIN }, - {.fd = STDOUT_FILENO, .events = events_b & POLLOUT }, - }; - - r = ppoll(p, ELEMENTSOF(p), ts, NULL); - } + struct pollfd p[3] = { + { .fd = fd, .events = events_a }, + { .fd = STDIN_FILENO, .events = events_b & POLLIN }, + { .fd = STDOUT_FILENO, .events = events_b & POLLOUT }, + }; + + r = ppoll(p, ELEMENTSOF(p), ts, NULL); if (r < 0) return log_error_errno(errno, "ppoll() failed: %m"); + if (p[0].revents & POLLNVAL || + p[1].revents & POLLNVAL || + p[2].revents & POLLNVAL) + return log_error_errno(SYNTHETIC_ERRNO(EBADF), "Invalid file descriptor to poll on?"); } return 0; diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index eb847191113..4371da47850 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -395,6 +395,10 @@ static int process_and_watch_password_files(bool watch) { return -errno; } + if (pollfd[FD_SIGNAL].revents & POLLNVAL || + pollfd[FD_INOTIFY].revents & POLLNVAL) + return -EBADF; + if (pollfd[FD_INOTIFY].revents != 0) (void) flush_fd(notify); diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c index 7d7044eac3c..1fa267c831c 100644 --- a/src/udev/udevadm-settle.c +++ b/src/udev/udevadm-settle.c @@ -202,7 +202,12 @@ int settle_main(int argc, char *argv[], void *userdata) { return -ETIMEDOUT; /* wake up when queue becomes empty */ - if (poll(&pfd, 1, MSEC_PER_SEC) > 0 && pfd.revents & POLLIN) { + r = poll(&pfd, 1, MSEC_PER_SEC); + if (r < 0) + return -errno; + if (pfd.revents & POLLNVAL) + return -EBADF; + if (r > 0 && pfd.revents & POLLIN) { r = udev_queue_flush(queue); if (r < 0) return log_error_errno(r, "Failed to flush queue: %m"); diff --git a/src/userdb/userwork.c b/src/userdb/userwork.c index ac04af0ca8d..4a8c98384d4 100644 --- a/src/userdb/userwork.c +++ b/src/userdb/userwork.c @@ -757,6 +757,8 @@ static int run(int argc, char *argv[]) { if (poll(&pfd, 1, 0) < 0) return log_error_errno(errno, "Failed to test for POLLIN on listening socket: %m"); + if (FLAGS_SET(pfd.revents, POLLNVAL)) + return log_error_errno(SYNTHETIC_ERRNO(EBADF), "Listening socket dead?"); if (FLAGS_SET(pfd.revents, POLLIN)) { pid_t parent;