Replace KILLPRIV with KILLPRIV_V2#532
Merged
slp merged 7 commits intocontainers:mainfrom Feb 17, 2026
Merged
Conversation
slp
requested review from
MatiasVara,
jakecorrenti,
mtjhrc and
tylerfanelli
as code owners
slp
force-pushed
the
virtiofs-killpriv-v2
branch
from
33af184 to
1c23c77
Compare
Collaborator
Author
|
Tested with pjdfstest on both Linux and macOS. |
We never really supported HANDLE_KILLPRIV, so let's be honest about it and stop announcing it. Signed-off-by: Sergio Lopez <slp@redhat.com>
We're going to need this library in the next commit libcap-ng-dev. Signed-off-by: Sergio Lopez <slp@redhat.com>
slp
force-pushed
the
virtiofs-killpriv-v2
branch
from
1c23c77 to
b01a30c
Compare
mtjhrc
reviewed
Feb 17, 2026
mtjhrc
reviewed
Feb 17, 2026
mtjhrc
reviewed
Feb 17, 2026
mtjhrc
reviewed
Feb 17, 2026
mtjhrc
reviewed
Feb 17, 2026
KILLPRIV_V2 is a FUSE flag that indicates that the filesystem is
responsible for clearing security.capability xattr and clearing setuid
and setgid bits, following these rules:
- clear "security.capability" on write, truncate and chown unconditionally
- clear suid/sgid if following is true. Note, sgid is cleared only if
group executable bit is set.
o setattr has FATTR_SIZE and FATTR_KILL_SUIDGID set.
o setattr has FATTR_UID or FATTR_GID
o open has O_TRUNC and FUSE_OPEN_KILL_SUIDGID
o create has O_TRUNC and FUSE_OPEN_KILL_SUIDGID flag set.
Signed-off-by: Sergio Lopez <slp@redhat.com>
Those structs were mostly redundant, converge on InodeHandle. This also allows us to simplify and unify many code paths. Signed-off-by: Sergio Lopez <slp@redhat.com>
Reuse the previously obtained libc::stat with get_xattr_* to avoid an additional stat syscall. Signed-off-by: Sergio Lopez <slp@redhat.com>
Enable set_xattr_stat to receive a bindings::stat struct so we it can be provided in the cases where we've already obtained it, saving an additional stat syscall. Signed-off-by: Sergio Lopez <slp@redhat.com>
slp
force-pushed
the
virtiofs-killpriv-v2
branch
2 times, most recently
from
0d3a1cb to
2d7b4b2
Compare
The methods [set|get]_attr_stat are used to store and read the guest ownership and permission values as extended attributes on APFS. So far, they were treated in an all-or-nothing fashion: you either have both ownership and permission data, or nothing. Also, when those values were missing, we defaulted to a very conservative root ownership. There are situations in which we may only have partial information (only ownership or only permission). For instance, this happens on files created from the host when the guest attempts to change ownership or the permission bits. In this change, we extend the format to of the attribute we store on APFS to allow having missing fields (signalled with an "x" instead of a u32). Also, when a field is missing, we use the host's ownership and permission bits instead of defaulting to root. This change is backwards compatible, as older versions of libkrun generate the same format strings for the extended attribute, but without missing fields. Signed-off-by: Sergio Lopez <slp@redhat.com>
slp
force-pushed
the
virtiofs-killpriv-v2
branch
from
2d7b4b2 to
0c1ddbd
Compare
mtjhrc
approved these changes
Feb 17, 2026
Collaborator
mtjhrc
left a comment
mtjhrc
left a comment
There was a problem hiding this comment.
Code LGTM, I am not very familiar with the all the semantics at play here, but I didn't find anything obviously wrong. Thanks!
pftbest
reviewed
Feb 17, 2026
| fd, | ||
| XATTR_KEY.as_ptr() as *const i8, | ||
| buf.as_mut_ptr() as *mut libc::c_void, | ||
| 64, |
Contributor
There was a problem hiding this comment.
Buffer size passed here is 64, but the actual Vec is only 32 bytes if I read this correctly.
Contributor
There was a problem hiding this comment.
Sorry, I missed this was already merged
Collaborator
Author
There was a problem hiding this comment.
@pftbest Good catch! We should change those hard coded numbers with a constant = 64. Since you found it, do you want to create a PR fixing it or should I do it myself?
Contributor
There was a problem hiding this comment.
@slp Please go ahead, I won't have time to fix this today.
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
4 participants
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.
While we were announcing it, we never really supported KILLPRIV, so this was breaking some fs tests. This PR stops announcing KILLPRIV support, and implements support for KILLPRIV_V2.