Re: [PATCH bpf] 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:
- [Critical] Incorrect struct member used in container_of() in __free_final_rcu() leads to a misaligned pointer and severe memory corruption.
--

commit bc9dc5f25db638cea0ee9ae53e66c94250192d0e
Author: Pu Lehui <pulehui@huawei.com>

bpf: Fix UAF due to concurrent consumption of waiting_for_gp_ttrace

This commit addresses a use-after-free in alloc_bulk() by deferring the node
reclamation of waiting_for_gp_ttrace. It detaches the nodes in the __free_rcu()
callback and queues them for final reclamation via a new normal RCU callback.

> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
> index e9662db7198fe..fb1e733bfb829 100644
> --- a/kernel/bpf/memalloc.c
> +++ b/kernel/bpf/memalloc.c
[ ... ]
> @@ -276,12 +278,28 @@ static int free_all(struct bpf_mem_cache *c, struct llist_node *llnode, bool per
>  	return cnt;
>  }
>  
> +static void __free_final_rcu(struct rcu_head *head)
> +{
> +	struct bpf_mem_cache *c = container_of(head, struct bpf_mem_cache, rcu);

[Severity: Critical]
Does this calculate the correct base pointer? 

Since the callback is queued using the rcu_reclaim field in __free_rcu(),
computing the container using the rcu field will result in a misaligned pointer.

Because rcu_reclaim is located after rcu in struct bpf_mem_cache, the offset
subtraction leaves c pointing past the actual object's start address. When
this corrupted pointer is passed to free_all(), could it cause memory
corruption or a crash by reading an invalid function pointer for c->dtor?

> +	struct llist_node *llnode = c->waiting_for_reclaim_gp;
> +
> +	c->waiting_for_reclaim_gp = NULL;
> +	free_all(c, llnode, !!c->percpu_size);
> +	atomic_set(&c->call_rcu_ttrace_in_progress, 0);
> +}
> +
>  static void __free_rcu(struct rcu_head *head)
>  {
>  	struct bpf_mem_cache *c = container_of(head, struct bpf_mem_cache, rcu_ttrace);
> +	struct llist_node *llnode = llist_del_all(&c->waiting_for_gp_ttrace);
>  
> -	free_all(c, llist_del_all(&c->waiting_for_gp_ttrace), !!c->percpu_size);
> -	atomic_set(&c->call_rcu_ttrace_in_progress, 0);
> +	if (!llnode) {
> +		atomic_set(&c->call_rcu_ttrace_in_progress, 0);
> +		return;
> +	}
> +
> +	c->waiting_for_reclaim_gp = llnode;
> +	call_rcu(&c->rcu_reclaim, __free_final_rcu);

[Severity: Critical]
Should __free_final_rcu() use container_of(head, struct bpf_mem_cache,
rcu_reclaim) to match this call_rcu() usage?

>  }

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