Re: [PATCH bpf v3] bpf: Fix UAF due to concurrent consumption of ttrace lists in alloc_bulk

[Pu Lehui <pulehui@huaweicloud.com>]

On 2026/9/1 23:55, bot+bpf-ci@kernel.org wrote:
>> This is expected because, as noted in commit 57b23c0f612d ("bpf: Retire
>> rcu_trace_implies_rcu_gp()"), an RCU Tasks Trace GP currently implies
>> (and will continue to imply in the future) a normal RCU GP. Since
>> alloc_bulk() runs in an RCU read-side CS (!PREEMPT_RT runs in IRQ
>> context, PREEMPT_RT runs with guard(rcu)), an RCU Tasks Trace GP cannot
>> complete while alloc_bulk() is accessing the list.
> 
> Is this claim about the RCU Tasks Trace GP accurate? Looking at the SRCU
> implementation, synchronize_rcu() sits in the middle of the SRCU scan
> (kernel/rcu/srcutree.c srcu_readers_active_idx_check()), not at the end.
> 
> An RCU GP is only required to wait for read-side critical sections that
> were already in progress when the GP started. Once a CPU reports its
> quiescent state, a new hardirq handler on that CPU does not delay the GP.
> So an alloc_bulk() that starts after the synchronize_rcu() call cannot
> hold off the RCU Tasks Trace GP at all.
> 
> Even if the GP could not complete during alloc_bulk(), that would not
> prevent the callback from running during alloc_bulk(). Callbacks are
> invoked asynchronously after the GP ends (srcu_invoke_callbacks workqueue
> in current kernels, or rcu_tasks_invoke_cbs in the 6.6 kernel that
> produced the splat).
> 
> The race diagram three paragraphs later shows __free_rcu executing
> concurrently with alloc_bulk's llist_del_first, which would not be
> possible if the statement above were true.
> 
> Would it be more accurate to say that RCU read-side protection is not the
> missing ingredient here, but rather mutual exclusion between the two
> llist consumers?

Regarding the commit message: there is no real contradiction. Those 
paragraphs were merely explaining our initial troubleshooting hypothesis 
and addressing earlier discussions where adding rcu_read_lock_trace() 
was suggested [0], rather than claiming that RCU Tasks Trace prevents 
the race.

However, to keep the commit log focused and avoid any confusion, I will 
simply drop that part in the next

[0] 
https://lore.kernel.org/bpf/3f72c4e7-340f-4374-9ebe-f9bffd08c755@paulmck-laptop/

> 
>> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
>> index e9662db7198f..5927943c765a 100644
>> --- a/kernel/bpf/memalloc.c
>> +++ b/kernel/bpf/memalloc.c
> 
> [ ... ]
> 
>> @@ -214,25 +215,25 @@ static void alloc_bulk(struct bpf_mem_cache *c, int cnt, int node, bool atomic)
>>   	gfp = __GFP_NOWARN | __GFP_ACCOUNT;
>>   	gfp |= atomic ? GFP_NOWAIT : GFP_KERNEL;
>>
>> -	for (i = 0; i < cnt; i++) {
>> -		/*
>> -		 * For every 'c' llist_del_first(&c->free_by_rcu_ttrace); is
>> -		 * done only by one CPU == current CPU. Other CPUs might
>> -		 * llist_add() and llist_del_all() in parallel.
>> -		 */
>> -		obj = llist_del_first(&c->free_by_rcu_ttrace);
>> -		if (!obj)
>> -			break;
>> -		add_obj_to_free_list(c, obj);
>> -	}
>> -	if (i >= cnt)
>> -		return;
>> +	scoped_guard(raw_spinlock_irqsave, &c->lock) {
>> +		for (i = 0; i < cnt; i++) {
>> +			/*
>> +			 * For every 'c' llist_del_first(&c->free_by_rcu_ttrace); is
>> +			 * done only by one CPU == current CPU. Other CPUs might
>> +			 * llist_add() and llist_del_all() in parallel.
>> +			 */
>> +			obj = llist_del_first(&c->free_by_rcu_ttrace);
>> +			if (!obj)
>> +				break;
>> +			add_obj_to_free_list(c, obj);
>> +		}
>>
>> -	for (; i < cnt; i++) {
>> -		obj = llist_del_first(&c->waiting_for_gp_ttrace);
>> -		if (!obj)
>> -			break;
>> -		add_obj_to_free_list(c, obj);
>> +		for (; i < cnt; i++) {
>> +			obj = llist_del_first(&c->waiting_for_gp_ttrace);
>> +			if (!obj)
>> +				break;
>> +			add_obj_to_free_list(c, obj);
>> +		}
>>   	}
>>   	if (i >= cnt)
>>   		return;
> 
> The comment moved inside the new critical section still states the
> lockless rationale:
> 
>      /*
>       * For every 'c' llist_del_first(&c->free_by_rcu_ttrace); is
>       * done only by one CPU == current CPU. Other CPUs might
>       * llist_add() and llist_del_all() in parallel.
>       */
> 
> The last sentence asserts that concurrent llist_del_all() is fine.
> According to include/linux/llist.h, the del_first x del_all combination
> requires a lock, and the commit message's first race diagram shows exactly
> this combination producing the KASAN splat.
> 
> Should this comment be updated to reflect that c->lock now serializes
> against __free_rcu() and the draining branch of do_call_rcu_ttrace(),
> rather than asserting that lockless operation is safe?

make sense to me.

> 
> The related comments in enque_to_free() ("Nothing races to add to

The comments in enque_to_free looks fine, no need to modify.

> free_by_rcu_ttrace list.") and drain_mem_cache() ("Except for
> waiting_for_gp_ttrace list, there are no concurrent operations on these
> lists") may also need updates, though they are outside this diff.

The comments in drain_mem_cache are slightly incorrect, but the impact 
is not significant. In addition, they are outside this diff.

> 
> 
> ---
> 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/33525009657