Skip to content

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

Closed
AdnanKhan-ak47 wants to merge 1 commit into
boykopovar:mainfrom
AdnanKhan-ak47:fix/libkernel-time-efault
Closed

AdnanKhan-ak47 wants to merge 1 commit into
boykopovar:mainfrom
AdnanKhan-ak47:fix/libkernel-time-efault

Conversation

@AdnanKhan-ak47

Copy link
Copy Markdown
Contributor

What

gettimeofday, sceKernelGettimeofday, sceKernelGettimezone, getitimer and setitimer fail with EFAULT (0x8002000e for the sceKernel forms) when a pointer argument is unmapped, as FreeBSD's copyin and copyout make sys_gettimeofday, sys_getitimer and sys_setitimer fail, instead of crashing the host. The null checks already in place stay as they were.

The outputs go through GuestCopyOut_nid_no_patch, which fstat and the descriptor calls already use. setitimer reads its value through a new GuestCopyIn_nid_no_patch in libc, the read counterpart of the existing checked write: ReadProcessMemory on Windows, process_vm_readv on Linux and mach_vm_read_overwrite on macOS, after GuestArenaPrepareHostRead. setitimer keeps FreeBSD's order, copyin then EINVAL checks then set then copyout, so a previous pointer that fails after the timer was set leaves it set, as sys_setitimer does; the regression probes that path with a disarming value. gettimeofday computes the time once into a local and copies it out, which is what let sceKernelGettimeofday answer its own error code.

GuestFreebsdClocks.cpp, GuestGetitimer.cpp and GuestItimer.cpp probe the issue's two addresses, 1 and 0xfffffffffffff000, on every pointer parameter and check that a failed call writes nothing to the other output.

Tested

Arch Linux x86-64, GCC 16.2.1: ctest -R '^guest_(freebsd_clocks|getitimer|itimer)$' 3/3 pass; with main's Time.cpp and Signals.cpp restored, all three fail with SegFault. tools/check_conventions.py --base origin/main clean. tools/pr_overlap.py reports #10499, which changes the body of sceKernelGettimezone to report host zone offsets; this PR changes the same two lines to a checked copy-out, so whichever lands second rebases that body. Windows and macOS not run; the copy helpers are the ones GuestCopyOut and the getrlimit fix (#8986) already exercise there.

Checklist

@MarcZX10 MarcZX10 left a comment

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.

Requesting changes for contribution coordination: #10975 was already open and implements the same six time-related exports, adds the same checked GuestCopyIn/GuestCopyOut helpers, and also declares Fixes: #10969. This PR independently changes those same exports and fixes the same issue, so the checklist statement that no open PR implements the same functions is no longer accurate. The repository asks authors of overlapping PRs to agree which one goes first and declare Depends on when one builds on the other.

There is useful distinct work here: this PR adds the checked macOS mach_vm_read_overwrite path that the current #10975 head is missing. Please coordinate with the #10975 author and maintainers on one implementation path, preserving that macOS fix and its tests without leaving two competing implementations of the same exports. Update the dependency/fix checklist and rebase the selected path once that order is agreed. I have not approved either implementation pending that coordination; the Linux and Windows-focused evidence does not settle the macOS runtime case.

@AdnanKhan-ak47

Copy link
Copy Markdown
Contributor Author

Agreed, #10975 was first and is the issue author's; withdrawing this one. @xXPBSXx the only piece here that #10975's current head lacks is the macOS branch of GuestCopyIn_nid_no_patch, which falls through to a raw memcpy there. Mirroring GuestCopyOut's macOS path:

#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;

Free to take anything else from AdnanKhan-ak47:fix/libkernel-time-efault (the setitimer probe with a disarming value covers the FreeBSD "timer stays set when the previous copy-out fails" order, which bit my first test version). Verified there on Linux: guest_freebsd_clocks, guest_getitimer and guest_itimer SegFault on main and pass with the fix.

@AdnanKhan-ak47
AdnanKhan-ak47 deleted the fix/libkernel-time-efault branch November 2, 2026 10:34
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.

fix(libkernel): gettimeofday, getitimer, setitimer and the sceKernel time getters crash on a pointer into unmapped memory instead of failing with EFAULT

2 participants