Re: [PATCH bpf-next v10 5/8] bpf: Add bpf_list_add to insert node after a given list node

[Kaitao Cheng <kaitao.cheng@linux.dev>]

在 2026/4/28 02:43, bot+bpf-ci@kernel.org 写道:
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -2570,6 +2570,16 @@ __bpf_kfunc int bpf_list_push_back_impl(struct bpf_list_head *head,
>>  	return bpf_list_push_back(head, node, meta__ign, off);
>>  }
>>
>> +__bpf_kfunc int bpf_list_add(struct bpf_list_head *head, struct bpf_list_node *new,
>> +			     struct bpf_list_node *prev, struct btf_struct_meta *meta,
>> +			     u64 off)
>> +{
>> +	struct bpf_list_node_kern *n = (void *)new, *p = (void *)prev;
>> +	struct list_head *prev_ptr = &p->list_head;
>> +
>> +	return __bpf_list_add(n, head, &prev_ptr, meta ? meta->record : NULL, off);
>> +}
>> +
> 
> The 'prev' argument is described in the commit message as "must already be
> in the list." How can a BPF program obtain such a reference and pass it to
> this kfunc?
> 
> A node already in the list is tracked by the verifier as a non-owning
> reference (PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF with ref_obj_id == 0).
> The typical way to obtain this is via bpf_list_front() or bpf_list_back(),
> which return non-owning references.
> 
> However, check_kfunc_args() in kernel/bpf/verifier.c handles
> KF_ARG_PTR_TO_LIST_NODE by requiring:
> 
>     if (reg->type != (PTR_TO_BTF_ID | MEM_ALLOC)) {
>         verbose(env, "%s expected pointer to allocated object\n", ...);
>         return -EINVAL;
>     }
>     if (!reg->ref_obj_id) {
>         verbose(env, "allocated object must be referenced\n");
>         return -EINVAL;
>     }
> 
> This check rejects non-owning references. The 'prev' argument in R3 is
> typed as struct bpf_list_node *, so the verifier classifies it as
> KF_ARG_PTR_TO_LIST_NODE and applies this validation.
> 
> Compare with KF_ARG_PTR_TO_RB_NODE, which distinguishes between insertion
> (requires owning ref) and other operations (allows non-owning refs):
> 
>     case KF_ARG_PTR_TO_RB_NODE:
>         if (meta->func_id == special_kfunc_list[KF_bpf_rbtree_add_impl]) {
>             /* owning ref required */
>         } else {
>             /* non-owning or refcounted allowed */
>         }
> 
> Without a similar distinction for KF_ARG_PTR_TO_LIST_NODE, is there any
> valid BPF register state that satisfies both the verifier check and the
> runtime semantics stated in the commit message?
> 
> It appears the API only works when 'prev' is an owning reference to a
> refcounted object still in the list. This is the pattern used in the
> selftests, but it limits the kfunc to a narrow use case that the commit
> message does not describe.

I have already fixed this issue in [PATCH bpf-next v10 7/8].

>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -10742,6 +10742,7 @@ enum special_kfunc_type {
>>  	KF_bpf_list_push_front,
>>  	KF_bpf_list_push_back_impl,
>>  	KF_bpf_list_push_back,
>> +	KF_bpf_list_add,
>>  	KF_bpf_list_pop_front,
>>  	KF_bpf_list_pop_back,
>>  	KF_bpf_list_del,
> 
> [ ... ]
> 
>> @@ -10923,7 +10925,8 @@ static bool is_bpf_list_push_kfunc(u32 func_id)
>>  	return func_id == special_kfunc_list[KF_bpf_list_push_front] ||
>>  	       func_id == special_kfunc_list[KF_bpf_list_push_front_impl] ||
>>  	       func_id == special_kfunc_list[KF_bpf_list_push_back] ||
>> -	       func_id == special_kfunc_list[KF_bpf_list_push_back_impl];
>> +	       func_id == special_kfunc_list[KF_bpf_list_push_back_impl] ||
>> +	       func_id == special_kfunc_list[KF_bpf_list_add];
>>  }
>>
>>  static bool is_bpf_rbtree_add_kfunc(u32 func_id)
>> @@ -19228,8 +19231,11 @@ struct bpf_insn addr[2] = { BPF_LD_IMM64(BPF_REG_2, (long)kptr_struct_meta) };
>>  		int struct_meta_reg = BPF_REG_3;
>>  		int node_offset_reg = BPF_REG_4;
>>
>> -		/* rbtree_add has extra 'less' arg, so args-to-fixup are in diff regs */
>> -		if (is_bpf_rbtree_add_kfunc(desc->func_id)) {
>> +		/* list_add/rbtree_add have an extra arg (prev/less),
>> +		 * so args-to-fixup are in diff regs.
>> +		 */
>> +		if (desc->func_id == special_kfunc_list[KF_bpf_list_add] ||
>> +		    is_bpf_rbtree_add_kfunc(desc->func_id)) {
>>  			struct_meta_reg = BPF_REG_4;
>>  			node_offset_reg = BPF_REG_5;
>>  		}
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25009536772

-- 
Thanks
Kaitao Cheng