Re: [PATCH bpf-next 1/2] bpf: Implement dtor for struct file BTF ID
Kumar Kartikeya Dwivedi <[email protected]>
| Newsgroups | dev.linux.lists.sashiko,org.kernel.vger.bpf |
|---|---|
| Message-ID | <CAP01T77=cADB+DmuTc2+RMdYKDMjSCaMn16bBRnCgEyuc5GqzA@mail.gmail.com> |
On Tue, 21 Apr 2026 at 21:43, Justin Suess <[email protected]> wrote: > > On Mon, Apr 20, 2026 at 10:18:39PM -0400, Justin Suess wrote: > > On Tue, Apr 21, 2026 at 01:05:35AM +0000, [email protected] 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 <[email protected]> > > > > > > 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. This is an orthogonal issue, but what you're adding in this set makes sense to me, but please respin after addressing the nits and some more tests as Song suggested. Let's continue discussing the issue here in a separate thread that you began. pw-bot: cr > > Justin > > > -- > > > Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1 >