kern: restore signal mask before ast() for pselect/ppoll
Some checks are pending
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-14, /usr/lib/llvm-14/bin, ubuntu-22.04, bmake libarchive-dev clang-14 lld-14, amd64, amd64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-14, /usr/lib/llvm-14/bin, ubuntu-22.04, bmake libarchive-dev clang-14 lld-14, arm64, aarch64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-18, /opt/homebrew/opt/llvm@18/bin, macos-latest, bmake libarchive llvm@18, amd64, amd64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-18, /opt/homebrew/opt/llvm@18/bin, macos-latest, bmake libarchive llvm@18, arm64, aarch64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-18, /usr/lib/llvm-18/bin, ubuntu-24.04, bmake libarchive-dev clang-18 lld-18, amd64, amd64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-18, /usr/lib/llvm-18/bin, ubuntu-24.04, bmake libarchive-dev clang-18 lld-18, arm64, aarch64) (push) Waiting to run

It's possible to take a signal after pselect/ppoll have set their return
value, but before we actually return to userland.  This results in
taking a signal without reflecting it in the return value, which weakens
the guarantees provided by these functions.

Switch both to restore the signal mask before we would deliver signals
on return to userland.  If a signal was received after the wait was
over, then we'll just have the signal queued up for the next time it
comes unblocked.  The modified signal mask is retained if we were
interrupted so that ast() actually handles the signal, at which point
the signal mask is restored.

des@ has a test case demonstrating the issue in D47738 which will
follow.

Note for MFC: TDA_PSELECT is a KBI break, we should just inline
ast_sigsuspend() in pselect/ppoll for stable branches.  It's not exactly
the same, but it will be close enough.

Reported by:	des
Reviewed by:	des (earlier version), kib
Sponsored by:	Klara, Inc.
Sponsored by:	NetApp, Inc.
Differential Revision:	https://reviews.freebsd.org/D47741
This commit is contained in:
Kyle Evans 2024-11-25 22:04:48 -06:00
parent cab31f5633
commit ccb973da1f

View File

@ -1049,14 +1049,26 @@ kern_pselect(struct thread *td, int nd, fd_set *in, fd_set *ou, fd_set *ex,
if (error != 0)
return (error);
td->td_pflags |= TDP_OLDMASK;
}
error = kern_select(td, nd, in, ou, ex, tvp, abi_nfdbits);
if (uset != NULL) {
/*
* Make sure that ast() is called on return to
* usermode and TDP_OLDMASK is cleared, restoring old
* sigmask.
* sigmask. If we didn't get interrupted, then the caller is
* likely not expecting a signal to hit that should normally be
* blocked by its signal mask, so we restore the mask before
* any signals could be delivered.
*/
ast_sched(td, TDA_SIGSUSPEND);
if (error == EINTR) {
ast_sched(td, TDA_SIGSUSPEND);
} else {
/* *select(2) should never restart. */
MPASS(error != ERESTART);
ast_sched(td, TDA_PSELECT);
}
}
error = kern_select(td, nd, in, ou, ex, tvp, abi_nfdbits);
return (error);
}
@ -1528,12 +1540,6 @@ kern_poll_kfds(struct thread *td, struct pollfd *kfds, u_int nfds,
if (error)
return (error);
td->td_pflags |= TDP_OLDMASK;
/*
* Make sure that ast() is called on return to
* usermode and TDP_OLDMASK is cleared, restoring old
* sigmask.
*/
ast_sched(td, TDA_SIGSUSPEND);
}
seltdinit(td);
@ -1556,6 +1562,22 @@ kern_poll_kfds(struct thread *td, struct pollfd *kfds, u_int nfds,
error = EINTR;
if (error == EWOULDBLOCK)
error = 0;
if (uset != NULL) {
/*
* Make sure that ast() is called on return to
* usermode and TDP_OLDMASK is cleared, restoring old
* sigmask. If we didn't get interrupted, then the caller is
* likely not expecting a signal to hit that should normally be
* blocked by its signal mask, so we restore the mask before
* any signals could be delivered.
*/
if (error == EINTR)
ast_sched(td, TDA_SIGSUSPEND);
else
ast_sched(td, TDA_PSELECT);
}
return (error);
}