[babyhuey]

What

clock_gettime and clock_getres wrote their result straight through the guest pointer, so a non-null pointer into unmapped or read-only memory crashed the host. gettimeofday and sceKernelGettimezone next to them already copy out with GuestCopyOut_nid_no_patch. FreeBSD's sys_clock_gettime and sys_clock_getres use copyout and return EFAULT.

• clock_gettime computes the value as before, then copies it out with GuestCopyOut_nid_no_patch, failing with EFAULT (14). A null pointer still fails with EFAULT after clock validation, as before.
• clock_getres computes into a local value (the body moves to a static ReadClockResolution, unchanged apart from the recursion for CPU-time clocks) and copies it out once. A null result still succeeds, and an unsupported clock still throws before the pointer is used.
• sceKernelClockGettime and sceKernelClockGetres return 0x8002000e through their existing SceKernelError mapping.

Tested

BadTimePointersFault in core/libs/tests/GuestFreebsdClocks.cpp: for CLOCK_REALTIME, CLOCK_MONOTONIC, CLOCK_SECOND and CLOCK_THREAD_CPUTIME_ID, both the _nid_postfix calls and the sceKernel wrappers with an unmapped pointer (0x10) and a read-only page fail with EFAULT / 0x8002000e, and the read-only page is left unchanged.

