[babyhuey]

What

ioctl(FIONBIO) read its int argument directly after the existing null check, so a non-null pointer into unmapped memory crashed the host. FIONBIO is an _IOW request: FreeBSD's sys_ioctl copies the argument in and returns EFAULT if that fails.

• The argument is copied in with GuestCopyIn_nid_no_patch before the status change; a failed copy returns EFAULT (14).
• The change stays inside the FIONBIO branch, after the descriptor lookup as before, so a bad descriptor still reports EBADF first.

Tested

core/libs/tests/GuestKernelFcntl.cpp adds an unmapped argument (0x10) next to the existing null case; both fail with EFAULT.

• Linux (anyps5-build CI image, Ubuntu 24.04, GCC 13, Release, BUILD_TESTING=ON), this head b4ae3afc61c on main f21ef7c99c5: guest_kernel_fcntl passes; the full ctest passes (2,740 of 2,740), and check_conventions.py passes. With main's DescriptorControl.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_kernel_fcntl passes 3 of 3 alone; with main's DescriptorControl.cpp it crashes (0xc0000005). CI's test selection for this change (2,384 tests): 1,644 pass, 739 skip, and one relinker test unrelated to this change, restricted_undefined_symbols_guest, timed out under --parallel 4 against its own 20 s limit; alone on this head it passes 3 of 3 in at most 3.7 s. check_conventions.py passes.

#10016, #11219 and #12474 also edit ioctl; each one's current head merges with this one without conflicts.

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: none
• Fixes: none
• Unblocks: none
• GPUs tested: n/a
• AI-assisted: yes

[MarcZX10]

Reviewed head b4ae3afc61c6a15a7f8a84e0966cca912316dcf2 against current main (319621ba2e578bd053b0baba2877fe70520e0737). The change reads the guest FIONBIO integer through GuestCopyIn_nid_no_patch before changing status, preserving descriptor lookup and its existing EBADF precedence while mapping an unreadable guest pointer to EFAULT. The added 0x10 regression covers the non-null unreadable case alongside null.

The current Windows workflow passes. The PR reports the focused and full Linux suites and a Windows full build/test run; I did not independently rerun those suites. The hosted Linux/product jobs are skipped. I also checked the current merge previews with #10016, #11219, and #12474; they combine cleanly, and their changed hunks do not require a declared ordering for this change. I found no blocking issue in the reviewed diff.

[MarcZX10]

Correction to the source links in my review above: the changed files are DescriptorControl.cpp and GuestKernelFcntl.cpp. My earlier paths were incorrect; the review conclusion is based on these two files.

[rooklz]

I reviewed head b4ae3afc61.

Linux (Ubuntu 24.04 amd64 container, GCC 13, Release, BUILD_TESTING=ON, worktree at this head): guest_kernel_fcntl passes 5 of 5 runs. RED check: with main's DescriptorControl.cpp (319621ba2e) under the head test, it segfaults (rc 139), as your body says.

Mutants (each rebuilt and run against the head test)

mutant	result
copy fault → return 0	killed
copy fault → EINVAL	killed
copy-in result ignored	killed
enable inverted	killed
copy-in of 2 bytes / 1 byte instead of sizeof(int)	survive

Error precedence: EFAULT should come before EBADF. The body says the copy-in stays after the descriptor lookup "so a bad descriptor still reports EBADF first". That is not FreeBSD's order. In sys_ioctl the IOC_IN argument is copied in (copyin(uap->data, data, size), then goto out on error) before kern_ioctl runs, and kern_ioctl is where fget returns EBADF. main already follows that order for a null argument: the line if ((command & RequestIn) && length != 0 && argument == nullptr) return Fail(GuestEfault); comes before Classify. So the head now gives two answers for the same bad-descriptor call:

ioctl(9999, FIONBIO, nullptr)       -> -1, errno 14 (EFAULT)
ioctl(9999, FIONBIO, (void*)0x10)   -> -1, errno 9  (EBADF)

FreeBSD returns EFAULT for both. Moving the copy-in up next to the existing null check fixes that. Your tests still pass with it (5 of 5):

     if ((command & RequestIn) && length != 0 && argument == nullptr) return Fail(GuestEfault);
+    int enable = 0;
+    if (command == NonblockingRequest && !GuestCopyIn_nid_no_patch(&enable, argument, sizeof(enable))) return Fail(GuestEfault);
     const auto kind = Classify(fd, "ioctl");
     if (!kind) return Fail(GuestEbadf);
@@
     if (command == NonblockingRequest) {
-        int enable = 0;
-        if (!GuestCopyIn_nid_no_patch(&enable, argument, sizeof(enable))) return Fail(GuestEfault);
         return ChangeStatus(fd, *kind, Nonblocking, enable != 0 ? Nonblocking : 0, "ioctl");

Here is a test diff that pins the order and the full-width read. Your head source fails it (rc 134), the moved version passes 5 of 5, and it kills both copy-size survivors: with 1 or 2 bytes, 0x10000 reads as 0 and the descriptor goes blocking.

     RequireFailure(ioctl_nid_postfix(file, NonblockingRequest, reinterpret_cast<void*>(std::uintptr_t{0x10})), GuestEfault);
+    RequireFailure(ioctl_nid_postfix(-1, NonblockingRequest, reinterpret_cast<void*>(std::uintptr_t{0x10})), GuestEfault);
@@ static void PipeFlagsRoundTrip() {
     Require(fcntl_nid_postfix(pipe[0], GetFlags) == (ReadWrite | NonBlocking));
+    int high = 0x10000;
+    Require(ioctl_nid_postfix(pipe[0], NonblockingRequest, &off) == 0);
+    Require(ioctl_nid_postfix(pipe[0], NonblockingRequest, &high) == 0);
+    Require(fcntl_nid_postfix(pipe[0], GetFlags) == (ReadWrite | NonBlocking));

Out of scope, for a follow-up: a guest socket never reaches File::Ioctl. ioctl_nid_postfix in Socket/src/Udp.cpp still dereferences the argument directly (value = *static_cast<int*>(argument) != 0;, and the FIONREAD store), so FIONBIO with an unmapped non-null pointer on a socket would still crash the host. I didn't run that one.

Gates: check_conventions.py (title + body) passes; git merge-tree against main 319621ba2e is clean, and so is each of #10016, #11219 and #12474 against this head. pr_overlap.py only lists files it could not compare (#7121, #9557, #9566, #11872, #11969: DescriptorControl.cpp).

[github-actions]

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 7121	core/libs/prx/libkernel/File/src/DescriptorControl.cpp
PR 9019	core/libs/prx/libkernel/File/src/DescriptorControl.cpp
PR 9557	core/libs/prx/libkernel/File/src/DescriptorControl.cpp
PR 9566	core/libs/prx/libkernel/File/src/DescriptorControl.cpp
PR 11872	core/libs/prx/libkernel/File/src/DescriptorControl.cpp
PR 11969	core/libs/prx/libkernel/File/src/DescriptorControl.cpp