Re: [PATCH bpf v2] bpf: Fix UAF due to concurrent consumption of waiting_for_gp_ttrace

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch fixes concurrent consumption for `waiting_for_gp_ttrace` but incorrectly claims `free_by_rcu_ttrace` is safe. A race condition during cache destruction leads to a Use-After-Free on `free_by_rcu_ttrace`.
--

commit ed5f43858e59ce799a585eeedfe466c5543df6aa
Author: Pu Lehui <pulehui@huawei.com>
Subject: bpf: Fix UAF due to concurrent consumption of waiting_for_gp_ttrace

This commit fixes a use-after-free caused by concurrent consumption of
waiting_for_gp_ttrace nodes by alloc_bulk() and __free_rcu(). It introduces
a raw spinlock to synchronize operations on this list.

The commit message notes:
> Note that free_by_rcu_ttrace does not suffer from this issue as it only
> has a single active consumer during normal operation.

> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
> index e9662db7198fe..58296e92a4fed 100644
> --- a/kernel/bpf/memalloc.c
> +++ b/kernel/bpf/memalloc.c
[ ... ]
> @@ -297,6 +307,7 @@ static void enque_to_free(struct bpf_mem_cache *c, void *obj)
>  static void do_call_rcu_ttrace(struct bpf_mem_cache *c)
>  {
>  	struct llist_node *llnode, *t;
> +	unsigned long flags;
>  
>  	if (atomic_xchg(&c->call_rcu_ttrace_in_progress, 1)) {
>  		if (unlikely(READ_ONCE(c->draining))) {
>  			llnode = llist_del_all(&c->free_by_rcu_ttrace);
>  			free_all(c, llnode, !!c->percpu_size);
>  		}

[Severity: High]
This is a pre-existing issue, but does free_by_rcu_ttrace actually suffer
from a similar race condition during cache destruction?

If userspace triggers destruction of a BPF memory allocator via
bpf_mem_alloc_destroy(), CPU 0 sets c->draining = true and synchronizes
the irq_work:

CPU 0: bpf_mem_alloc_destroy()
    WRITE_ONCE(c->draining, true);
    irq_work_sync(&c->refill_work);

Meanwhile, CPU 1 could be executing alloc_bulk() inside the refill work,
consuming from free_by_rcu_ttrace:

CPU 1: alloc_bulk()
    obj = llist_del_first(&c->free_by_rcu_ttrace);

If CPU 1 reads the head pointer in llist_del_first() but is delayed before
READ_ONCE(entry->next), and concurrently an RCU callback on CPU 2 calls
do_call_rcu_ttrace(), CPU 2 will observe draining == true here and
immediately free the nodes.

When CPU 1 resumes and executes READ_ONCE(entry->next), won't this result
in a use-after-free on the node that was just freed to the slab allocator?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827084013.1062816-1-pulehui@huaweicloud.com?part=1