Re: [PATCH bpf v3] bpf: Fix UAF due to concurrent consumption of ttrace lists in alloc_bulk
From: Pu Lehui
Date: Fri Sep 04 2026 - 22:03:55 EST
On 2026/9/1 23:55, bot+bpf-ci@xxxxxxxxxx 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