Skip to content

fix(libkernel): reject invalid open flag combinations - #6857

Open
MarcZX10 wants to merge 1 commit into
boykopovar:mainfrom
MarcZX10:codex/open-invalid-flags-current-main
Open

MarcZX10 wants to merge 1 commit into
boykopovar:mainfrom
MarcZX10:codex/open-invalid-flags-current-main

Conversation

@MarcZX10

@MarcZX10 MarcZX10 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

What

Reject two invalid flag combinations in the libkernel open entry points before path lookup: access mode 3 and the O_CREAT | O_EXCL | O_DIRECTORY combination. They return EINVAL (or the corresponding SCE error) instead of throwing or creating a regular file. Adds regression coverage for sceKernelOpen, open, _open, and openat.

This PR carries Hussein Kadhim’s existing commit from issue #6855. The commit author is preserved; the Windows tests, fuzz probes, and independent reviews described in the issue are Hussein’s reported results, not runs by me.

Tested

  • Linux validation reported by root-over: guest_filesystem, guest_kernel_sys, file_streams, guest_dup, guest_kernel_fcntl, guest_posix_filesystem, and guest_truncate pass; reverting Open.cpp makes the two new test executables abort as expected. Two unrelated file_writer_* failures also occur on main with Ubuntu’s uutils cp.
  • python3 tools/check_conventions.py --base upstream/main passed on the current-main cherry-pick.
  • git diff --check upstream/main...HEAD passed.
  • No local build/test is claimed.

Related open PRs

Manual overlap review of the intake report: #5831 changes Windows append-write handling in Open.cpp; #5835 adds archive-aware path handling to Open.cpp; #6044 reorganizes test registration and shares test paths. These changes do not implement the invalid-flag behavior. When integrating with #5835, retain the early invalid-flag check and its archive-aware path resolution.

Checklist

  • Based on current main (82ddae7)
  • One topic per PR
  • No comments or unrelated files added
  • No new dependencies
  • Depends on: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes; original change by Hussein Kadhim, carried forward with preserved commit authorship

…_CREAT|O_EXCL|O_DIRECTORY

FreeBSD rejects an access mode of 3 in kern_openat and the combination of O_CREAT, O_EXCL and O_DIRECTORY in vn_open_cred before it looks the path up. sceKernelOpen, open and openat threw std::invalid_argument for access mode 3 and, on Windows, created the file for the second combination.

(cherry picked from commit 038a070)
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

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/File/src/Open.cpp
PR 6044 core/libs/tests/GuestFilesystem.cpp
core/libs/tests/GuestKernelSys.cpp
PR 6897 core/libs/tests/GuestFilesystem.cpp
PR 6936 core/libs/prx/libkernel/File/src/Open.cpp
PR 6942 core/libs/prx/libkernel/File/src/Open.cpp
PR 7039 core/libs/tests/GuestKernelSys.cpp
PR 7121 core/libs/prx/libkernel/File/src/Open.cpp
PR 7201 core/libs/prx/libkernel/File/src/Open.cpp
PR 7332 core/libs/prx/libkernel/File/src/Open.cpp
core/libs/tests/GuestKernelSys.cpp

@root-over

root-over commented Oct 16, 2026 •

Copy link
Copy Markdown

Tested on Linux, since the description says only Windows was built and run.

  • Setup: Ubuntu (GCC 15.2.0, CMake 4.2.3, Ninja), Release with BUILD_TESTING=ON, the PR merged into main at 4524944; Ryzen 5 5625U, RADV (Mesa 26.0.8).
  • guest_filesystem, guest_kernel_sys, file_streams, guest_dup, guest_kernel_fcntl, guest_posix_filesystem and guest_truncate pass.
  • With core/libs/prx/libkernel/File/src/Open.cpp reverted to main and the new tests kept, guest_filesystem and guest_kernel_sys abort, so the tests catch the bug on Linux too.
  • Full suite: 1285 of 1287 pass. The two failures, file_writer_binary and file_writer_text, also fail on main here and are unrelated: the test copies /bin/sleep under another name, and uutils coreutils (the Ubuntu default) refuses to run under a name that is not a utility.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for the Linux run and reverted-source negative control. I updated the validation section to include your exact scope and the unrelated uutils cp failures. I also reviewed the overlap notice: #5831 changes Windows append writes and #5835 adds archive-aware open resolution in the same source file; those are distinct behaviors, though the #5835 integration should preserve this PR’s early flag check.

@jotitadev jotitadev 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.

Built and ran this on Linux, since the PR has no local run. Head fd407e34a0ea380e6aee225e8975080fef86e120 merged on main 452494439. Intel i7-8700, Ubuntu 26.04 (kernel 7.0), GCC 15.2, CMake 4.2.3, Ninja, Release, BUILD_TESTING=ON, full build.

  • guest_filesystem and guest_kernel_sys pass, 3 of 3 runs.
  • With main's Open.cpp and the PR's tests, both abort at the first access-mode-3 check: sceKernelOpen: invalid access mode (std::invalid_argument).
  • The O_CREAT | O_EXCL | O_DIRECTORY checks already pass on main on this host. I removed only the access-mode-3 assertions and kept main's Open.cpp: both tests pass, and no invalid-flags.txt is created. The host open() already rejects that combination with EINVAL on this kernel (7.0). So on this host these checks would not catch a regression; they still matter for hosts whose open() accepts it. The access-mode half does fail without the fix.
  • Non-GPU suite (ctest -E "^agc|gpu|vulkan|video_out" -j6): 648/650 both with this PR and on main. The two failures, file_writer_binary and file_writer_text, also fail on main on this machine.

Not checked: Windows, macOS.

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for running the tests on current Linux and separating the host-dependent checks. The access-mode-3 negative control confirms that part detects the regression on this host; I have noted that the create/exclusive/directory case is already rejected by this kernel and therefore is not independently demonstrated by that control. The two non-GPU suite failures reproduce on main.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for the Linux cross-checks. The access-mode regression is reproduced with Open.cpp reverted, while the O_CREAT | O_EXCL | O_DIRECTORY case is already rejected by this Linux kernel on main; the new assertion still covers hosts whose open() accepts that combination. The two non-GPU failures reproduce on baseline as noted. I have recorded these host-specific results and will leave the assertions in place.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

The conventions failure on the current head is only the required Depends on checklist value. I filled in Depends on: none because the PR body lists no prerequisite PRs. The workflow should now rerun; no source changes were needed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants