Re: [bpf-next v3 1/2] bpf: Offload kptr destructors that run from NMI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] TOCTOU race in `bpf_dtor_kptr_release_one()` can lead to NMI deadlocks.
- [Low] Memory leak on initialization failure in `bpf_dtor_kptr_init()`.
--

commit 488cdc390b90427a591d47211600c09addc95fcd
Author: Justin Suess <utilityemal77@gmail.com>
Date:   Thu May 7 17:54:52 2026 +0000

    bpf: Offload kptr destructors that run from NMI

This commit queues destructor-backed kptr teardowns to irq_work when
running from NMI context. It maintains an idle-slot surplus counter to
ensure offload slots are preallocated to prevent deadlocks.

Closes: https://lore.kernel.org/bpf/20260421201035.1729473-1-utilityemal77@gmail.com/

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 3b1f0ba02f613..d34fdb99eb8a9 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
[ ... ]
> @@ -65,6 +69,111 @@ static DEFINE_SPINLOCK(map_idr_lock);
> +static void bpf_dtor_kptr_release_one(void)
> +{
> +	struct bpf_dtor_kptr_work *job;
> +	long surplus;
> +
> +	for (;;) {
> +		surplus = atomic_long_read(&bpf_dtor_kptr_surplus);
> +		if (surplus <= 0)
> +			return;
> +
> +		job = bpf_dtor_kptr_pop_idle();

Could this sequence lead to a race condition that drains the idle pool
too early?

If multiple threads read a positive surplus value here, they might all call
bpf_dtor_kptr_pop_idle() before the cmpxchg executes. This could temporarily
drain the idle pool below the required number of reserved slots.

If an NMI fires during this window and attempts to tear down a kptr, would
bpf_dtor_kptr_offload() find the idle pool empty? If so, it looks like it
would trigger the fallback warning and run the destructor inline from the
NMI context, which might reintroduce the deadlocks this patch aims to
prevent.

> +		if (!job)
> +			return;
> +
> +		if (!atomic_long_try_cmpxchg(&bpf_dtor_kptr_surplus, &surplus,
> +						     surplus - 1)) {
> +			pcpu_freelist_push(&bpf_dtor_kptr_idle, &job->fnode);
> +			continue;
> +		}
> +
> +		bpf_mem_free(&bpf_global_ma, job);
> +		return;
> +	}
> +}
[ ... ]
> +static int __init bpf_dtor_kptr_init(void)
> +{
> +	int err;
> +
> +	err = pcpu_freelist_init(&bpf_dtor_kptr_idle);
> +	if (err)
> +		return err;
> +
> +	err = pcpu_freelist_init(&bpf_dtor_kptr_jobs);
> +	if (err)
> +		return err;

Does this error path leak the per-cpu memory allocated for the idle list?

If the second pcpu_freelist_init() fails, the function returns immediately.
Should there be a cleanup path here to free the already initialized
bpf_dtor_kptr_idle list?

> +
> +	return 0;
> +}
> +late_initcall(bpf_dtor_kptr_init);

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