Re: [PATCH RESEND bpf-next v10 5/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] bpf_list_add drops the 'new' node on runtime failure, but the verifier unconditionally converts 'new' to a non-owning reference, leading to a reliable Use-After-Free.
- [High] The bpf_list_add kfunc is marked with KF_IMPLICIT_ARGS but lacks the required _impl counterpart, causing the verifier to unconditionally reject it.
--

commit 2a493a875461b6ae4bffbc4548a568dfaca01438
Author: Kaitao Cheng <chengkaitao@kylinos.cn>

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

This patch adds a new kfunc bpf_list_add() to insert a node after a given
list node in a BPF linked list. The new node must be an owning reference
which is consumed by the kfunc, and on failure, the kernel drops the
passed-in node.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 5388078f31710..2b8e8d4284a54 100644
> --- 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);
> +}

If __bpf_list_add() fails at runtime (e.g., because prev is not in the list
indicated by head), it drops the 'new' node:

__bpf_list_add()
...
fail:
	__bpf_obj_drop_impl((void *)n - off, rec, false);
	return -EINVAL;

However, the verifier unconditionally converts 'new' into a non-owning
reference during check_kfunc_call(). If __bpf_list_add() fails, the BPF program
retains this non-owning reference to a freed node. Could this allow the program
to trigger a use-after-free within the lock critical section?

[ ... ]
> @@ -4748,6 +4758,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)

Since bpf_list_add is marked with KF_IMPLICIT_ARGS, won't the verifier look
for a bpf_list_add_impl counterpart during program load?

Without defining and registering a bpf_list_add_impl function,
find_kfunc_impl_proto() will fail to find the required _impl suffix in the
vmlinux BTF. Will this cause the verifier to unconditionally reject the kfunc?

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