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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The `bpf_list_add` kfunc frees the inserted node on runtime failure, but the BPF verifier unconditionally converts it to a non-owning reference, leading to a Use-After-Free (UAF) vulnerability.
- [High] The `bpf_list_add` kfunc is registered with `KF_IMPLICIT_ARGS` but lacks the required `_impl` function and BTF ID, causing the verifier to reject all programs attempting to call it.
--

commit 41c84b8182697296804a152b66297f50e3a604cb
Author: Kaitao Cheng <chengkaitao@kylinos.cn>

    bpf: Add bpf_list_add to insert node after a given list node

This commit adds a new kfunc bpf_list_add() that inserts 'new' after 'prev'
in a BPF linked list. The new node must be an owning reference, which the
kfunc consumes and converts to a non-owning reference upon insertion.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 1c69476c8a094..89579165ef4dc 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
[ ... ]
> @@ -2577,6 +2577,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__nonown_allowed,
> +			     struct btf_struct_meta *meta, u64 off)
> +{
> +	struct bpf_list_node_kern *n = (void *)new, *p = (void *)prev__nonown_allowed;
> +	struct list_head *prev_ptr = &p->list_head;
> +
> +	return __bpf_list_add(n, head, &prev_ptr, meta ? meta->record : NULL, off);
> +}

[Severity: Critical]
Is it possible for this to result in a use-after-free if __bpf_list_add() fails?

Looking at __bpf_list_add(), it introduces a runtime check that fails if the
prev node doesn't belong to the target list. When it fails, the function
calls __bpf_obj_drop_impl() to free the new node and returns -EINVAL.

However, the BPF verifier treats bpf_list_add() as a list push operation.
During check_kfunc_call(), it unconditionally consumes the owning reference to
the new node and converts it to a non-owning reference, assuming static
success.

If the runtime check fails, the BPF program would be granted a valid
non-owning reference to the freed new node inside the spinlock critical
section, potentially leading to memory corruption.

[ ... ]
> @@ -4756,6 +4766,7 @@ BTF_ID_FLAGS(func, bpf_list_push_front, KF_IMPLICIT_ARGS)
>  BTF_ID_FLAGS(func, bpf_list_push_front_impl)
>  BTF_ID_FLAGS(func, bpf_list_push_back, KF_IMPLICIT_ARGS)
>  BTF_ID_FLAGS(func, bpf_list_push_back_impl)
> +BTF_ID_FLAGS(func, bpf_list_add, KF_IMPLICIT_ARGS)

[Severity: High]
Does this kfunc lack a required _impl function?

When a kfunc is marked with KF_IMPLICIT_ARGS, the verifier's
find_kfunc_impl_proto() explicitly requires a corresponding _impl function
to resolve the full prototype.

Since there is no bpf_list_add_impl() defined in kernel/bpf/helpers.c and
registered with BTF, fetch_kfunc_meta() will return -EINVAL. Will this cause
the verifier to unconditionally reject any BPF program attempting to use
this kfunc?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521032306.97118-1-kaitao.cheng@linux.dev?part=6