Re: [PATCH bpf-next v1 2/2] bpf: Fix is_tracing_prog_type() to cover fentry/fexit/fmod_ret

["Kumar Kartikeya Dwivedi" <memxor@gmail.com>]

On Sun Jul 19, 2026 at 4:09 PM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [High] Tracing programs can bypass the bpf_spin_lock restriction using bpf_map_update_elem() with BPF_F_LOCK, leading to per-CPU IRQ state corruption.
> - [Medium] EXT programs extending iterators are incorrectly classified as tracing programs, restricting them from using spin locks, lists, and rbtrees.
>
> Pre-existing issues:
> - [High] Kernel warning and memory leak in `bpf_obj_free_fields` when freeing map elements containing lists or rbtrees protected by `bpf_res_spin_lock`.
> --
>
> --- Patch [2]: [PATCH bpf-next v1 2/2] bpf: Fix is_tracing_prog_type() to cover fentry/fexit/fmod_ret ---
> commit 94d71df738bb27d6ad27a15a0ce402fd7032420d
> Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>
> bpf: Fix is_tracing_prog_type() to cover fentry/fexit/fmod_ret
>
> This patch tightens the tracing program type checks and delays the evaluation
> of map spin lock and list restrictions to the point of use rather than during
> map compatibility checks.
>
>> Fix this by tightening the check, and moving predicates checking for the
>> restriction closer to the point of use. This change might affect
>> existing programs using them in unsafe contexts, but in such cases, it
>> is recommended for users to switch over to bpf_res_spin_lock instead.
>
> [Severity: High]
> This is a pre-existing issue, but recommending users switch to
> bpf_res_spin_lock exposes an issue in bpf_obj_free_fields() when freeing
> map elements containing lists or rbtrees.
>
> When btf_parse_fields() encounters BPF_RES_SPIN_LOCK, it sets
> res_spin_lock_off but leaves spin_lock_off as -EINVAL:
>
> kernel/bpf/btf.c:btf_parse_fields() {
>     ...
>     rec->spin_lock_off = -EINVAL;
>     rec->res_spin_lock_off = -EINVAL;
>     ...
> }
>
> Later, bpf_obj_free_fields() unconditionally checks spin_lock_off:
>
> kernel/bpf/syscall.c:bpf_obj_free_fields() {
>     ...
>     if (WARN_ON_ONCE(rec->spin_lock_off < 0))
>         continue;
>     ...
> }
>
> Could this trigger a WARN_ON_ONCE and leak memory for list/rbtree nodes
> because the check skips the freeing logic when a bpf_res_spin_lock is
> used instead of a standard bpf_spin_lock?
>

Sent fix for this.

>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -17769,9 +17780,11 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env,
> [ ... ]
>> -static bool is_tracing_prog_type(enum bpf_prog_type type)
>> +static bool is_tracing_prog_type(struct bpf_prog *prog)
>>  {
>> -	switch (type) {
>> +	switch (resolve_prog_type(prog)) {
>> +	case BPF_PROG_TYPE_TRACING:
>> +		return prog->expected_attach_type != BPF_TRACE_ITER;
>
> [Severity: Medium]
> Does this check correctly handle extension programs (BPF_PROG_TYPE_EXT)?
>
> For an extension program, the resolved type is BPF_PROG_TYPE_TRACING, but
> its expected_attach_type is its own attach type (typically 0), not the
> target's attach type which is stored in saved_dst_attach_type.
>
> Could this cause valid extension programs that extend iterators to be
> erroneously rejected by the verifier when trying to use spin locks, lists,
> or rbtrees?
>

This is a separate problem in multiple places, let's keep this one as is for
now, bpf_prog_check_recur() has similar issue. We need audit and fix all places
properly.

>
>> @@ -17802,14 +17815,6 @@ static int check_map_prog_compatibility(struct bpf_verifier_env *env,
> [ ... ]
>> -	if (btf_record_has_field(map->record, BPF_SPIN_LOCK)) {
>> -		if (is_tracing_prog_type(prog_type)) {
>> -			verbose(env, "tracing progs cannot use bpf_spin_lock yet\n");
>> -			return -EINVAL;
>> -		}
>> -	}
>> +	/*
>> +	 * Restrictions on using BPF list,rbtree,spin_lock is checked later upon
>> +	 * use, since rejecting accesing map containing them in programs is too
>> +	 * conservative.
>> +	 */
>
> [Severity: High]
> Does removing this map-level check allow tracing programs to bypass the
> lock restriction using bpf_map_update_elem() with the BPF_F_LOCK flag?
>
> While the explicit lock helper calls are now blocked in process_spin_lock(),
> the BPF_F_LOCK flag bypasses those verifier checks and executes
> copy_map_value_locked():
>
> kernel/bpf/helpers.c:copy_map_value_locked() {
>     ...
>     __bpf_spin_lock_irqsave(lock);
>     ...
> }
>
> Can this corrupt the irqsave_flags per-CPU variable when tracing programs
> execute in NMI or arbitrary IRQ contexts, potentially leading to deadlocks?

Hmm, valid point. Let me think about this, I might have to keep it during map
compatibility checks, and see how tests should be adjusted.