在 2026/4/28 02:43, [email protected] 写道:
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>
> [ ... ]
>
>> @@ -2575,12 +2577,16 @@ static struct bpf_list_node *__bpf_list_del(struct
>> bpf_list_head *head,
>>
>> __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);
>> }
>
> This addresses an unaddressed review comment from Emil Tsalapatis in v8.
> The 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 the function will eventually initialize the list and return NULL without
> dereferencing the NULL pointer:
>
> kernel/bpf/helpers.c:__bpf_list_del() {
> if (unlikely(!h->next)) {
> INIT_LIST_HEAD(h);
> return NULL;
> }
> ...
> }
>
> this is counterintuitive - passing a potentially garbage value that is later
> ignored.
>
> Emil suggested moving the init check logic into the top-level kfuncs
> (bpf_list_pop_front/bpf_list_pop_back) to ensure the list_head passed 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.
>
> The same issue applies to bpf_list_pop_back() which reads h->prev before
> the initialization check.
>
> Original comment from Emil Tsalapatis (Thu, 19 Mar 2026 12:17:50 -0400):
> https://lore.kernel.org/bpf/[email protected]/
>
> "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."
>
> This issue remains unaddressed in v10.
>
>
> ---
> 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/25009536772
The '__bpf_list_del' function is a static function, so its scope and
associated risk are both very limited. I think its purpose is code
reuse and avoiding duplication.
If we move 'unlikely(!h->next)' to the outer callers, I believe it
brings no benefit other than making the code more redundant. Also,
I think validating a function's parameters before execution is not
counterintuitive; on the contrary, for global functions it is almost
a hard requirement.
--
Thanks
Kaitao Cheng
