Re: [PATCH bpf-next v8 2/8] bpf: refactor __bpf_list_del to take list node pointer

["Emil Tsalapatis" <emil@etsalapatis.com>]

On Mon Mar 16, 2026 at 7:28 AM EDT, Chengkaitao wrote:
> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>
> Refactor __bpf_list_del to accept (head, struct list_head *n) instead of
> (head, bool tail). The caller now passes the specific node to remove:
> bpf_list_pop_front passes h->next, bpf_list_pop_back passes h->prev.
>
> Prepares for introducing bpf_list_del(head, node) kfunc to remove an
> arbitrary node when the user holds ownership.
>
> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
> ---
>  kernel/bpf/helpers.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index cb6d242bd093..e87b263c5fe6 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2426,9 +2426,10 @@ __bpf_kfunc int bpf_list_push_back_impl(struct bpf_list_head *head,
>  	return __bpf_list_add(n, head, true, meta ? meta->record : NULL, off);
>  }
>  
> -static struct bpf_list_node *__bpf_list_del(struct bpf_list_head *head, bool tail)
> +static struct bpf_list_node *__bpf_list_del(struct bpf_list_head *head,
> +					    struct list_head *n)
>  {
> -	struct list_head *n, *h = (void *)head;
> +	struct list_head *h = (void *)head;

Note: The cast to void then back to list_head is necessary to avoid an
"incompatible pointer types" error.

>  	struct bpf_list_node_kern *node;
>  
>  	/* If list_head was 0-initialized by map, bpf_obj_init_field wasn't
> @@ -2439,7 +2440,6 @@ static struct bpf_list_node *__bpf_list_del(struct bpf_list_head *head, bool tai
>  	if (list_empty(h))
>  		return NULL;
>  
> -	n = tail ? h->prev : h->next;

The new code reads n _before_ we check if the list is initialized. So the n we
are passing from the caller may well be NULL. However, __bpf_list_del()
will in that case now a) call INIT_LIST_HEAD(() to properly set up
prev/next, b) call list_empty() on the newly initialized list and exit
without ever reading the NULL passed by the caller.

This is kind of counterintuitive: We are passing essentially a garbage
value to __bpf_list_del that we thankfully end upi ignoring. Can you
move the init check logic into the top-level kfuncs to make sure the
list_head we're passing to __bpf_list_del is always valid? You can also
just init the list and return NULL in that case - we know it's empty.

>  	node = container_of(n, struct bpf_list_node_kern, list_head);
>  	if (WARN_ON_ONCE(READ_ONCE(node->owner) != head))
>  		return NULL;
> @@ -2451,12 +2451,16 @@ static struct bpf_list_node *__bpf_list_del(struct bpf_list_head *head, bool tai
>  
>  __bpf_kfunc struct bpf_list_node *bpf_list_pop_front(struct bpf_list_head *head)
>  {
> -	return __bpf_list_del(head, false);
> +	struct list_head *h = (void *)head;
> +
> +	return __bpf_list_del(head, h->next);
>  }
>  
>  __bpf_kfunc struct bpf_list_node *bpf_list_pop_back(struct bpf_list_head *head)
>  {
> -	return __bpf_list_del(head, true);
> +	struct list_head *h = (void *)head;
> +
> +	return __bpf_list_del(head, h->prev);
>  }
>  
>  __bpf_kfunc struct bpf_list_node *bpf_list_front(struct bpf_list_head *head)