From b5610f303491ad416ed63ae207ad87f44ebb532e Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Fri, 25 Oct 2024 18:03:51 +0800 Subject: [PATCH] Report `POLLNVAL` in `poll` for invalid FDs --- kernel/src/syscall/poll.rs | 32 ++++++++++++++++++++++---- kernel/src/syscall/select.rs | 18 +++++++++------ test/apps/epoll/poll_err.c | 44 ++++++++++++++++++++++++++++++++++++ test/apps/scripts/fs.sh | 1 + 4 files changed, 84 insertions(+), 11 deletions(-) create mode 100644 test/apps/epoll/poll_err.c diff --git a/kernel/src/syscall/poll.rs b/kernel/src/syscall/poll.rs index d7cfc668..8dbe2f63 100644 --- a/kernel/src/syscall/poll.rs +++ b/kernel/src/syscall/poll.rs @@ -56,7 +56,13 @@ pub fn sys_poll(fds: Vaddr, nfds: u64, timeout: i32, ctx: &Context) -> Result, ctx: &Context) -> Result { - let files = hold_files(poll_fds, ctx)?; + let (result, files) = hold_files(poll_fds, ctx); + match result { + FileResult::AllValid => (), + FileResult::SomeInvalid => { + return Ok(count_all_events(poll_fds, &files)); + } + } let poller = match register_poller(poll_fds, files.as_ref()) { PollerResult::AllRegistered(poller) => poller, @@ -89,11 +95,17 @@ pub fn do_poll(poll_fds: &[PollFd], timeout: Option, ctx: &Context) -> } } +enum FileResult { + AllValid, + SomeInvalid, +} + /// Holds all the files we're going to poll. -fn hold_files(poll_fds: &[PollFd], ctx: &Context) -> Result>>> { +fn hold_files(poll_fds: &[PollFd], ctx: &Context) -> (FileResult, Vec>>) { let file_table = ctx.process.file_table().lock(); let mut files = Vec::with_capacity(poll_fds.len()); + let mut result = FileResult::AllValid; for poll_fd in poll_fds.iter() { let Some(fd) = poll_fd.fd() else { @@ -101,10 +113,18 @@ fn hold_files(poll_fds: &[PollFd], ctx: &Context) -> Result>]) -> for (poll_fd, file) in poll_fds.iter().zip(files.iter()) { let Some(file) = file else { + if !poll_fd.revents.get().is_empty() { + // This is only possible for POLLNVAL. + counter += 1; + } continue; }; diff --git a/kernel/src/syscall/select.rs b/kernel/src/syscall/select.rs index 7a073f12..3ad3e157 100644 --- a/kernel/src/syscall/select.rs +++ b/kernel/src/syscall/select.rs @@ -146,7 +146,7 @@ fn do_select( for poll_fd in &poll_fds { let fd = poll_fd.fd().unwrap(); let revents = poll_fd.revents().get(); - let (readable, writable, except) = convert_events_to_rwe(&revents); + let (readable, writable, except) = convert_events_to_rwe(revents)?; if let Some(ref mut fds) = readfds && readable { @@ -169,8 +169,8 @@ fn do_select( Ok(total_revents) } -// Convert select's rwe input to poll's IoEvents input according to Linux's -// behavior. +/// Converts `select` RWE input to `poll` I/O event input +/// according to Linux's behavior. fn convert_rwe_to_events(readable: bool, writable: bool, except: bool) -> IoEvents { let mut events = IoEvents::empty(); if readable { @@ -185,13 +185,17 @@ fn convert_rwe_to_events(readable: bool, writable: bool, except: bool) -> IoEven events } -// Convert poll's IoEvents results to select's rwe results according to Linux's -// behavior. -fn convert_events_to_rwe(events: &IoEvents) -> (bool, bool, bool) { +/// Converts `poll` I/O event results to `select` RWE results +/// according to Linux's behavior. +fn convert_events_to_rwe(events: IoEvents) -> Result<(bool, bool, bool)> { + if events.contains(IoEvents::NVAL) { + return_errno_with_message!(Errno::EBADF, "the file descriptor is invalid"); + } + let readable = events.intersects(IoEvents::IN | IoEvents::HUP | IoEvents::ERR); let writable = events.intersects(IoEvents::OUT | IoEvents::ERR); let except = events.contains(IoEvents::PRI); - (readable, writable, except) + Ok((readable, writable, except)) } const FD_SETSIZE: usize = 1024; diff --git a/test/apps/epoll/poll_err.c b/test/apps/epoll/poll_err.c new file mode 100644 index 00000000..de9f1729 --- /dev/null +++ b/test/apps/epoll/poll_err.c @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: MPL-2.0 + +#include "../network/test.h" +#include +#include + +FN_TEST(poll_nval) +{ + int fildes[2]; + int rfd, wfd; + struct pollfd fds[3]; + + TEST_SUCC(pipe(fildes)); + rfd = fildes[0]; + wfd = fildes[1]; + TEST_SUCC(write(wfd, "", 1)); + + fds[0].fd = rfd; + fds[1].fd = 1000; + fds[2].fd = wfd; + + fds[0].events = POLLIN | POLLOUT; + fds[1].events = POLLIN | POLLOUT; + fds[2].events = POLLIN | POLLOUT; + + TEST_RES(poll(fds, 3, 0), _ret == 3 && fds[0].revents == POLLIN && + fds[1].revents == POLLNVAL && + fds[2].revents == POLLOUT); + + TEST_SUCC(close(rfd)); + TEST_SUCC(close(wfd)); +} +END_TEST() + +FN_TEST(select_bafd) +{ + fd_set rfds; + + FD_ZERO(&rfds); + FD_SET(100, &rfds); + + TEST_ERRNO(select(200, &rfds, NULL, NULL, NULL), EBADF); +} +END_TEST() diff --git a/test/apps/scripts/fs.sh b/test/apps/scripts/fs.sh index 55c6dda5..3df1597f 100755 --- a/test/apps/scripts/fs.sh +++ b/test/apps/scripts/fs.sh @@ -64,3 +64,4 @@ echo "All fdatasync test passed." pipe/pipe_err pipe/short_rw epoll/epoll_err +epoll/poll_err