> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index baa12b24bb64..9cd7b028592c 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2550,22 +2550,24 @@ __bpf_kfunc int bpf_list_push_back_impl(struct
> bpf_list_head *head,
> return bpf_list_push_back(head, node, meta__ign, 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;
> struct bpf_list_node_kern *node;
>
> /* If list_head was 0-initialized by map, bpf_obj_init_field wasn't
> * called on its fields, so init here
> */
> - if (unlikely(!h->next))
> + if (unlikely(!h->next)) {
> INIT_LIST_HEAD(h);
> + return NULL;
> + }
> if (list_empty(h))
> return NULL;
>
> - n = tail ? h->prev : h->next;
> node = container_of(n, struct bpf_list_node_kern, list_head);
> - if (WARN_ON_ONCE(READ_ONCE(node->owner) != head))
> + if (unlikely(READ_ONCE(node->owner) != head))
> return NULL;
>
> list_del_init(n);
> @@ -2575,12 +2577,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);
> }
The new code reads h->next before __bpf_list_del checks if the list is
initialized. If the list is uninitialized (h->next is NULL), a NULL pointer
is passed to __bpf_list_del.
While __bpf_list_del eventually initializes the list and returns NULL without
dereferencing the NULL pointer, this is counterintuitive - passing a
potentially garbage value that is later ignored.
Can you move the initialization check into the top-level kfuncs
(bpf_list_pop_front and bpf_list_pop_back) to ensure the list_head passed to
__bpf_list_del is always valid? You could initialize the list and return NULL
in that case - we know it's empty.
This concern was raised by Emil Tsalapatis in v8:
https://lore.kernel.org/bpf/[email protected]/
>
> __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);
> }
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25716874656
