Skip to content

Stop pselect6's fallback hiding a ready descriptor - #318

Open
xalestar wants to merge 1 commit into
sysprog21:mainfrom
xalestar:pselect-fallback-pollnval
Open

Stop pselect6's fallback hiding a ready descriptor#318
xalestar wants to merge 1 commit into
sysprog21:mainfrom
xalestar:pselect-fallback-pollnval

Conversation

@xalestar

@xalestar xalestar commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

pselect6 falls back to poll() when a host descriptor is out of the fd_set range. The descriptors that macOS poll() refuses (/dev/null, /dev/zero, /dev/urandom, directories, and kqueue fds) then reach the guest as not ready, whereas Linux select() reports all of them as ready. The fallback routes those entries through poll_eval_unpollable(), the helper sys_ppoll() already uses for the same descriptor set.

Reproduction is the new "pselect always-ready device at a high fd" case in tests/test-poll.c, which fails on main and passes here. make check is clean, make lint is unchanged, and tests/test-matrix.sh passes all within baseline with zero failures (elfuse-aarch64 255, qemu-aarch64 234, elfuse-x86_64 78).


Summary by cubic

Fixes pselect6’s poll fallback so it no longer hides readiness for descriptors that macOS poll() refuses. Previously, when a host fd was >= FD_SETSIZE, we fell back to poll and copied POLLNVAL for /dev/null, /dev/zero, /dev/urandom, directories, and kqueue fds, which made the guest see them as not ready; now these entries are handled via select and reported ready like Linux select/ppoll.

Review notes

  • Fallback takes POLLNVAL entries out of the poll set, runs them through poll_eval_unpollable(), merges revents into the result, and adjusts the return count; a single restart occurs when the refused set is first discovered.
  • Introduces small helpers/structs to contain this logic (poll_unpollable_t, pselect_fallback_t, pselect_fallback_pass); behavior outside the fallback is unchanged.
  • Wakeup pipe behavior is preserved; zero-timeout returns immediately if any refused entries are ready.
  • Adds tests: “pselect always-ready devices” and “pselect always-ready device at a high fd”; both fail on main and pass here.

Written for commit 54e9703. Summary will update on new commits.

Review in cubic

pselect6 leaves fd_set behind and waits in poll() when a host
descriptor lands at or above FD_SETSIZE. macOS poll() answers POLLNVAL
for every descriptor it will not put on a kqueue: /dev/null, /dev/zero,
/dev/random, /dev/urandom, directories, and kqueue descriptors
themselves. The fallback copies that revents into the request, where
POLLNVAL maps to no fd_set bit, so a guest whose descriptor table is
close to full is told a live /dev/null is not ready while the return
count still counts it. Linux select() reports every one of them ready.

poll_eval_unpollable() answers those entries for sys_ppoll already. The
fallback takes them out of the poll set on the pass that exposes them
and hands them to the same function, so the rule has one copy. A
refused entry makes poll() return at once, so that single restart waits
for nothing.

Reaching the fallback needs no descriptor pressure beyond a high
number. host_fd_ref_open_io() borrows the host descriptor instead of
duplicating it while one thread is active, so a single-threaded guest
gets there as soon as a borrowed descriptor is numbered at or above
FD_SETSIZE.

The poll pass moves into pselect_fallback_pass() with its state in one
struct, since sys_pselect6 crosses the 400-line clang-tidy advisory
otherwise.

Verified: tests/test-poll.c carries two new cases and reports 21
passed, 0 failed both under elfuse and inside the qemu-system-aarch64
reference VM, and its high-descriptor case fails against main as it
stands. make check is clean, make lint reports no new finding, and
tests/test-matrix.sh all stays within baseline with zero failures:
elfuse-aarch64 255 passed, qemu-aarch64 234 passed, elfuse-x86_64 78
passed.
cubic-dev-ai[bot]

This comment was marked as resolved.

Comment thread src/syscall/poll.c
for (int i = 0; i < req_count; i++)
if (unp[i].fd >= 0)
reqs[i].revents = unp[i].revents;
ret += (int) fb.ready;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fb.ready counts refused entries, one per descriptor, but select's return value is the number of bits set across the three sets. The write-back below lights both the read and the write bit for a descriptor like /dev/null, so ret comes back one short. The direct pselect path returns the host's bit count, which is what the new "pselect always-ready devices" case asserts with r == 2 * opened; the same guest program now gets 2 for a low host fd and 1 for a high one. Count each refused entry once per bit it sets (read, write, except) instead of once per entry.

Comment thread src/syscall/poll.c
}
free(poll_heap);
bool restart;
ret = pselect_fallback_pass(&fb, has_timeout ? &ts : &poll_ts,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the pass fails its allocation it returns -1 with ENOMEM, but control now falls into the three interrupt predicates before save_errno = errno, and any of them can overwrite errno. The inlined code broke out of the loop at that point. Break out here too when the pass reports an allocation failure.

Comment thread tests/test-poll.c
TEST("pselect always-ready device at a high fd");
{
static int fillers[FD_SETSIZE];
int n = 0, subject = -1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop also breaks when open() returns -1, and subject is then a low descriptor whose host fd stays under FD_SETSIZE. The assertions still pass, so a run under a low RLIMIT_NOFILE reports green without ever entering the fallback this case exists to cover. Record which of the two breaks fired and fail loudly when the table never filled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants