On Mon Mar 16, 2026 at 7:28 AM EDT, Chengkaitao wrote:
> From: Kaitao Cheng <[email protected]>
>
> 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 <[email protected]>
> ---
>  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)


Reply via email to