BPF List
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Anastasios Papagiannis" <tasos.papagiannnis@gmail.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v6 2/5] exec: Clear bprm->mm before dropping its reference
Date: Tue, 08 Sep 2026 14:06:57 +0000	[thread overview]
Message-ID: <20260908140720.1DDF91F00A3D@smtp.kernel.org> (raw)
In-Reply-To: <20260908135302.74963-3-tasos.papagiannnis@gmail.com>

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

Pre-existing issues:
- [High] The patch clears `bprm->mm` to prevent a BPF Use-After-Free, but misses `bprm->file`, which is also explicitly marked as a trusted pointer by the BPF verifier and is left dangling.
--

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

exec: Clear bprm->mm before dropping its reference

The patch ensures that once mmput() drops the final reference to bprm->mm,
the pointer is no longer accessible through struct linux_binprm. This prevents
BPF kfuncs from accessing a freed mm_struct.

> 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);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but while attempting to fix the BPF
use-after-free for bprm->mm, could this leave a dangling pointer in bprm->file
further down in free_bprm()?

In free_bprm(), the reference to bprm->file is dropped, but the pointer is not
cleared:

    static void free_bprm(struct linux_binprm *bprm)
    {
        ...
        do_close_execat(bprm->file);
        /* An unconsumed PT_INTERP substitute from a binfmt_misc loader entry. */
        bprm_drop_loader(bprm);
        ...
    }

Immediately after, the globally visible bprm_drop_loader(bprm) is called.
If a BPF fentry tracing program is attached to bprm_drop_loader, it receives
bprm as a trusted argument. Since the BPF verifier explicitly marks
bprm->file as BTF_TYPE_SAFE_TRUSTED, this exposes the stale pointer to the
tracing program as a trusted argument.

This could allow a use-after-free or direct memory load of a freed struct
file if the program passes this dangling pointer to kfuncs requiring a trusted
struct file *.

Would it be safer to also clear bprm->file after closing it here, similar to
how bprm->mm is being handled?

Note that fields like cred, user_ns, and old_mm do not have this issue because
they are not marked as trusted by the verifier and are treated as untrusted
PTR_TO_BTF_ID pointers.

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

  reply	other threads:[~2026-09-08 14:07 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-08 13:52 [PATCH bpf-next v6 0/5] bpf: Add user memory access kfuncs for mm_struct Anastasios Papagiannis
2026-09-08 13:52 ` [PATCH bpf-next v6 1/5] mm: Add copy_remote_mm_str() Anastasios Papagiannis
2026-09-08 14:07   ` David Hildenbrand (Arm)
2026-09-08 13:52 ` [PATCH bpf-next v6 2/5] exec: Clear bprm->mm before dropping its reference Anastasios Papagiannis
2026-09-08 14:06   ` sashiko-bot [this message]
2026-09-08 13:53 ` [PATCH bpf-next v6 3/5] bpf: Add user memory access kfuncs for mm_struct Anastasios Papagiannis
2026-09-08 16:18   ` bot+bpf-ci
2026-09-09  7:24     ` Anastasios Papagiannis
2026-09-08 13:53 ` [PATCH bpf-next v6 4/5] bpf: Mark linux_binprm->mm as trusted-or-null Anastasios Papagiannis
2026-09-08 13:53 ` [PATCH bpf-next v6 5/5] selftests/bpf: Test mm_struct user memory kfuncs with linux_binprm Anastasios Papagiannis
2026-09-08 16:18   ` bot+bpf-ci

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260908140720.1DDF91F00A3D@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=tasos.papagiannnis@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox