Re: [PATCH RESEND 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/5/12 14:41, 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)
> 
> The commit message states that 'prev' 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 owning references and rejects non-owning
> references.
> 
> Without a similar distinction for KF_ARG_PTR_TO_LIST_NODE (as exists for
> KF_ARG_PTR_TO_RB_NODE), is there any valid BPF register state that satisfies
> both the verifier check and the runtime semantics stated in the commit
> message?
> 
> The BPF CI bot reported this issue in v10. The author responded that this was
> fixed in [PATCH bpf-next v10 7/8], which introduces the __nonown_allowed
> annotation and corresponding verifier logic. However, patch 7/8 is not
> present in this commit (feed006fb969), so the parameter lacks the
> __nonown_allowed suffix and the verifier lacks the is_kfunc_arg_nonown_allowed()
> check.

If this is an issue that must be fixed, I will move [PATCH bpf-next v10 7/8]
earlier in the next version.

> Reference: https://lore.kernel.org/bpf/cb7368c6062232b6ad5f9ab1c6a97bc8530566a8b3db5c4deefdd5ea63d31538@mail.kernel.org/
> 
>> +{
>> +	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);
>> +}
>> +
> 
> [ ... ]
> 
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
> 
> [ ... ]
> 
>> @@ -19235,8 +19238,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.
>> +		 */
> 
> This isn't a bug, but the multi-line comment format doesn't follow the BPF
> subsystem style. The preferred form in kernel/bpf/ has the opening /* on its
> own line:
> 
>     /*
>      * 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/25716874656

-- 
Thanks
Kaitao Cheng