Merge pull request #10501 from harisibrar1/fix/pselect-pending-signal-eintr

fix(libkernel): return EINTR from pselect when its mask unblocks a pending signal

core/libs/prx/libkernel/Socket/src/Pselect.cpp

@@ -1,31 +1,69 @@
 #include "prx/libkernel/include/Api.hpp"
 #include "prx/libc/include/Api.hpp"
 #include "prx/libc/include/general/VabiMacros.hpp"
+#include <array>
 #include <cstdint>
+#include <cstring>
+
+namespace {
+
+constexpr int SetMask = 3;
+constexpr int Eintr = 4;
+constexpr int DescriptorLimit = 1024;
+
+struct Timeval {
+    std::int64_t seconds;
+    std::int64_t microseconds;
+};
+
+using SavedSet = std::array<std::uint64_t, DescriptorLimit / 64>;
+
+void Save(SavedSet& saved, const void* set, std::size_t bytes) {
+    if (set != nullptr) std::memcpy(saved.data(), set, bytes);
+}
+
+void Restore(void* set, const SavedSet& saved, std::size_t bytes) {
+    if (set != nullptr) std::memcpy(set, saved.data(), bytes);
+}
+
+int ScanAfterCaughtSignal(int nfds, void* readfds, void* writefds, void* exceptfds, const Timeval* wait) {
+    const bool validWait = wait == nullptr || (wait->seconds >= 0 && wait->microseconds >= 0 && wait->microseconds < 1000000);
+    if (nfds < 0 || nfds > DescriptorLimit || !validWait) return select_nid_postfix(nfds, readfds, writefds, exceptfds, wait);
+    const std::size_t bytes = static_cast<std::size_t>((nfds + 63) / 64) * sizeof(std::uint64_t);
+    SavedSet read{}, write{}, except{};
+    Save(read, readfds, bytes);
+    Save(write, writefds, bytes);
+    Save(except, exceptfds, bytes);
+    const Timeval zero{};
+    const int result = select_nid_postfix(nfds, readfds, writefds, exceptfds, &zero);
+    if (result != 0 || (wait != nullptr && wait->seconds == 0 && wait->microseconds == 0)) return result;
+    Restore(readfds, read, bytes);
+    Restore(writefds, write, bytes);
+    Restore(exceptfds, except, bytes);
+    *__error_nid_postfix() = Eintr;
+    return -1;
+}
+
+}
 
 extern "C" {
 
 int APS5_VABI pselect_nid_postfix(int nfds, void* readfds, void* writefds, void* exceptfds, const void* timeout, const void* sigmask) {
-    constexpr int SetMask = 3;
-    struct Timeval {
-        std::int64_t seconds;
-        std::int64_t microseconds;
-    } limit{};
+    Timeval limit{};
     const Timeval* wait = nullptr;
     if (timeout != nullptr) {
         const auto* spec = static_cast<const std::int64_t*>(timeout);
         limit = {spec[0], spec[1] / 1000};
         wait = &limit;
     }
-    std::uint32_t previous[4]{};
-    if (sigmask != nullptr && sigprocmask_nid_postfix(SetMask, sigmask, previous) != 0) return -1;
-    const int result = select_nid_postfix(nfds, readfds, writefds, exceptfds, wait);
-    if (sigmask != nullptr) {
-        const int error = *__error_nid_postfix();
-        sigprocmask_nid_postfix(SetMask, previous, nullptr);
-
-        *__error_nid_postfix() = error;
-    }
+    if (sigmask == nullptr) return select_nid_postfix(nfds, readfds, writefds, exceptfds, wait);
+    GuestSignalSet previous{};
+    const bool caught = SetSignalMaskCatching_nid_no_patch(static_cast<const GuestSignalSet*>(sigmask), &previous) != 0;
+    const int result = caught ? ScanAfterCaughtSignal(nfds, readfds, writefds, exceptfds, wait)
+                              : select_nid_postfix(nfds, readfds, writefds, exceptfds, wait);
+    const int error = *__error_nid_postfix();
+    sigprocmask_nid_postfix(SetMask, &previous, nullptr);
+    *__error_nid_postfix() = error;
     return result;
 }
 

core/libs/prx/libkernel/System/src/Signals.cpp

@@ -298,11 +298,13 @@ bool DeliverPendingSignal(int guest) {
     return false;
 }
 
-void DeliverUnblockedPending() {
+bool DeliverUnblockedPending() {
+    bool caught = false;
     for (int guest = 1; guest <= MaxSignal; ++guest) {
         if (SignalBlocked(guest) || !Deliverable(guest) || !TakePending(guest)) continue;
-        DeliverPendingSignal(guest);
+        if (DeliverPendingSignal(guest)) caught = true;
     }
+    return caught;
 }
 
 int WaitForPendingInSet(const GuestSignalSet& set, bool waitForever, const KernelTimespec* timeout, int* guest) {
@@ -540,7 +542,7 @@ thread_local GuestStack alternateStack{nullptr, 0, SsDisable};
 
 namespace {
 
-int ChangeBlockedMask(int how, const GuestSignalSet* set, GuestSignalSet* previousSet, bool releaseHolds) {
+int ChangeBlockedMask(int how, const GuestSignalSet* set, GuestSignalSet* previousSet, bool releaseHolds, bool* caught = nullptr) {
     if (set != nullptr && (how < 1 || how > 3)) { *__error_nid_postfix() = 22; return -1; }
     {
         std::lock_guard lock(registration);
@@ -575,12 +577,18 @@ int ChangeBlockedMask(int how, const GuestSignalSet* set, GuestSignalSet* previo
         }
     }
     std::atomic_thread_fence(std::memory_order_seq_cst);
-    DeliverUnblockedPending();
+    if (DeliverUnblockedPending() && caught != nullptr) *caught = true;
     return 0;
 }
 
 }
 
+extern "C" int APS5_VABI SetSignalMaskCatching_nid_no_patch(const GuestSignalSet* set, GuestSignalSet* previousSet) {
+    bool caught = false;
+    ChangeBlockedMask(GuestSigsetmask, set, previousSet, true, &caught);
+    return caught ? 1 : 0;
+}
+
 extern "C" {
 GuestHandler APS5_VABI signal_nid_postfix(int guest, GuestHandler handler) {
     const auto invalid = reinterpret_cast<GuestHandler>(static_cast<std::uintptr_t>(-1));

core/libs/prx/libkernel/include/Api.hpp

@@ -729,6 +729,7 @@ int APS5_VABI killpg_nid_postfix(int pgrp, int guest);
 int APS5_VABI pthread_kill_nid_postfix(Pthread thread, int guest);
 int APS5_VABI sigaltstack_nid_postfix(const GuestStack* stack, GuestStack* previous);
 int APS5_VABI _sigprocmask_nid_postfix(int how, const GuestSignalSet* set, GuestSignalSet* previousSet);
+int APS5_VABI SetSignalMaskCatching_nid_no_patch(const GuestSignalSet* set, GuestSignalSet* previousSet);
 int APS5_VABI sigprocmask_nid_postfix(int how, const void* set, void* previousSet);
 int APS5_VABI pthread_sigmask_nid_postfix(int how, const GuestSignalSet* set, GuestSignalSet* previousSet);
 int APS5_VABI sigtimedwait_nid_postfix(const GuestSignalSet* set, void* info, const KernelTimespec* timeout);

core/libs/tests/GuestKernelSysPselect.cpp

@@ -12,6 +12,7 @@
 
 namespace {
 
+constexpr int Eintr = 4;
 constexpr int Einval = 22;
 constexpr int SigBlock = 1;
 constexpr int SigSetmask = 3;
@@ -52,6 +53,17 @@ bool Same(const SignalSet& left, const SignalSet& right) {
     return std::memcmp(left.bits, right.bits, sizeof(left.bits)) == 0;
 }
 
+int caught = 0;
+
+void APS5_VABI Count(int) {
+    ++caught;
+}
+
+void RaiseBlocked(const SignalSet& blocked) {
+    Require(sigprocmask_nid_postfix(SigBlock, &blocked, nullptr) == 0);
+    Require(raise_nid_postfix(SigUsr1) == 0 && caught == 0);
+}
+
 }
 
 int main() {
@@ -94,6 +106,60 @@ int main() {
         const Timespec zero{0, 0};
         Require(pselect_nid_postfix(-1, nullptr, nullptr, nullptr, &zero, nullptr) == -1 && Error() == Einval);
     }
+    Require(signal_nid_postfix(SigUsr1, Count) != reinterpret_cast<GuestHandler>(static_cast<std::uintptr_t>(-1)));
+    const SignalSet empty{};
+    {
+        RaiseBlocked(blocked);
+        const SignalSet withUsr1 = CurrentMask();
+        DescriptorSet read{};
+        read.Add(writer);
+        const DescriptorSet original = read;
+        const Timespec limit{0, 300000000};
+        const auto start = std::chrono::steady_clock::now();
+        Require(pselect_nid_postfix(writer + 1, read.words.data(), nullptr, nullptr, &limit, &empty) == -1 && Error() == Eintr);
+        Require(std::chrono::steady_clock::now() - start < std::chrono::milliseconds(150));
+        Require(caught == 1);
+        Require(read.words == original.words);
+        Require(Same(CurrentMask(), withUsr1));
+        caught = 0;
+    }
+    {
+        RaiseBlocked(blocked);
+        Require(pselect_nid_postfix(0, nullptr, nullptr, nullptr, nullptr, &empty) == -1 && Error() == Eintr && caught == 1);
+        caught = 0;
+    }
+    {
+        RaiseBlocked(blocked);
+        DescriptorSet read{};
+        read.Add(reader);
+        const Timespec limit{0, 300000000};
+        Require(pselect_nid_postfix(reader + 1, read.words.data(), nullptr, nullptr, &limit, &empty) == 1 && caught == 1);
+        Require(read.Has(reader));
+        caught = 0;
+    }
+    {
+        RaiseBlocked(blocked);
+        const Timespec zero{0, 0};
+        DescriptorSet read{};
+        read.Add(writer);
+        Require(pselect_nid_postfix(writer + 1, read.words.data(), nullptr, nullptr, &zero, &empty) == 0 && caught == 1);
+        caught = 0;
+    }
+    {
+        RaiseBlocked(blocked);
+        const Timespec invalid{0, 1000000000};
+        Require(pselect_nid_postfix(0, nullptr, nullptr, nullptr, &invalid, &empty) == -1 && Error() == Einval && caught == 1);
+        caught = 0;
+    }
+    {
+        RaiseBlocked(blocked);
+        const Timespec limit{0, 200000000};
+        const auto start = std::chrono::steady_clock::now();
+        Require(pselect_nid_postfix(0, nullptr, nullptr, nullptr, &limit, &blocked) == 0 && caught == 0);
+        Require(std::chrono::steady_clock::now() - start >= std::chrono::milliseconds(150));
+        Require(sigprocmask_nid_postfix(SigSetmask, &empty, nullptr) == 0 && caught == 1);
+        caught = 0;
+    }
     Require(sigprocmask_nid_postfix(SigSetmask, &before, nullptr) == 0);
     Require(close_nid_postfix(reader) == 0 && close_nid_postfix(writer) == 0);
     std::puts("pselect passed");

docs/dev/debt/libkernel.md

@@ -172,7 +172,7 @@
 - [sceKernelGetFsSandboxRandomWord](../../../core/libs/prx/libkernel/System/src/Sandbox.cpp) (libkernel) returns a fixed directory name, `gkbHtXc3Wq`. On the console the name is the process's randomized path (syscall 602, `randomized_path`), and the sandbox exposes system files under it, such as `common/lib` and `common/cert`. Nothing is mounted under the name here, so `/gkbHtXc3Wq/common/...` resolves under the guest root like any other path and does not exist, except `/gkbHtXc3Wq/common/cert/CA_LIST.cer`, which [SandboxCertificates.cpp](../../../core/libs/prx/libkernel/File/src/SandboxCertificates.cpp) serves from the host trust store
 - [ktimer_create, ktimer_settime, ktimer_gettime, ktimer_getoverrun, ktimer_delete](../../../core/libs/prx/libkernel/Time/Ktimer.cpp) (libkernel) - a `SIGEV_SIGNAL` or `SIGEV_THREAD_ID` timer, and one created with a null `sigevent` (FreeBSD defaults it to `SIGEV_SIGNAL`), throws, because delivering a guest signal from a timer is not modelled. A non-zero value under one tick rounds up to 1 ms, as FreeBSD 11 does at hz 1000; the console's hz is not measured (`kern.clockrate` is unanswered). `SIGEV_NONE` timers on `CLOCK_REALTIME` and `CLOCK_MONOTONIC` follow `kern_time.c`: ids 3 to 31 (`EAGAIN` when full), `EINVAL` for another clock, a bad notify type or signal, an id that is not live or a bad `itimerspec` field, and `TIMER_ABSTIME` (1). Expiry is worked out from the guest clock when the timer is read, and `ktimer_getoverrun` returns 0, as FreeBSD only updates it when a signal is accepted
 - [The sandbox CA list](../../../core/libs/prx/libkernel/File/src/SandboxCertificates.cpp) (libkernel), `/<sandbox word>/common/cert/CA_LIST.cer`, is a PEM bundle of the host's trust anchors, written once per process to `cert_cache/CA_LIST.cer` in the run directory with the owner write bit cleared: on Windows the current user's `ROOT` store, filtered through Disallowed and the combined EKU extension/property for TLS server authentication (Windows fetches missing roots on demand, so a root that was never fetched is absent), elsewhere `SSL_CERT_FILE` or the first of `/etc/ssl/certs/ca-certificates.crt`, `/etc/pki/tls/certs/ca-bundle.crt`, `/etc/ssl/ca-bundle.pem` and `/etc/ssl/cert.pem` that exists. On macOS, `/etc/ssl/cert.pem` is a static snapshot and does not reflect roots added to or distrusted in the Keychain. Existing guest files, explicit aliases and blocked mounts take precedence over the provider. The PEM snapshot does not carry Windows CTL restrictions beyond EKU, native revocation policy or later trust-store changes. Without any certificate the path does not exist. The console file's format is unverified: only that bundled OpenSSL accepts a PEM bundle there is shown (ProsperoRadio PPSA99001 and SwanStation PPSA98510, #5001). The other entries of the console's `common/cert` directory, the directory itself (it does not exist here) and the error a write open gets (`EACCES` from the host here) are unverified
-- [pselect](../../../core/libs/prx/libkernel/Socket/src/Pselect.cpp) (libkernel) sets the signal mask with `sigprocmask`, calls `select` and restores the mask, as FreeBSD 11 `kern_pselect` does, but the swap is not atomic with the wait: a signal that becomes deliverable between the swap and the wait is not guaranteed to interrupt it
+- [pselect](../../../core/libs/prx/libkernel/Socket/src/Pselect.cpp) (libkernel) sets the signal mask, calls `select` and restores the mask, as FreeBSD 11 `kern_pselect` does. A pending signal that the new mask unblocks has its handler run during the swap, and `pselect` then returns `EINTR` unless a descriptor is already ready, the timeout is zero or an argument is invalid, as FreeBSD's `kern_select` decides before it sleeps. A signal raised during the wait, including one sent by another thread, does not interrupt it, because `select` has no `EINTR` path; and the mask is the process-wide one, so it applies to every thread for the duration of the call
 - [getcontext and setcontext](../../../core/libs/prx/libkernel/System/src/UserContext.cpp) (libkernel) and [makecontext](../../../core/libs/prx/libSceLibcInternal/UserContext.cpp) (libSceLibcInternal) use FreeBSD 11's amd64 `ucontext_t` (`mcontext` at 0x10, `mc_len` 800, `uc_link` at 0x330, `uc_stack` at 0x338, 0x370 bytes), the layout homebrew built against FreeBSD's headers passes: PPSA99039's OpenSSL `ASYNC` (`crypto/async/arch/async_posix.c`) captures into a 0x370-byte stack buffer. `getcontext` writes the 0x330 bytes before `uc_link`, as FreeBSD's `UC_COPY_SIZE`. The console's own layout for these functions is not verified; the exception handler's context is a different one (see `sceKernelRaiseException`)
 - [getcontext and setcontext](../../../core/libs/prx/libkernel/System/src/UserContext.cpp) (libkernel) keep the 512-byte FXSAVE image in `mc_fpstate` (x87, MXCSR, XMM0-15), so the upper halves of YMM0-15 are neither saved nor restored, and `getcontext` leaves `mc_xfpustate` and `mc_xfpustate_len` 0. `getcontext` sets `mc_fpformat` and `mc_ownedfp` to FreeBSD's `_MC_FPFMT_XMM` and `_MC_FPOWNED_FPU`, `mc_cs` and `mc_ss` to the host selectors, and `mc_flags`, `mc_fsbase` and `mc_gsbase` to 0. `setcontext` loads `mc_fpstate` whatever `mc_fpformat` and `mc_ownedfp` say, and restores neither the segment selectors, `mc_fsbase` and `mc_gsbase` nor the extended state, which FreeBSD restores when `mc_flags` asks for them
 - [setcontext](../../../core/libs/prx/libkernel/System/src/UserContext.cpp) (libkernel) and [longjmp](../../../core/libs/prx/libc/src/JumpBuffer.cpp) (libc) on Windows set the thread's stack bounds in the TEB to the stack they switch to: a `makecontext` stack, or else the committed region around the target stack pointer. A switch to any other guest stack, such as a context built by hand, gets the bounds of that region only