Re: [PATCH bpf-next v1 1/5] bpf: Treat non-iterator tracing progs as tracing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Tracing programs can deadlock the kernel by acquiring `bpf_spin_lock` via `bpf_map_update_elem` with the `BPF_F_LOCK` flag, bypassing the verifier's safety checks.
--

commit 0ab13739c010beba3fc294b839936a5123924643
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Treat non-iterator tracing progs as tracing

This patch updates the is_tracing_prog_type() helper to include non-iterator
tracing programs. It modifies check_map_prog_compatibility() to defer rejecting
tracing progs that access maps with bpf_spin_lock until their first use,
avoiding overly conservative rejections.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index ed7ba0e6a9ce0..26bfb44657253 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -17697,24 +17712,16 @@ static int check_map_prog_compatibility(struct bpf_verifier_env *env,
>  		return -EACCES;
>  	}
>  
>  	if (btf_record_has_field(map->record, BPF_SPIN_LOCK | BPF_RES_SPIN_LOCK)) {
>  		if (prog_type == BPF_PROG_TYPE_SOCKET_FILTER) {
>  			verbose(env, "socket filter progs cannot use bpf_spin_lock yet\n");
>  			return -EINVAL;
>  		}
> -
> -		if (is_tracing_prog_type(prog_type)) {
> -			verbose(env, "tracing progs cannot use bpf_spin_lock yet\n");
> -			return -EINVAL;
> -		}
> +		/*
> +		 * Rejecting tracing progs accessing maps with bpf_spin_lock in
> +		 * them here would be too conservative; let's defer rejection
> +		 * until seeing first use.
> +		 */

[Severity: High]
Is it possible for tracing programs to deadlock the kernel by acquiring
the spinlock via bpf_map_update_elem()?

By deferring the rejection to process_spin_lock(), we prevent explicit
bpf_spin_lock() calls in the tracing program. However, tracing programs might
still be able to call bpf_map_update_elem() with the BPF_F_LOCK flag.

If a tracing program passes BPF_F_LOCK:

kernel/bpf/hashtab.c:htab_map_update_elem() {
	if (unlikely(map_flags & BPF_F_LOCK)) {
		if (unlikely(!btf_record_has_field(map->record, BPF_SPIN_LOCK)))
			return -EINVAL;
    ...
}

This leads to copy_map_value_locked() which takes the lock:

kernel/bpf/helpers.c:copy_map_value_locked() {
    ...
	preempt_disable();
	__bpf_spin_lock_irqsave(lock);
    ...
}

Since tracing programs can run in NMI context, if an NMI interrupts another
context that is currently holding this exact spinlock, wouldn't it result in
an AA deadlock?

Does the verifier block BPF_F_LOCK dynamically at load time, or does removing
this map compatibility check expose us to this deadlock vector?

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