Hi Pádraig,
Pádraig Brady <[email protected]> writes:
> On 31/10/2026 03:55, Collin Funk wrote:
>> if (specified_context == NULL)
>> {
>> - int status = (affect_symlink_referent
>> - ? getfileconat (fd, file, &file_context)
>> - : lgetfileconat (fd, file, &file_context));
>> -
>> - if (status < 0 && errno != ENODATA)
>> + /* libselinux versions before 3.4 did not support O_PATH.
>> + The O_PATH support works by opening the file descriptor under
>> + /proc/self/fd. We could check if fsetfilecon returns EBADF and
>> + do the same, but there is likely not much benefit over just
>> + using O_RDONLY. */
>> + fd = openat (dirfd, file,
>> + (O_RDONLY | O_NOCTTY | O_NONBLOCK
>> + | (affect_symlink_referent ? 0 : O_NOFOLLOW)));
>
>
> I dislike the real open for a metadata update.
> There may be side effects on devices etc.
> or requiring read permissions where previously we did not,
> and also symlinks could cause issues.
> E.g. the following now fails with ELOOP:
>
> d='testd'
> mkdir -p $d
> touch $d/file
> ln -s file $d/link
> se_type=$(stat -c %C $d/link | cut -d: -f3)
> src/chcon -h -t $se_type $d/link
>
> This is a tricky one due to the O_PATH limitation you commented on.
> Though libselinux 3.4 was 2022, and missing /proc would be very
> unusual on selinux systems, so it's probably fine to fall back
> to the existing implementation in that case. Note I would not
> fall back on any openat() failure as an attacker could be replacing
> a file so you'd want to bail out to avoid that. I.e. only fall back
> when fgetfilecon() returns EBADF.
> Note we could fail rather than fall back if any partial option is used,
> but that would be a bit too draconian for this I think.
Thanks for the input. I think I am entirely in agreement with you. I
just want to share a few things I noticed while investigating this, so
you are aware and can take them into account.
First, I think our implementation would currently fail with /proc
missing. See the current strace output before this patch:
$ mkdir -p a/b; strace chcon -R -u root a; rm -rf a
[...]
openat(AT_FDCWD, "a",
O_RDONLY|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY) = 3
fstat(3, {st_mode=S_IFDIR|0755, st_size=2, ...}) = 0
fcntl(3, F_GETFL) = 0x38800 (flags
O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_NOFOLLOW|O_DIRECTORY)
fcntl(3, F_SETFD, FD_CLOEXEC) = 0
fcntl(3, F_DUPFD_CLOEXEC, 3) = 4
[...]
lgetxattr("/proc/self/fd/4/b", "security.selinux",
"unconfined_u:object_r:gconf_home"..., 255) = 38
access("/var/run/setrans/.setrans-unix", F_OK) = -1 ENOENT (No such file or
directory)
futex(0x7f865971a5a0, FUTEX_WAKE_PRIVATE, 2147483647) = 0
lsetxattr("/proc/self/fd/4/b", "security.selinux",
"root:object_r:gconf_home_t:s0", 30, 0) = 0
[...]
I.e., the file descriptor functions in selinux-at.h use /proc.
If we use my patch, but with s/O_RDONLY/O_PATHSEARCH/g, then here is
what we see:
$ mkdir -p a/b; strace ./src/chcon -R -u root a; rm -rf a
[...]
openat(AT_FDCWD, "a",
O_RDONLY|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY) = 3
fstat(3, {st_mode=S_IFDIR|0755, st_size=2, ...}) = 0
fcntl(3, F_GETFL) = 0x38800 (flags
O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_NOFOLLOW|O_DIRECTORY)
fcntl(3, F_SETFD, FD_CLOEXEC) = 0
fcntl(3, F_DUPFD_CLOEXEC, 3) = 4
[...]
openat(4, "b", O_RDONLY|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW|O_PATH) = 3
fgetxattr(3, "security.selinux", 0x3f7ca9c0, 255) = -1 EBADF (Bad file
descriptor)
fcntl(3, F_GETFL) = 0x220000 (flags
O_RDONLY|O_NOFOLLOW|O_PATH)
getxattr("/proc/self/fd/3", "security.selinux",
"unconfined_u:object_r:gconf_home"..., 255) = 38
access("/var/run/setrans/.setrans-unix", F_OK) = -1 ENOENT (No such file or
directory)
futex(0x7f332549b5a0, FUTEX_WAKE_PRIVATE, 2147483647) = 0
fsetxattr(3, "security.selinux", "root:object_r:gconf_home_t:s0", 30, 0) =
-1 EBADF (Bad file descriptor)
fcntl(3, F_GETFL) = 0x220000 (flags
O_RDONLY|O_NOFOLLOW|O_PATH)
setxattr("/proc/self/fd/3", "security.selinux",
"root:object_r:gconf_home_t:s0", 30, 0) = 0
I.e., when we use O_PATH, libselinux-3.4 and later will get EBADF from
fgetxattr or fsetxattr, and call getxattr or setxattr respectively on
the /proc/self/fd path associated with the file descriptor. My
understanding is that the versions before libselinux-3.4 would just
return EBADF instead of that fallback.
This also means that a portable implementation could just open a file
descriptor with O_PATH, and call getfilecon and setfilecon on the
/proc/self/fd path directly, instead of letting libselinux handle the
fallback for us. WDYT?
Collin
