Re: [PATCH] bpf: Fix smp_processor_id() call trace for preemptible kernels

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Silencing a preemption warning with `raw_smp_processor_id()` leaves a severe cross-CPU task migration hazard intact, leading to lockless list corruption in the BPF memory allocator.
--

commit b3d3adf96792401d9a691908ba210c6315bac1bb
Author: Edward Adam Davis <eadavis@qq.com>

bpf: Fix smp_processor_id() call trace for preemptible kernels

This patch attempts to resolve a preemption warning in
bpf_mem_cache_free_rcu() by replacing this_cpu_ptr() with per_cpu_ptr()
and raw_smp_processor_id().

Closes: https://syzkaller.appspot.com/bug?extid=fd7e415d891073b83e1f

> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
> index e9662db7198fe..f2b7828e6ac7c 100644
> --- a/kernel/bpf/memalloc.c
> +++ b/kernel/bpf/memalloc.c
> @@ -951,7 +951,7 @@ void notrace bpf_mem_cache_free_rcu(struct bpf_mem_alloc *ma, void *ptr)
>  	if (!ptr)
>  		return;
>  
> -	unit_free_rcu(this_cpu_ptr(ma->cache), ptr);
> +	unit_free_rcu(per_cpu_ptr(ma->cache, raw_smp_processor_id()), ptr);

[Severity: High]
This is a pre-existing issue with bpf_mem_cache_free_rcu() being called in a
preemptible context, but does silencing the warning with
raw_smp_processor_id() leave the underlying cross-CPU task migration hazard
intact?

If a task evaluates raw_smp_processor_id(), gets preempted, and then migrates
to another CPU, unit_free_rcu() will execute on the new CPU while still
operating on the original CPU's cache.

Since unit_free_rcu() only disables local interrupts for the current CPU,
the original CPU can concurrently modify its own cache. This could lead to
data races on the local_t active counter and concurrent execution of the
non-atomic __llist_add() operation, as seen here:

kernel/bpf/memalloc.c:unit_free_rcu() {
    ...
    local_irq_save(flags);
    if (local_inc_return(&c->active) == 1) {
        if (__llist_add(llnode, &c->free_by_rcu))
    ...
}

Could this concurrent access corrupt the lockless lists?

>  }
>  
>  /* Directly does a kfree() without putting 'ptr' back to the free_llist

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_C4C3064909A16EDCAEDB0C8767D886D01206@qq.com?part=1