From: sashiko-bot@kernel.org
To: "Justin Suess" <utilityemal77@gmail.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v3 1/2] bpf: Implement dtor for struct file BTF ID
Date: Fri, 24 Apr 2026 19:59:57 +0000 [thread overview]
Message-ID: <20260424195958.AED4EC19425@smtp.kernel.org> (raw)
In-Reply-To: <20260424192252.695240-2-utilityemal77@gmail.com>
Sashiko AI review found 1 potential issue(s):
- [High] Calling `fput()` directly from `bpf_file_release_dtor` can lead to a hard deadlock when invoked from NMI context.
--
commit 6368f91be1fd91bc9cb38d3498f35d279bad04c9
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 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 2bb60200c2665e..ee5ceb82e4f812 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);
> +}
Can this cause a deadlock if the destructor is invoked from NMI context?
If an NMI-context BPF program (such as a perf event tracing program)
updates or deletes a map element containing a struct file kptr, the BPF
framework will synchronously invoke this destructor to clean up the old
kptr.
When fput() drops the reference count to zero, it calls
__fput_deferred(). Since in_interrupt() evaluates to true in NMI
context, __fput_deferred() bypasses task_work_add() and instead calls
schedule_delayed_work().
This leads to the following call chain:
bpf_file_release_dtor()
fput()
__fput_deferred()
schedule_delayed_work()
add_timer()
The add_timer() function attempts to acquire the timer_base->lock raw
spinlock. If the NMI interrupted a thread that was already holding this
lock, the NMI handler will spin indefinitely waiting for it, resulting
in a hard deadlock.
Would it be safer to defer the fput() operation using an explicitly
NMI-safe mechanism like irq_work_queue() to handle arbitrary BPF
contexts?
> +
> +CFI_NOSEAL(bpf_file_release_dtor);
> +
> #ifdef CONFIG_CGROUPS
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260424192252.695240-1-utilityemal77@gmail.com?part=1
next prev parent reply other threads:[~2026-04-24 19:59 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-24 19:22 [PATCH bpf-next v3 0/2] Allow storing referenced struct file kptrs in BPF maps Justin Suess
2026-04-24 19:22 ` [PATCH bpf-next v3 1/2] bpf: Implement dtor for struct file BTF ID Justin Suess
2026-04-24 19:52 ` bot+bpf-ci
2026-04-24 19:59 ` sashiko-bot [this message]
2026-04-24 20:12 ` Justin Suess
2026-04-24 22:05 ` Alexei Starovoitov
2026-04-24 23:20 ` Justin Suess
2026-04-25 1:25 ` Alexei Starovoitov
2026-04-25 2:17 ` Justin Suess
2026-04-24 19:22 ` [PATCH bpf-next v3 2/2] selftests/bpf: Add test for map-stored struct file kptrs Justin Suess
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=20260424195958.AED4EC19425@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=sashiko@lists.linux.dev \
--cc=utilityemal77@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 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.