• Linux (anyps5-build CI image, Ubuntu 24.04, GCC 13, Release, BUILD_TESTING=ON), this head f4863ca1f5a on main 2227f136618: guest_freebsd_clocks passes; the full ctest passes (2,680 run, 0 failed), and check_conventions.py passes. With main's Time.cpp the same test segfaults.
• Windows: Windows 11, Ryzen 7 5800X, RTX 3080, WinLibs GCC 15.2, Release, BUILD_TESTING=ON, this head: full build and libs succeed; guest_freebsd_clocks passes 3 of 3 alone; with main's Time.cpp it crashes (0xc0000005). CI's test selection for this change (2,348 tests): 1,621 pass, 726 skip, and the only failure is guest_raise_exception, which also fails on main on this machine (libkernel (Windows): guest_raise_exception slowed about 3x and sometimes aborts at "address waking" since 540364a24 (#10666) #11813). check_conventions.py passes.

Checklist

• Based on current main; no other open PR implements the same functions
• One topic per PR; follow-ups go in a new PR
• No comments in code except technical debt
• Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
• No notes, investigation .md files or images added to the repository (attach them to this PR)
• New third-party code is a submodule built from source
• Generic behaviour, not specific to one title
• Depends on: a fast path in the guest-copy helpers (see the review; separate PR coming). The time-zone fix the new assertions need on non-UTC hosts is test(libkernel): pin the zone before the kernel timezone check #12747, already merged.
• Fixes: none
• Unblocks: none
• GPUs tested: n/a
• AI-assisted: yes

[MarcZX10]

Reviewed exact head f4863ca. The change computes into local KernelTimespec values and routes only the final guest write through GuestCopyOut, preserving clock validation before output-pointer validation for clock_gettime and preserving the null-output success path for clock_getres. The regression covers unmapped and read-only targets for four valid clock IDs, both libc NID calls and SceKernel wrappers, and verifies the read-only target remains unchanged. The current-main comparison shows no intervening changes to Time.cpp or GuestFreebsdClocks.cpp since the tested 2227f13 base; current-main diff check and conventions pass. I did not rerun the large Linux/Windows suites locally. Hosted Windows and report checks were pending when checked, so I have not approved.

[github-actions]

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request	Conflicting files (lines on main)	Same exports	Same new files
PR 14557	core/libs/prx/libkernel/Time/Time.cpp L640

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request	Shared files not compared
PR 5021	core/libs/prx/libkernel/Time/Time.cpp
PR 7804	core/libs/prx/libkernel/Time/Time.cpp
PR 12130	core/libs/prx/libkernel/Time/Time.cpp
PR 12642	core/libs/prx/libkernel/Time/Time.cpp
PR 15035	core/libs/prx/libkernel/Time/Time.cpp core/libs/tests/GuestFreebsdClocks.cpp

[oneandonlydean]

@babyhuey thanks; GuestCopyOut_nid_no_patch is the right way to detect the bad pointer (the same check gettimeofday uses), the clock is still validated first, and clock_getres with a null res still succeeds.

One question about cost: clock_gettime is on a hot path. Titles' own libc.prx calls it for their time and chrono functions, often many times per frame. Every successful call now also goes through the HostWrite guard and a process_vm_writev system call on Linux (VirtualQuery on Windows), where before it was a plain store after the clock read. Could you measure the per-call time of clock_gettime_nid_postfix on main and on this branch (for example a loop of a few million calls)? If the difference is significant, could the valid case keep a direct write, using the checked copy only when the destination isn't a known writable guest allocation?

[rooklz]

Reviewed exact head f4863ca1f5, adding the macOS evidence the body doesn't have (it lists Linux and Windows only).

Gates. git merge-tree against current main 750cf6cf48 is clean. check_conventions.py --base main passes. pr_overlap.py reports only the three uncomparable Time.cpp sharers. I checked them by hand: #12130 touches only CancellableSleep (L354), and merge-tree pr-12418 × pr-12130 is clean. #7804 and #5021 conflict with main already.

What GuestCopyOut does with these pointers on macOS (standalone probe, Apple clang, arm64, 16-byte mach_vm_write like the __APPLE__ arm):

destination	kr	target after
host stack	0	written
RW mmap page + 8	0	written
PROT_READ page	1 (KERN_INVALID_ADDRESS)	unchanged
PROT_NONE page	1	n/a
0x10	1	n/a
16 bytes straddling RW → RO	1	first 8 bytes unchanged (no partial write)

So on macOS both cases in BadTimePointersFault hit the false path, and the read-only page is left untouched, as the final Require expects. The same probe in the Linux (process_vm_writev, Ubuntu 24.04 GCC 13) image gives stack → 16, RO → -1/EFAULT, 0x10 → -1/EFAULT. The straddle case → 8, and the RW half is written. GuestCopyOut still returns false there (copied != bytes), so the result is EFAULT with a partial write. That matches FreeBSD copyout faulting part-way through, so it isn't a problem here. I'm only noting it because the test's "unchanged" assertion is only meaningful for a whole-page RO target, which is what the test uses.

Host callers still work. clock_gettime_nid_postfix is also called internally with host-stack KernelTimespecs: Pthread/Posix/Common.hpp:30 RelativeMicroseconds (every timed mutex/cond/sem/rwlock wait), Ktimer.cpp:55, SyncOnAddress.cpp:236, Socket/src/Udp.cpp:1673 (per received datagram), SystemConfiguration.cpp:103 and libkernel_sys/Uuid.cpp:37. The probe above shows host-stack writes succeed on both POSIX primitives. On Windows, BeginHostWrite only walks views, so a non-guest address returns true. On macOS, GuestWriteWatch::forEachPage only visits registered ranges, so these callers keep working.

Cost (non-blocking). 2M iterations of CLOCK_MONOTONIC + 16-byte checked copy to a stack local:

host	clock_gettime	+ checked copy
macOS arm64 (M-series)	13.9 ns	399 ns
macOS x86_64 under Rosetta	22.0 ns	494 ns
Linux x86_64 (emulated container)	19.5–19.9 ns	448–459 ns

For guest calls this is the price of correctness, and gettimeofday already pays it. The internal host callers pay it too, though, for pointers that can't fault. On macOS that also means taking the write-watch _lock plus a _hostWrites push/erase when the watch is available. Timed waits and per-datagram stamping hide a few hundred ns, so I'm not blocking on it. A cheap follow-up would be to route those callers through the existing unchecked ReadClock (e.g., a ReadClock_nid_no_patch host export) instead of the guest-facing entry point.

Semantics checked against main. These stay the same: clock validation before the null/fault check in clock_gettime (OmittedTimeIsFaultAfterClockValidation), null res → 0 before validation in clock_getres, unsupported clock still throws without touching the output (UnsupportedResolutionStillThrows), and the CPU-clock recursion now goes through ReadClockResolution into the local value. I didn't build the libs lane on macOS (no submodules here), so guest_freebsd_clocks itself wasn't run on macOS. The primitive-level probe above covers the host-specific part of the change.

The change looks correct to me. I'm leaving this as a comment, not an approval, because I couldn't run the PR's own test on macOS. If someone with a libs-lane macOS build runs guest_freebsd_clocks on this head, that closes the gap. The cost note doesn't block.

AI-assisted: yes

[MarcZX10]

I ran a small native Linux proxy loop to estimate the syscall cost raised above: one million direct clock_gettime(CLOCK_REALTIME, &out) calls measured 18.8–23.2 ns/call over two rounds; clock_gettime into a local timespec followed by process_vm_writev(getpid(), ...) to a local output measured 1,191.8–1,213.1 ns/call (about 52–63× in this run).

This is only a host-side proxy for the syscall component. It does not execute AnyPS5's guest clock_gettime_nid_postfix or its GuestCopyOut/HostWrite guards, and is not a matched main/branch benchmark. It does show why an exact guest-path measurement would be useful before deciding whether the checked copy is acceptable on this hot path; I have not treated these numbers as evidence of the PR's total overhead.

[ebosido444-ai]

The GuestCopyOut move is the right shape — no sentinel write, no torn value, and the read-only page in the test stays zero. Two things it leaves behind:

1. core/libs/prx/libkernel/Time/Time.cpp:716 — clock_getres_nid_postfix still returns success on a null res, while clock_gettime_nid_postfix:587 now returns EFAULT for null. The same PR makes the two entry points disagree on what a null output pointer means. core/libs/tests/GuestFreebsdClocks.cpp:366 asserts the success behaviour for nullResolutionClocks, so whichever is intended is now pinned in a test rather than decided.

2. :716 returns before ReadClockResolution, so clock_getres(unsupported_clock, nullptr) reports success while clock_getres(unsupported_clock, valid) reports the unsupported-clock error. clock_gettime deliberately validates the clock first (:585 before the null/copy-out check) and OmittedTimeIsFaultAfterClockValidation pins that order for the time side; the resolution side has no equivalent, so a null pointer now bypasses clock validation entirely.

If the null behaviour is a PS5 quirk worth keeping, a one-line comment on :716 saying so closes point 1; point 2 needs the null check moved after the read if you want the two calls to validate alike.

[MarcZX10]

One semantic clarification on the new null-result question: FreeBSD's sys_clock_getres returns success immediately when tp == NULL, before calling kern_clock_getres (which validates the clock ID). That matches this implementation's existing null-result behavior, including an unsupported ID with a null result. Source: https://cgit.freebsd.org/src/tree/sys/kern/kern_time.c?id=3f8455b0905a05552b1155e09165787e1d098060 . So I do not consider the null-pointer validation ordering a blocker against the FreeBSD behavior this PR is matching. The per-call cost question remains useful follow-up evidence; the PR's exact guest-path overhead is not established by the host proxy measurements posted so far.

[babyhuey]

@oneandonlydean measured on the guest path, as you asked. clock_gettime_nid_postfix(CLOCK_MONOTONIC, &t) into a stack KernelTimespec, 2,000,000 calls after a 100,000-call warm-up, three rounds, Linux (anyps5-build CI image, Release, Ryzen 7 PRO 5850U), same build with only Time.cpp swapped:

build	ns per call
main 2227f136618	30.3 / 30.5 / 30.4
this head f4863ca1f5a	1,308.2 / 1,307.9 / 1,307.2
gettimeofday_nid_postfix(&tv, nullptr) on the same build, unchanged from main	1,315.3 / 1,329.2 / 1,312.7

So the checked copy costs about 1.28 µs per call here, about 43× main's clock_gettime. It's the HostWrite guard plus process_vm_writev in GuestCopyOut_nid_no_patch, and gettimeofday already pays the same on main. I haven't measured Windows (VirtualQuery there).

That's too much to add to a call titles make many times per frame, so I don't think this should merge as it is. I see two ways forward and would like your view before changing anything:

1. A fast path in GuestCopyOut_nid_no_patch itself, so every caller benefits, gettimeofday included: store directly when the destination is known writable and fall back to the checked copy otherwise. I didn't find an existing cheap writability lookup in GuestArena or the memory code, so this needs one (or a fault-tolerant store), and it's a bigger change that I'd do as its own PR.
2. Keep clock_gettime's direct store until that exists, and close this PR for now.

AI-assisted: yes (Claude Code)

[babyhuey]

One more measurement, since the same question covers my other EFAULT PRs: every one of them uses GuestCopyIn_nid_no_patch / GuestCopyOut_nid_no_patch, so whatever is decided here applies to all of them. kevent_nid_postfix(kq, nullptr, 0, events, 16, &zero) with 4 user events ready, 300,000 calls after warm-up, three rounds, same Linux setup as above:

build	ns per call
main	97.1 / 99.8 / 98.2
#12503 (timeout copied in, events copied out)	2,773.3 / 2,860.7 / 2,784.3

So the two hot paths, clock_gettime (#12418) and kevent (#12503), shouldn't merge with the checked copy as it is. The others (#12462 poll, #12502 select, #12525 pselect, #12538 utimes family, #12556 rtprio_thread/sched_rr_get_interval, #12563 setrlimit, #12579 ioctl(FIONBIO)) add the same ~1.3 µs per copy, but poll/select/pselect already make a host system call per call, and the rest are rarely called; I haven't measured those. If a fast path in the copy helpers is the way you'd like to go, I'll write it as its own PR and rebase these on it.

AI-assisted: yes (Claude Code)

[JJenkx]

Windows run of this head (f4863ca1f5), merged onto main edf6804cf5 (clean merge, local only). Windows 11 Enterprise 10.0.22621, WinLibs GCC 15.2.0 UCRT r7, Ninja, Release, BUILD_TESTING=ON, host zone Eastern Standard Time.

• This PR alone: guest_freebsd_clocks aborts. It aborts on plain main edf6804cf5 on this host too, so that is not this PR. On main the test still has the KernelTimezoneIsUtc check, which only holds when the host zone is UTC. fix(tests): drop the guest_freebsd_clocks check that the host zone is UTC #12660 removes that check.
• This PR plus fix(tests): drop the guest_freebsd_clocks check that the host zone is UTC #12660 (ab03f37893): guest_freebsd_clocks passes 3 of 3 (ctest --repeat until-fail:3).
• Negative control on that combined tree: with core/libs/prx/libkernel/Time/Time.cpp put back to main's version and both PRs' tests kept, guest_freebsd_clocks fails. So the new EFAULT assertions catch the change on Windows.

AI-assisted: yes (Claude Code).

[MarcZX10]

Re-reviewed exact head f4863ca1f5a65ea06b7eea491713c1a5628c1491 and the new Windows report. The checked guest write and invalid-pointer regressions are the right correctness direction; I agree the FreeBSD null-result behavior is preserved. The remaining issue is the reported hot-path cost: the guest-path measurement is about 1.31 µs/call versus 30.4 ns on main (about 43×), and the author says titles call this path many times per frame. I would not restore an unchecked store, which would lose the EFAULT behavior. Please pursue a safe fast path in the shared guest-copy helper as a prerequisite (so the sibling syscall fixes can use it too), or provide representative title/frame call-count evidence showing the aggregate cost is acceptable before merging this hot-path change. Keep the bad-pointer tests against the checked behavior.

The Windows run reports the PR alone still aborts on an Eastern-time host because main’s unrelated KernelTimezoneIsUtc assertion is present; it passes when combined with open #12660. Please coordinate and record the order with #12660 so rebasing preserves both its timezone-test correction and this PR’s bad-pointer regression. The reported Windows run is on the combined tree; it is not a standalone pass for this head. Linux/product hosted jobs are skipped. I did not run these tests locally.

[babyhuey]

@MarcZX10 thanks, I'll do the fast path in the shared helpers as its own PR first. Before writing it, here's what I found on main and the two designs I'd consider. I'd like your pick, because they trade exactness against size.

What the helpers pay today. On Linux the cost is only the process_vm_readv / process_vm_writev syscall: GuestWriteWatchBeginHostWrite/EndHostWrite are no-ops there (GuestWriteWatch.cpp, only macOS does work; uffd write-protect sees plain stores). On Windows it's ReadProcessMemory or VirtualQuery plus WindowsMappings::BeginHostWrite's mutex. On macOS the HostWrite guard does real work (unprotect and mark written) and has to stay.

What's tracked. g_protections in DirectMemory.cpp (updated by RecordProtection on map, mprotect and munmap) covers guest stacks (AllocateGuestStack maps them through DoMapAnon) and the guest heap, but it's a std::map behind a plain mutex and stores the guest-reported protection. I haven't confirmed that module data and BSS go through it yet.

Option 1: per-thread cache with a global generation. DoMapAnon / DoMprotect / DoMunmap bump an atomic generation before changing host mappings. Each thread caches the last page range it checked as readable or writable, and the generation it saw. A hit (same range, same generation) does a plain memcpy, about 3-5 ns; a miss takes the current checked path and refills the cache. Small and contained (a counter in DirectMemory.cpp, a thread-local in General.cpp). It is not exact: if another thread unmaps or protects the page between the generation check and the store, the host can still fault. That's a guest racing its own munmap against a syscall on the same buffer, but it isn't FreeBSD's guarantee.

Option 2: fault-tolerant store. memcpy under a thread-local recovery flag, with a SIGSEGV/SIGBUS handler (vectored on Windows) that unwinds to an EFAULT return. Exact, no race, near-zero cost. But there's no resumable fault mechanism on main today: it means a handler chained ahead of macOS write-watch and the Windows crash handler, care with the HostWrite guard on the fault path, and testing on all three OSes. Much bigger.

I'd start with option 1 unless the race in it is a blocker for you, and add option 2 later if strict exactness is wanted. Either way the bad-pointer tests stay on the checked behaviour, and I'll report the same per-call benchmark on main, the current branches and the fast path.

AI-assisted: yes (Claude Code)

[adriadam10]

Thanks, the change itself is right: GuestCopyOut gives the EFAULT, clock validation still comes first, the null clock_getres result matches FreeBSD, and the test pins the unmapped and read-only cases.

Blocking, as already raised by @oneandonlydean and @MarcZX10:

• core/libs/prx/libkernel/Time/Time.cpp (clock_gettime_nid_postfix): the checked copy takes a process_vm_writev syscall on every successful call, 1,308 ns vs 30 ns on main by your own measurement. clock_gettime is on a hot path, so please merge this only after the guest-copy fast path lands (your option 1, a per-thread cache invalidated by a generation bumped in map/mprotect/munmap, is a reasonable start; keep the bad-pointer tests on the checked path) and rerun the same per-call benchmark on this head.
• Order: #12660 first (the new clock assertions need its time-zone fix on non-UTC hosts), then the fast path, then this PR rebased.

[JJenkx]

@babyhuey #12660 is closed: #12747 merged into main and fixes the same KernelTimezoneIsUtc host-zone assumption. On main 184afa49ad, guest_freebsd_clocks passes 3 of 3 here on a non-UTC host (US Eastern). The time-zone part of your Depends on is therefore already met by main, and only the guest-copy fast path you list remains. You may want to replace #12660 with #12747 (merged) in the checklist.

AI-assisted: yes (Claude Code).

[bunbeans2-arch]

A Windows run with GCC 16: Windows 11 Pro 10.0.22631, i9-9900K, MSYS2 UCRT64 GCC 16.1.0, Release, this PR (f4863ca1f5) merged into main f7f03da.

• With this PR: guest_freebsd_clocks passed 5 of 5.
• Same tree with only libkernel Time.cpp put back to main (this PR's test kept): it crashed with an access violation (0xc0000005) in 5 of 5 runs.
• main f7f03da alone (its own test and source): guest_freebsd_clocks passed 3 of 3, so the crash comes from the new bad-pointer checks this PR adds.

AI-assisted: yes (Claude Code)

[Gruzzberry]

Gruzzberry's #15035 (settimeofday and clock_settime return EPERM for a jailed caller) touches neighbouring lines of the same file. Proposal: yours first, mine rebases. AI-assisted: yes (Claude Code).