Re: [PATCH bpf-next 3/5] selinux: use kernel sid in security_bpf_*

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] A Time-of-Check to Time-of-Use (TOCTOU) race condition in SELinux BPF hooks allows unprivileged users to steal the `creator_sid` of a highly-privileged file (e.g., on procfs), bypassing SELinux policy restrictions for BPF operations.
--

--- Patch [3]: [PATCH bpf-next 3/5] selinux: use kernel sid in security_bpf_* ---
commit 74fdd38ce08ed06b1a82fae50df139cbd161c51e
Author: Thiébaud Weksteen <[email protected]>

selinux: use kernel sid in security_bpf_*

The security_bpf hooks provides a boolean to indicate if the call is
coming from within the kernel or not. If true, use the kernel SID
instead of relying on the current process SID. For the token-aware
functions, the kernel sid is used to decide on the access, but the caller
remains owner of the object (program or map).

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f197cf476190d..e7c5993f69545 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c

[ ... ]

> @@ -7296,7 +7296,7 @@ static int selinux_bpf_map_create(struct bpf_map *map, union bpf_attr *attr,
>  	bpfsec->sid = current_sid();
>  
>  	if (!token)
> -		ssid = bpfsec->sid;
> +		ssid = kernel ? SECINITSID_KERNEL : bpfsec->sid;
>  	else
>  		ssid = selinux_bpffs_creator_sid(attr->map_token_fd);

[Severity: Critical]
This is a pre-existing issue, but does using selinux_bpffs_creator_sid()
with the raw attr->map_token_fd here expose a Time-of-Check to Time-of-Use
(TOCTOU) race condition?

Since this performs a secondary file descriptor lookup, could a concurrent
thread replace the file descriptor (e.g., via dup3()) with a highly
privileged file before this SELinux hook runs?

If the core kernel has already safely resolved the token object, would it be
safer to use the token's security struct directly instead of performing a
completely new FD table lookup?

> @@ -7314,7 +7314,7 @@ static int selinux_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr,
>  	bpfsec->sid = current_sid();
>  
>  	if (!token)
> -		ssid = bpfsec->sid;
> +		ssid = kernel ? SECINITSID_KERNEL : bpfsec->sid;
>  	else
>  		ssid = selinux_bpffs_creator_sid(attr->prog_token_fd);

[Severity: Critical]
This is a pre-existing issue, but similar to selinux_bpf_map_create() above,
does this secondary lookup of attr->prog_token_fd allow an attacker to
swap the file descriptor mid-syscall and bypass SELinux restrictions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3