Re: [PATCH RESEND bpf-next v10 7/8] bpf: allow non-owning list-node args via __nonown_allowed

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] bpf_list_add() dynamically frees the 'new' node on failure, but the verifier unconditionally assumes success, enabling a Use-After-Free.
- [Critical] bpf_list_del() does not invalidate the passed non-owning reference, leaving a direct alias to potentially freed memory (Use-After-Free).
--

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

bpf: allow non-owning list-node args via __nonown_allowed

This commit introduces the __nonown_allowed annotation for list-node
arguments. It allows bpf_list_add, bpf_list_del, and bpf_list_is_first/last
to accept non-owning references.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
[ ... ]
> @@ -2571,10 +2571,10 @@ __bpf_kfunc int bpf_list_push_back_impl(struct bpf_list_head *head,
>  }
>  
>  __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 *prev__nonown_allowed,
> +			     struct btf_struct_meta *meta, u64 off)
>  {
> -	struct bpf_list_node_kern *n = (void *)new, *p = (void *)prev;
> +	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);

If a BPF program passes a non-owning reference from a completely different
list (protected by the same spinlock) to prev__nonown_allowed, can this
cause a use-after-free?

At runtime, __bpf_list_add() detects that the prev node doesn't belong to
the target head and aborts the insertion, calling __bpf_obj_drop_impl()
on the new node to free it back to the allocator.

However, the verifier unconditionally assumes bpf_list_add() successfully
consumed the new owning reference and safely converts it into a non-owning
reference in its internal state.

Because the verifier considers this non-owning reference valid until the
spinlock is unlocked, does this allow the BPF program to continuously
dereference the freed memory while holding the lock?

[ ... ]
> @@ -2620,9 +2620,9 @@ __bpf_kfunc struct bpf_list_node *bpf_list_pop_back(struct bpf_list_head *head)
>  }
>  
>  __bpf_kfunc struct bpf_list_node *bpf_list_del(struct bpf_list_head *head,
> -					       struct bpf_list_node *node)
> +				struct bpf_list_node *node__nonown_allowed)
>  {
> -	struct bpf_list_node_kern *kn = (void *)node;
> +	struct bpf_list_node_kern *kn = (void *)node__nonown_allowed;
>  
>  	/* verifier guarantees node is a list node rather than list head */
>  	return __bpf_list_del(head, &kn->list_head);

Does passing a non-owning reference here create an alias that leads to
a use-after-free?

When bpf_list_del() removes the node from the list, it returns a new owning
reference (with ref_obj_id > 0) pointing to the exact same object.

The verifier does not explicitly invalidate the original non-owning
reference argument (which has ref_obj_id == 0), so it remains fully valid
in the verifier's state until the active spinlock is released.

If the BPF program subsequently frees the object by passing the returned
owning reference to bpf_obj_drop(), won't the original non-owning reference
remain as a direct, unchecked alias to the freed memory?

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