> 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.
> 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