Re: [PATCH bpf-next v5 2/7] exec: Clear bprm->mm before dropping its reference

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch addresses this exact class of BPF Use-After-Free (UAF) vulnerability for `bprm->mm` while missing that `bprm->file` suffers from the same flaw. The BPF verifier explicitly marks `bprm->file` as a fully trusted pointer via `BTF_TYPE_SAFE_TRUSTED(struct linux_binprm) { struct file *file; };`. In `free_bprm()`, the file's reference is dropped via `do_close_execat(bprm->file)`, but the pointer is not cleared. Immediately afterward, `bprm_drop_loader(bprm)` is called, which serves as a BPF fentry tracepoint. A BPF program attaching to this hook will receive a dangling `bprm->file` that the verifier incorrectly considers `PTR_TRUSTED` and completely safe to pass to kfuncs, bypassing BPF safety guarantees.
--

commit a611d673e7752074063d4d9476d61d335949964b
Author: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>

exec: Clear bprm->mm before dropping its reference

The patch clears bprm->mm to NULL before dropping its reference via mmput()
to ensure safe access from BPF kfuncs.

> diff --git a/fs/exec.c b/fs/exec.c
> index 745f6eb5279e6..4ddd403fd91c7 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1456,9 +1456,12 @@ void bprm_drop_loader(struct linux_binprm *bprm)
>  
>  static void free_bprm(struct linux_binprm *bprm)
>  {
> -	if (bprm->mm) {
> +	struct mm_struct *mm = bprm->mm;
> +
> +	if (mm) {
>  		acct_arg_size(bprm, 0);
> -		mmput(bprm->mm);
> +		bprm->mm = NULL;
> +		mmput(mm);
>  	}
>  	if (bprm->user_ns)
>  		put_user_ns(bprm->user_ns);

[Severity: High]
This isn't a bug introduced by this patch, but while it addresses a BPF
Use-After-Free vulnerability for bprm->mm, it appears bprm->file in the same
function suffers from the same flaw.

Further down in free_bprm(), the reference to bprm->file is dropped via
do_close_execat(), but the pointer itself is not cleared:

    do_close_execat(bprm->file);
    bprm_drop_loader(bprm);

Immediately afterward, bprm_drop_loader() is called. This acts as a BPF
fentry attach point where a BPF program can access the dangling bprm->file.

The BPF verifier explicitly marks bprm->file as a fully trusted pointer via
BTF_TYPE_SAFE_TRUSTED(struct linux_binprm) { struct file *file; };.

Because bprm->file is left dangling, a BPF fentry program attached to
bprm_drop_loader() will receive a pointer to a released file that the verifier
incorrectly considers PTR_TRUSTED and safe to pass to kfuncs, bypassing BPF
safety guarantees.

Should we apply the same pattern here, storing bprm->file in a local variable,
setting bprm->file to NULL, and then calling do_close_execat() on the local
variable to prevent BPF programs from accessing a zero-refcount object?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907165220.52431-1-tasos.papagiannnis@gmail.com?part=2