Re: [PATCH v2] 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:
- [High] The patch addresses the syzbot warning at the wrong layer by adding `get_cpu()`/`put_cpu()` inside the BPF memory allocator. This masks the caller's missing migration protection while leaving sibling allocator APIs vulnerable to the exact same crash.
--

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

bpf: Fix smp_processor_id() call trace for preemptible kernels

This patch wraps unit_free_rcu() with get_cpu() and put_cpu() in
bpf_mem_cache_free_rcu() to avoid a preemption warning.

Reported-by: syzbot+fd7e415d891073b83e1f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=fd7e415d891073b83e1f

> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
> index e9662db7198fe..8f91f2b3848ff 100644
> --- a/kernel/bpf/memalloc.c
> +++ b/kernel/bpf/memalloc.c
> @@ -951,7 +951,9 @@ void notrace bpf_mem_cache_free_rcu(struct bpf_mem_alloc *ma, void *ptr)
>  	if (!ptr)
>  		return;
>  

[Severity: High]
Does this fix the issue at the right layer? By adding the preemption
protection here, it masks the fact that the caller is running with migration
enabled, which leaves sibling APIs vulnerable.

For example, in __rhtab_map_lookup_and_delete_batch() in
kernel/bpf/hashtab.c, the batch loop iterates and calls rhtab_delete_elem()
without disabling migration.

If the map elements contain special fields (like BPF_KPTR),
rhtab_delete_elem() will first call rhtab_check_and_free_fields(), which
eventually calls bpf_mem_free().

Since bpf_mem_free() also uses this_cpu_ptr() without get_cpu() protection:

kernel/bpf/memalloc.c:bpf_mem_free() {
    ...
    unit_free(this_cpu_ptr(ma->caches)->cache + idx, ptr);
    ...
}

Will this result in the exact same BUG being triggered on that path? Should
the missing migration protection be added to
__rhtab_map_lookup_and_delete_batch() instead of modifying the allocator
here?

> +	get_cpu();
>  	unit_free_rcu(this_cpu_ptr(ma->cache), ptr);
> +	put_cpu();
>  }

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