Re: [PATCH bpf-next v1 2/5] bpf: Reject bpf_obj_drop() from tracing progs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] BPF_TIMER and BPF_WORKQUEUE fields are incorrectly whitelisted as NMI-safe, causing a deadlock when bpf_obj_drop() is called from NMI context.
--

commit 3287c13e44b513ce0d9b41e4aa24cd264b519b04
Author: Justin Suess <utilityemal77@gmail.com>

bpf: Reject bpf_obj_drop() from tracing progs

This commit restricts bpf_obj_drop() and bpf_percpu_obj_drop() in tracing
programs to only allow objects whose fields are explicitly whitelisted as
NMI-safe. It replaces the previous blacklist approach with a whitelist to
better handle future field types.

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -492,6 +492,35 @@ static inline bool btf_record_has_field(const struct btf_record *rec, enum btf_f
>  	return rec->field_mask & type;
>  }
>  
> +static inline bool btf_field_is_nmi_safe(enum btf_field_type type)
> +{
> +	switch (type) {
> +	case BPF_SPIN_LOCK:
> +	case BPF_RES_SPIN_LOCK:
> +	case BPF_TIMER:
> +	case BPF_WORKQUEUE:

[Severity: High]
This is a pre-existing issue, but is it safe to include BPF_TIMER and
BPF_WORKQUEUE in this NMI-safe whitelist?

Dropping an object with these fields calls bpf_timer_cancel_and_free() or
bpf_wq_cancel_and_free(), which synchronously invokes bpf_prog_put() via
bpf_async_update_prog_callback():

kernel/bpf/helpers.c:bpf_async_cancel_and_free() {
    ...
    bpf_async_update_prog_callback(cb, NULL, NULL);
    ...
}

If this drops the last reference to the BPF program while in NMI context,
__bpf_prog_put() falls back to schedule_work() since irqs are disabled:

kernel/bpf/syscall.c:__bpf_prog_put() {
    if (in_hardirq() || irqs_disabled()) {
        INIT_WORK(&aux->work, bpf_prog_put_deferred);
        schedule_work(&aux->work);
    } else {
        ...
}

Since schedule_work() takes a raw spin lock on the workqueue pool, couldn't
this hard deadlock if the NMI interrupted another thread that was already
holding the workqueue pool lock?

Unlike BPF_TASK_WORK, which safely defers destruction via irq_work_queue
when irqs_disabled(), these fields do not appear to be NMI-safe.

> +	case BPF_TASK_WORK:
> +	case BPF_KPTR_UNREF:
> +	case BPF_REFCOUNT:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}

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