linux: close four path- and fd-policy gaps in the syscall handlers - #108
Open
perbu wants to merge 1 commit into
Open
linux: close four path- and fd-policy gaps in the syscall handlers#108perbu wants to merge 1 commit into
perbu wants to merge 1 commit into
Conversation
openat (#94): the read-only branch passed the guest's O_CREAT, O_TRUNC and O_APPEND straight into openat2's open_how, having consulted only the readable-path policy. may_open() adds MAY_WRITE for O_TRUNC and honours O_CREAT regardless of the O_RDONLY access mode, so a path approved for reading could be truncated or created. Deny the open with -EACCES rather than quietly clearing the bits: a guest told the open succeeded would go on believing the file had been truncated or created. Creating and truncating belong on the write branch, which consults is_writable_path(). statx (#95): the -EPERM from the readable-path check sat in a bare `if` with no `else` and no early return, so the host statx() below it ran anyway and overwrote sysret(). Gate the fd translation and the host call the way the sibling newfstatat handler already does, keeping the empty-path (AT_EMPTY_PATH) case. unlinkat (#96): the handler read flags from sysarg(1) and the path pointer from sysarg(2), so the pathname pointer was OR'd into the flags word and the flags word was dereferenced as a string. It also called is_writable_path() before the path was populated -- the policy callback only ever saw "" -- and OR'd in AT_SYMLINK_NOFOLLOW, which unlinkat does not accept. Read the registers in the Linux order, mask flags to AT_REMOVEDIR, resolve the path before consulting the policy, and adopt the try/catch -> -EBADF idiom the neighbouring handlers use. An empty path now returns -ENOENT instead of being handed to the policy layer. fcntl(F_SETFL) and setsockopt (#97): both mutate the host fd but resolved the vfd with the plain translate(). Use translate_writable_vfd(), as F_SETLK64 in the same handler already does. setsockopt had no try/catch, so it grows one -- translate_writable_vfd() throws for a non-writable fd, and that must not escape system_call(). Tests: the openat, statx and unlinkat cases in tests/unit/syscalls.cpp, plus a new fcntl case that checks both the return value and, via F_GETFL, that O_NONBLOCK did not stick on the host fd. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #94, #95, #96 and #97 — the four policy gaps from the audit. Each is small and independent, but they are all in the same handler file and share one theme (a path or fd check that does not actually gate what follows), so they are batched.
One judgement call worth review, on #94. The obvious fix is to clear
O_CREAT|O_TRUNC|O_APPENDbefore buildingopen_how, but thenopen(path, O_RDONLY|O_CREAT)on a missing file "succeeds" without creating it, andO_TRUNC"succeeds" without truncating — the guest is told something that is not true. This instead denies the open with-EACCES. Mutation is what the write branch andis_writable_path()are for. Happy to switch to stripping if you would rather keep the call succeeding.setsockoptgrows atry/catchas part of the #97 fix:translate_writable_vfd()throws for a non-writable fd, and that handler had no catch, so switching to it without one would have turned a policy check into a foreign exception escapingsystem_call()— the bug in #101/#102.Tests: the
openat,statxandunlinkatcases already written for the audit, plus a newfcntlcase (#97 had none) that checks both the return value and, viaF_GETFL, thatO_NONBLOCKdid not stick on the host fd. Verified it goes red without the fix.tests/unit/syscalls.cppis 16/16 green on this branch. The pre-existingtegridyfailures and thereset"crash recovery" failure are untouched and also fail on master.🤖 Generated with Claude Code