fix(libkernel): fail the time getters and setitimer with EFAULT on an unmapped pointer

core/libs/prx/libc/include/General.hpp

@@ -31,6 +31,7 @@ extern "C" int CopyGuestPath_nid_no_patch(const char* source, std::string& desti
 
 extern "C" std::filesystem::path ResolvePath_nid_no_patch(const char* path);
 extern "C" bool GuestCopyOut_nid_no_patch(void* destination, const void* source, std::size_t bytes);
+extern "C" bool GuestCopyIn_nid_no_patch(void* destination, const void* source, std::size_t bytes);
 extern "C" std::filesystem::path ResolvePathOrRelinkedModule_nid_no_patch(const char* path);
 extern "C" std::filesystem::path ResolveArchivedPath_nid_no_patch(const char* path, bool module = false);
 extern "C" std::filesystem::path ResolveDiskPath_nid_no_patch(const char* path);

core/libs/prx/libc/src/General.cpp

@@ -837,6 +837,32 @@ extern "C" bool GuestCopyOut_nid_no_patch(void* destination, const void* source,
 #endif
 }
 
+extern "C" bool GuestCopyIn_nid_no_patch(void* destination, const void* source, std::size_t bytes) {
+    if (bytes == 0) return true;
+    const auto start = reinterpret_cast<std::uintptr_t>(source);
+    if (start == 0 || bytes > UINTPTR_MAX - start) return false;
+    GuestArena::GuestArenaPrepareHostRead_nid_no_patch(source, bytes);
+#ifdef _WIN32
+    SIZE_T copied = 0;
+    return ReadProcessMemory(GetCurrentProcess(), source, destination, bytes, &copied) != 0 && copied == bytes;
+#elif defined(__linux__)
+    iovec local{destination, bytes};
+    iovec remote{const_cast<void*>(source), bytes};
+    const ssize_t copied = ::process_vm_readv(::getpid(), &local, 1, &remote, 1, 0);
+    if (copied < 0) {
+        if (errno == EFAULT) return false;
+        throw std::runtime_error("GuestCopyIn: process_vm_readv failed with errno " + std::to_string(errno));
+    }
+    return static_cast<std::size_t>(copied) == bytes;
+#elif defined(__APPLE__)
+    mach_vm_size_t copied = 0;
+    return ::mach_vm_read_overwrite(::mach_task_self(), reinterpret_cast<mach_vm_address_t>(source), bytes,
+        reinterpret_cast<mach_vm_address_t>(destination), &copied) == KERN_SUCCESS && copied == bytes;
+#else
+    throw std::runtime_error("GuestCopyIn: no checked read is available on this host");
+#endif
+}
+
 extern "C" std::filesystem::path ResolvePath_nid_no_patch(const char* path) {
     auto resolved = ResolveLocked(path, false, true);
     AppArchiveRefuseOnly_nid_no_patch(resolved, path, "reading");

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

@@ -368,12 +368,12 @@ extern "C" int APS5_VABI getitimer_nid_postfix(int which, GuestItimerval* output
         *__error_nid_postfix() = 22;
         return -1;
     }
-    if (output == nullptr) {
-        *__error_nid_postfix() = 14;
-        return -1;
-    }
     if (which != 0) {
-        *output = {};
+        constexpr GuestItimerval none{};
+        if (!GuestCopyOut_nid_no_patch(output, &none, sizeof(none))) {
+            *__error_nid_postfix() = 14;
+            return -1;
+        }
         return 0;
     }
     GuestItimerval result{};
@@ -390,7 +390,10 @@ extern "C" int APS5_VABI getitimer_nid_postfix(int which, GuestItimerval* output
         result.interval.tv_sec = interval / 1000000;
         result.interval.tv_usec = interval % 1000000;
     }
-    *output = result;
+    if (!GuestCopyOut_nid_no_patch(output, &result, sizeof(result))) {
+        *__error_nid_postfix() = 14;
+        return -1;
+    }
     return 0;
 }
 
@@ -421,15 +424,16 @@ extern "C" int APS5_VABI setitimer_nid_postfix(int which, const GuestItimerval*
     if (value == nullptr && previous == nullptr) { *__error_nid_postfix() = 14; return -1; }
     GuestItimerval requested{};
     if (value != nullptr) {
-        requested = *value;
+        if (!GuestCopyIn_nid_no_patch(&requested, value, sizeof(requested))) { *__error_nid_postfix() = 14; return -1; }
         if (!ValidItimer(requested.value)) { *__error_nid_postfix() = 22; return -1; }
         if (!ItimerSet(requested.value)) requested.interval = {};
         else if (!ValidItimer(requested.interval)) { *__error_nid_postfix() = 22; return -1; }
     }
     if (which != ItimerReal) {
         if (ItimerSet(requested.value))
             throw std::runtime_error(std::string("setitimer: ") + (which == ItimerProf ? "ITIMER_PROF" : "ITIMER_VIRTUAL") + " needs process CPU time accounting, which is not implemented");
-        if (previous != nullptr) *previous = {};
+        constexpr GuestItimerval none{};
+        if (previous != nullptr && !GuestCopyOut_nid_no_patch(previous, &none, sizeof(none))) { *__error_nid_postfix() = 14; return -1; }
         return 0;
     }
     if (requested.interval.tv_sec == 0 && requested.interval.tv_usec != 0 && requested.interval.tv_usec < MinExactIntervalMicros)
@@ -448,7 +452,7 @@ extern "C" int APS5_VABI setitimer_nid_postfix(int which, const GuestItimerval*
             NotifyAlarms(alarms);
         }
     }
-    if (previous != nullptr) *previous = old;
+    if (previous != nullptr && !GuestCopyOut_nid_no_patch(previous, &old, sizeof(old))) { *__error_nid_postfix() = 14; return -1; }
     return 0;
 }
 

core/libs/prx/libkernel/Time/Time.cpp

@@ -501,27 +501,31 @@ int APS5_VABI clock_gettime_nid_postfix(int clockId, KernelTimespec* tp) {
     return 0;
 }
 
-int APS5_VABI gettimeofday_nid_postfix(KernelTimeval* tv, KernelTimezone* tz) {
-    KernelTimeval ignoredTime{};
-    if (tv == nullptr) tv = &ignoredTime;
+static bool ReadTimeOfDay(KernelTimeval& tv) {
 #ifdef _WIN32
     FILETIME ft{};
     GetSystemTimePreciseAsFileTime(&ft);
     std::uint64_t t = (static_cast<std::uint64_t>(ft.dwHighDateTime) << 32) | ft.dwLowDateTime;
     t -= 116444736000000000ULL;
-    tv->tv_sec = static_cast<std::int64_t>(t / 10000000ULL);
-    tv->tv_usec = static_cast<std::int64_t>((t % 10000000ULL) / 10ULL);
+    tv.tv_sec = static_cast<std::int64_t>(t / 10000000ULL);
+    tv.tv_usec = static_cast<std::int64_t>((t % 10000000ULL) / 10ULL);
+    return true;
 #else
     struct timeval native{};
-    if (gettimeofday(&native, nullptr) != 0) {
-        return -1;
-    }
-    tv->tv_sec = static_cast<std::int64_t>(native.tv_sec);
-    tv->tv_usec = static_cast<std::int64_t>(native.tv_usec);
+    if (gettimeofday(&native, nullptr) != 0) return false;
+    tv.tv_sec = static_cast<std::int64_t>(native.tv_sec);
+    tv.tv_usec = static_cast<std::int64_t>(native.tv_usec);
+    return true;
 #endif
-    if (tz != nullptr) {
-        tz->tz_minuteswest = 0;
-        tz->tz_dsttime = 0;
+}
+
+int APS5_VABI gettimeofday_nid_postfix(KernelTimeval* tv, KernelTimezone* tz) {
+    KernelTimeval time{};
+    if (!ReadTimeOfDay(time)) return -1;
+    constexpr KernelTimezone zone{0, 0};
+    if ((tv != nullptr && !GuestCopyOut_nid_no_patch(tv, &time, sizeof(time))) || (tz != nullptr && !GuestCopyOut_nid_no_patch(tz, &zone, sizeof(zone)))) {
+        *__error_nid_postfix() = GUEST_EFAULT;
+        return -1;
     }
     return 0;
 }
@@ -704,14 +708,14 @@ int APS5_VABI sceKernelConvertUtcToLocaltime(int64_t utc_time, int64_t* local_ti
 }
 
 int APS5_VABI sceKernelGettimeofday(KernelTimeval* tp) {
-    if (tp == nullptr) return SceKernelErrorEfault;
-    return gettimeofday_nid_postfix(tp, nullptr);
+    KernelTimeval time{};
+    if (!ReadTimeOfDay(time)) return -1;
+    return GuestCopyOut_nid_no_patch(tp, &time, sizeof(time)) ? 0 : SceKernelErrorEfault;
 }
 
 int APS5_VABI sceKernelGettimezone(KernelTimezone* tz) {
-    if (tz == nullptr) return SceKernelErrorEfault;
-    *tz = {0, 0};
-    return 0;
+    constexpr KernelTimezone zone{0, 0};
+    return GuestCopyOut_nid_no_patch(tz, &zone, sizeof(zone)) ? 0 : SceKernelErrorEfault;
 }
 
 uint64_t APS5_VABI sceKernelReadTsc(void) {

core/libs/tests/GuestFreebsdClocks.cpp

@@ -312,6 +312,23 @@ int main() {
     KernelTimezone zone{-1, -1};
     Require(gettimeofday_nid_postfix(nullptr, nullptr) == SCE_OK);
     Require(gettimeofday_nid_postfix(nullptr, &zone) == SCE_OK && zone.tz_minuteswest == 0 && zone.tz_dsttime == 0);
+    constexpr int sceKernelErrorEfault = static_cast<int>(0x8002000e);
+    Require(sceKernelGettimeofday(nullptr) == sceKernelErrorEfault && sceKernelGettimezone(nullptr) == sceKernelErrorEfault);
+    for (const auto address : {std::uintptr_t{1}, std::uintptr_t{0xfffffffffffff000}}) {
+        auto* const unmappedTime = reinterpret_cast<KernelTimeval*>(address);
+        auto* const unmappedZone = reinterpret_cast<KernelTimezone*>(address);
+        KernelTimeval now{-1, -1};
+        *__error_nid_postfix() = 0;
+        Require(gettimeofday_nid_postfix(unmappedTime, nullptr) == -1 && *__error_nid_postfix() == 14);
+        *__error_nid_postfix() = 0;
+        Require(gettimeofday_nid_postfix(&now, unmappedZone) == -1 && *__error_nid_postfix() == 14);
+        *__error_nid_postfix() = 0;
+        Require(gettimeofday_nid_postfix(unmappedTime, &zone) == -1 && *__error_nid_postfix() == 14);
+        Require(sceKernelGettimeofday(unmappedTime) == sceKernelErrorEfault && sceKernelGettimezone(unmappedZone) == sceKernelErrorEfault);
+    }
+    KernelTimeval now{-1, -1};
+    Require(sceKernelGettimeofday(&now) == SCE_OK && now.tv_sec > 0 && now.tv_usec >= 0 && now.tv_usec < 1000000);
+    Require(sceKernelGettimezone(&zone) == SCE_OK && zone.tz_minuteswest == 0 && zone.tz_dsttime == 0);
     const int nullResolutionClocks[] = {GUEST_CLOCK_REALTIME, GUEST_CLOCK_VIRTUAL, GUEST_CLOCK_MONOTONIC, GUEST_CLOCK_SECOND, GUEST_CLOCK_PROCESS_CPUTIME_ID};
     for (const int clockId : nullResolutionClocks) Require(clock_getres_nid_postfix(clockId, nullptr) == SCE_OK);
 

core/libs/tests/GuestGetitimer.cpp

@@ -58,6 +58,13 @@ int main() {
         Require(getitimer_nid_postfix(invalid, nullptr) == -1 && *__error_nid_postfix() == 22, "invalid timer validation order changed");
     }
     Require(getitimer_nid_postfix(0, nullptr) == -1 && *__error_nid_postfix() == 14, "null output did not report EFAULT");
+    for (const auto address : {std::uintptr_t{1}, std::uintptr_t{0xfffffffffffff000}}) {
+        auto* const unmapped = reinterpret_cast<GuestItimerval*>(address);
+        for (int which : {0, 1, 2}) {
+            *__error_nid_postfix() = 0;
+            Require(getitimer_nid_postfix(which, unmapped) == -1 && *__error_nid_postfix() == 14, "unmapped output did not report EFAULT");
+        }
+    }
     for (int disarmed : {1, 2}) {
         GuestItimerval output{{11, 22}, {33, 44}};
         *__error_nid_postfix() = 12345;

core/libs/tests/GuestItimer.cpp

@@ -107,6 +107,19 @@ int main(int argc, char** argv) {
     for (int which : {0, 1, 2}) {
         *__error_nid_postfix() = 0;
         Require(setitimer_nid_postfix(which, nullptr, nullptr) == -1 && *__error_nid_postfix() == 14, "a read into null did not report EFAULT");
+        for (const auto address : {std::uintptr_t{1}, std::uintptr_t{0xfffffffffffff000}}) {
+            auto* const unmapped = reinterpret_cast<GuestItimerval*>(address);
+            GuestItimerval previous{{11, 22}, {33, 44}};
+            const auto saved = previous;
+            *__error_nid_postfix() = 0;
+            Require(setitimer_nid_postfix(which, unmapped, &previous) == -1 && *__error_nid_postfix() == 14, "an unmapped value did not report EFAULT");
+            Require(std::memcmp(&previous, &saved, sizeof(previous)) == 0, "an unmapped value changed the previous output");
+            *__error_nid_postfix() = 0;
+            Require(setitimer_nid_postfix(which, nullptr, unmapped) == -1 && *__error_nid_postfix() == 14, "an unmapped previous did not report EFAULT");
+            *__error_nid_postfix() = 0;
+            const GuestItimerval idle{};
+            Require(setitimer_nid_postfix(which, &idle, unmapped) == -1 && *__error_nid_postfix() == 14, "an unmapped previous with a value did not report EFAULT");
+        }
         for (const GuestItimerval bad : {GuestItimerval{{0, 0}, {-1, 0}}, GuestItimerval{{0, 0}, {0, -1}},
                  GuestItimerval{{0, 0}, {0, 1000000}}, GuestItimerval{{0, 0}, {INT_MAX / 2 + 1, 0}},
                  GuestItimerval{{-1, 0}, {1, 0}}, GuestItimerval{{0, 1000000}, {1, 0}}, GuestItimerval{{INT_MAX / 2 + 1, 0}, {1, 0}}})

docs/dev/debt/libkernel.md

@@ -148,7 +148,7 @@
 - [Windows file identity](../../../core/libs/prx/libkernel/File/src/NativeStat.cpp) (libkernel) - `stat` and `fstat` report the volume serial number as `st_dev` and the 64-bit file index folded into the guest's 32-bit `st_ino` (never 0), so two different files can in principle share an `st_ino`; the fold is not taken from a reference. `st_nlink` is clamped to 65535, the width of the guest field. The call keeps the `_wstat64` values when `GetFileInformationByHandle` fails
 - [lstat of a Windows symbolic link](../../../core/libs/prx/libkernel/File/src/NativeStat.cpp) (libkernel) reports a link whose target is relative, as `symlink` creates them, with the link's own times, `st_dev` and `st_ino`, mode `S_IFLNK | 0777` and the length of the target `readlink` returns as `st_size`. FreeBSD gives a new link `0777` less the creating process's umask, which the host does not keep, so the permission bits are not the console's. A link with an absolute target is reported the same way, using its print name, or its substitute name when the print name is empty, without a `\??\` or `\\?\` prefix. A directory junction is reported the same way when the name left after that prefix is a drive path (`X:\...`): the print name when that name is a drive path, otherwise the substitute name. `open` with `O_NOFOLLOW` returns `EMLINK` for that junction and does not open the target
 - [lstat of a Windows reparse point that is not a link](../../../core/libs/prx/libkernel/File/src/NativeStat.cpp) (libkernel) reports a mounted volume (`\??\Volume{GUID}\`) or another name surrogate as the reparse point itself, not followed, with the mode of its attributes, as CPython's `os.lstat` does, and a reparse point without the name surrogate bit (deduplication, cloud files) as `stat` reports it
-- [setitimer](../../../core/libs/prx/libkernel/System/src/Signals.cpp) (libkernel) follows FreeBSD's `sys_setitimer` and `kern_setitimer`: `EINVAL` for timers other than `ITIMER_REAL`, `ITIMER_VIRTUAL` and `ITIMER_PROF`, and for a value, or the interval of a non-zero value, with a negative field, `tv_usec` of 1000000 or more, or more than `INT32_MAX / 2` seconds; a null value reads the timer as `getitimer` does, with `EFAULT` when the output is null too. `ITIMER_REAL` shares the timer thread of `alarm`, which clears its interval: a zero value disarms it, the previous value is returned, and each expiry re-arms it by `it_interval`. With `SIG_IGN` an expiry is dropped. With `SIGALRM` blocked it is left on the pending set of `sigtimedwait`, as `kern_psignal` does, where `sigwait`, `sigtimedwait` and `sigwaitinfo` take it; unblocking it runs a guest handler on the unblocking thread, and with the default disposition exits the process with status 142. Unblocked, the default disposition exits the process with status 142 and a handler throws, since guest `SIGALRM` handlers are never run from the timer thread. The value is not raised to `tick / 16` as `itimerfix` does, and an interval below 1 ms throws, since the console's `hz`, and so that rounding, is unknown (1 ms covers any `hz` of 63 or more). `ITIMER_VIRTUAL` and `ITIMER_PROF` throw when armed, as process CPU time is not accounted, and read and disarm as zero. PPSA99420 imports it and fails to load without it
+- [setitimer](../../../core/libs/prx/libkernel/System/src/Signals.cpp) (libkernel) follows FreeBSD's `sys_setitimer` and `kern_setitimer`: `EINVAL` for timers other than `ITIMER_REAL`, `ITIMER_VIRTUAL` and `ITIMER_PROF`, and for a value, or the interval of a non-zero value, with a negative field, `tv_usec` of 1000000 or more, or more than `INT32_MAX / 2` seconds; a null value reads the timer as `getitimer` does, with `EFAULT` when the output is null too. A value, previous or `getitimer` output pointer into unmapped memory is `EFAULT` through the checked guest copies, as `copyin` and `copyout` fail in FreeBSD; a previous pointer that fails after the timer was set leaves it set, as `sys_setitimer` does, and `gettimeofday`, `sceKernelGettimeofday` and `sceKernelGettimezone` check their outputs the same way. `ITIMER_REAL` shares the timer thread of `alarm`, which clears its interval: a zero value disarms it, the previous value is returned, and each expiry re-arms it by `it_interval`. With `SIG_IGN` an expiry is dropped. With `SIGALRM` blocked it is left on the pending set of `sigtimedwait`, as `kern_psignal` does, where `sigwait`, `sigtimedwait` and `sigwaitinfo` take it; unblocking it runs a guest handler on the unblocking thread, and with the default disposition exits the process with status 142. Unblocked, the default disposition exits the process with status 142 and a handler throws, since guest `SIGALRM` handlers are never run from the timer thread. The value is not raised to `tick / 16` as `itimerfix` does, and an interval below 1 ms throws, since the console's `hz`, and so that rounding, is unknown (1 ms covers any `hz` of 63 or more). `ITIMER_VIRTUAL` and `ITIMER_PROF` throw when armed, as process CPU time is not accounted, and read and disarm as zero. PPSA99420 imports it and fails to load without it
 - On Linux, [direct memory](../../../core/libs/prx/libkernel/DirectMemory/DirectMemory.cpp) lives in one sparse memfd at file offset = physical address, so the [GPU imports](../../../core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp) can share one udmabuf buffer object per 64 MiB of it. Released direct memory is not punched out of the file, because a live import of its chunk pins the pages and the GPU would keep reading the old ones: its pages stay committed (the process keeps the high-water mark of the direct memory it used) and are zeroed when the range is allocated again.
 - [system](../../../core/libs/prx/libkernel/System/src/Process.cpp) (libkernel) runs no command. It follows FreeBSD 11's `lib/libc/stdlib/system.c` for a sandbox without `/bin/sh`: a null command returns 1 unconditionally, as that version does, and any command returns the wait status of a child that could not exec the shell and called `_exit(127)` (`0x7f00`). Whether the console's application sandbox has no shell, and whether `vfork` succeeds there (FreeBSD returns -1 when it fails), is unverified
 - [sceKernelWriteThrottlingStatus](../../../core/libs/prx/libkernel/File/src/Stdio.cpp) (libkernel) reports no write throttling: the host SSD has no write budget, so the first qword of the 32-byte status is set to `UINT32_MAX` and the rest is zeroed; returns `EFAULT` (`0x8002000e`) when passed a null status buffer pointer. PPSA17221 reads only that qword: it multiplies it by 64 KiB and compares the product with byte thresholds up to 0x4AF00000 to pick one of five tiers. When the call fails the title assumes 0x4B000001 bytes, which selects the same top tier as `UINT32_MAX`. What the value measures on hardware (a remaining budget or a rate) and the meaning of the other three qwords are unknown