All of lore.kernel.org
 help / color / mirror / Atom feed
From: Justin Suess <utilityemal77@gmail.com>
To: sashiko@lists.linux.dev
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next 1/2] bpf: Implement dtor for struct file BTF ID
Date: Tue, 21 Apr 2026 15:38:23 -0400	[thread overview]
Message-ID: <aefSL_kZ8GSbpdfH@zenbox> (raw)
In-Reply-To: <aebef2Mz9fLoChwK@zenbox>

On Mon, Apr 20, 2026 at 10:18:39PM -0400, Justin Suess wrote:
> On Tue, Apr 21, 2026 at 01:05:35AM +0000, sashiko-bot@kernel.org wrote:
> > Sashiko AI review found 1 potential issue(s):
> > - [High] bpf_file_release_dtor() directly calls fput(), which is unsafe in NMI context and can lead to a system deadlock.
> > --
> > 
> > commit b9990bf55467191e86e004a62cdb84e6dc71b794
> > Author: Justin Suess <utilityemal77@gmail.com>
> > 
> > bpf: Implement dtor for struct file BTF ID
> > 
> > This commit implements a destructor for struct file, allowing it to be stored
> > in a BPF map as a referenced kptr. The destructor calls fput() to release
> > the reference.
> > 
> > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > > index 2bb60200c2665..ee5ceb82e4f81 100644
> > > --- a/kernel/bpf/helpers.c
> > > +++ b/kernel/bpf/helpers.c
> > [ ... ]
> > > @@ -2752,6 +2753,13 @@ __bpf_kfunc void bpf_task_release_dtor(void *p)
> > >  }
> > >  CFI_NOSEAL(bpf_task_release_dtor);
> > >  
> > > +__bpf_kfunc void bpf_file_release_dtor(void *file)
> > > +{
> > > +	fput((struct file *)file);
> > > +}
> > 
> > Is it safe to call fput() here?
> >
> > BPF programs can update or delete map elements from NMI context (for example,
> > in BPF_PROG_TYPE_PERF_EVENT programs), which will synchronously invoke this
> > destructor.
> > 
> > When fput() is called in an interrupt context, it adds the file to the
> > delayed_fput_list and calls schedule_delayed_work(). However,
> > schedule_delayed_work() is not NMI-safe, as it attempts to acquire spinlocks
> > such as the workqueue pool lock and timer_base->lock.
> > 
> > If the NMI interrupted a context that already held one of these locks, won't
> > this result in a hard deadlock?
> > 
> > Would it be better to defer releasing the struct file using an NMI-safe
> > mechanism, similar to how other BPF destructors handle this?
> > 
> BPF_PROG_TYPE_PERF_EVENT allows the kfunc bpf_put_file which also just
> calls fput().
> 
> So if calling fput in BPF_PROG_TYPE_PERF_EVENT is safe for
> bpf_put_file, why would it be unsafe in a dtor running in the same
> context?
>
Disregard.

The AI was partially correct, but my course of investigation revealed a
real bug / reproducible deadlock in upstream.

In addition to my patch having an NMI unsafe dtor, the existing dtors
are also unsafe in NMI handlers.

I was able to make a reliable reproducer that deadlocks the kernel with
the task_struct dtor on upstream (without this patch).

I'll send the full report in a bit.

Justin
> > -- 
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260420203306.3107246-1-utilityemal77@gmail.com?part=1

  reply	other threads:[~2026-04-21 19:38 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-20 20:33 [PATCH bpf-next 0/2] Allow storing referenced struct file kptrs in BPF maps Justin Suess
2026-04-20 20:33 ` [PATCH bpf-next 1/2] bpf: Implement dtor for struct file BTF ID Justin Suess
2026-04-20 22:17   ` Song Liu
2026-04-21  1:05   ` sashiko-bot
2026-04-21  2:18     ` Justin Suess
2026-04-21 19:38       ` Justin Suess [this message]
2026-04-21 20:29         ` Kumar Kartikeya Dwivedi
2026-04-20 20:33 ` [PATCH bpf-next 2/2] selftests/bpf: Add test for map-stored struct file kptrs Justin Suess
2026-04-20 21:07   ` bot+bpf-ci
2026-04-20 22:28   ` Song Liu
2026-04-21  1:31   ` sashiko-bot

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=aefSL_kZ8GSbpdfH@zenbox \
    --to=utilityemal77@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.