Re: [PATCH bpf-next 1/2] bpf: Implement dtor for struct file BTF ID
Justin Suess <[email protected]>
| Newsgroups | dev.linux.lists.sashiko,org.kernel.vger.bpf |
|---|---|
| Message-ID | <aebef2Mz9fLoChwK@zenbox> |
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? > -- > Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1