Re: [PATCH bpf] bpf: Fix UAF due to concurrent consumption of waiting_for_gp_ttrace
From: Pu Lehui
Date: Thu Aug 27 2026 - 03:44:27 EST
Thanks very much!
The review comments on bpf ci look excellent, and I will extract the valuable information from them.
On 2026/8/26 19:16, bot+bpf-ci@xxxxxxxxxx wrote:
alloc_bulk() runs in an IRQ context (serving as an implicit normal RCU
read-side critical section), an RCU Tasks Trace GP cannot complete while
alloc_bulk() is accessing the list.
Is this accurate on CONFIG_PREEMPT_RT? Looking at init_refill_work() in
kernel/bpf/memalloc.c, it uses init_irq_work() without IRQ_WORK_INIT_HARD,
so on RT the irq_work goes to lazy_list and is executed by the preemptible
per-CPU 'irq_work/N' kthread, not in hardirq context. The allocator's own
comment says "In RT irq_work runs in per-cpu kthread."
The conclusion still holds because run_irq_workd() wraps
irq_work_run_list(&lazy_list) in guard(rcu)(), making it an explicit RCU
read-side critical section on RT. So alloc_bulk() is protected by RCU, but
the stated reason ('IRQ context') is only true on !RT.
Could the commit message clarify that the RCU-reader property comes from
hardirq on !RT and explicit rcu_read_lock() in run_irq_workd() on RT?
Thus, the callback __free_rcu cannot run concurrently, ruling out
missing RCU read-side locks as the cause.
Can __free_rcu really not run concurrently with alloc_bulk()? The race
diagram two paragraphs later shows exactly that happening:
CPU0 CPU1
__free_rcu (RCU Tasks Trace callback)
alloc_bulk (irq context)
llist_del_first(&c->waiting_for_gp_ttrace)
entry = smp_load_acquire(&head->first);
free_all(llist_del_all(&c->waiting_for_gp_ttrace))
next = READ_ONCE(entry->next); <-- trigger UAF
Looking at the guarantees from 57b23c0f612d, the RCU Tasks Trace GP
contains at least one synchronize_rcu(), which waits for alloc_bulk()
invocations that started before that internal synchronize_rcu(). But a
fresh alloc_bulk() entered after the GP expired is not waited for, and
__free_rcu can run while that fresh invocation is in flight.
So what's actually true is narrower: a ttrace GP cannot expire while an
alloc_bulk() read section that began before the GP's internal
synchronize_rcu() is still in flight. A fresh alloc_bulk() after the GP
expired can race with __free_rcu().
Could the explanation be more precise about which alloc_bulk() invocations
are protected and which can race?
diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
index e9662db7198f..fb1e733bfb82 100644
--- a/kernel/bpf/memalloc.c
+++ b/kernel/bpf/memalloc.c
@@ -110,7 +110,9 @@ struct bpf_mem_cache {
struct llist_node *free_by_rcu_tail;
struct llist_head waiting_for_gp;
struct llist_node *waiting_for_gp_tail;
+ struct llist_node *waiting_for_reclaim_gp;
struct rcu_head rcu;
+ struct rcu_head rcu_reclaim;
atomic_t call_rcu_in_progress;
struct llist_head free_llist_extra_rcu;
[ ... ]
@@ -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);
+ 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);
Does free_mem_alloc() wait for this new callback? Looking at the teardown
path:
kernel/bpf/memalloc.c:free_mem_alloc() {
/*
* Trace grace period implies RCU grace period, so all __free_rcu
* don't need extra call_rcu() (and thus extra rcu_barrier() here).
*/
rcu_barrier(); /* wait for __free_by_rcu */
rcu_barrier_tasks_trace(); /* wait for __free_rcu */
free_mem_alloc_no_barrier(ma);
-> free_percpu(ma->cache) / free_percpu(ma->caches)
}
The barriers run in that order: normal RCU first, then tasks-trace. If
__free_rcu() executes during rcu_barrier_tasks_trace() and queues
call_rcu(&c->rcu_reclaim, __free_final_rcu), there's no rcu_barrier()
left to wait for it. So free_percpu() runs while __free_final_rcu() is
still queued on the now-freed per-cpu memory.
Concrete sequence:
1. do_call_rcu_ttrace() queues call_rcu_tasks_trace(&c->rcu_ttrace,
__free_rcu)
2. Map destroyed, destroy path computes rcu_in_progress >= 1 from
atomic_read(&c->call_rcu_ttrace_in_progress), takes deferred path
3. free_mem_alloc(): rcu_barrier() at 716 completes, rcu_barrier_tasks_trace()
at 717 waits for __free_rcu() which runs and queues call_rcu(&c->rcu_reclaim,
__free_final_rcu), barrier returns with normal-RCU callback newly queued
4. free_mem_alloc_no_barrier() frees the per-cpu memory holding c
5. When normal grace period ends, __free_final_rcu() runs on freed memory
Also, drain_mem_cache() drains seven lists but not waiting_for_reclaim_gp,
and check_mem_cache() WARN_ON_ONCE()s on those seven but never inspects
waiting_for_reclaim_gp, so the leak is silent.
Does this need an rcu_barrier() after rcu_barrier_tasks_trace() (or the
two swapped plus an added one)? And should the comment at lines 706-715
be updated since it now says the opposite of what the code does?
Also note check_mem_cache() should probably check waiting_for_reclaim_gp.
The same llist_del_first-vs-llist_del_all hazard that this patch fixes
for waiting_for_gp_ttrace also exists on free_by_rcu_ttrace. Looking at
kernel/bpf/memalloc.c:
alloc_bulk() does:
obj = llist_del_first(&c->free_by_rcu_ttrace);
while do_call_rcu_ttrace() does:
llnode = llist_del_all(&c->free_by_rcu_ttrace);
...
free_all(c, llnode, percpu); /* draining path, immediate free */
and drain_mem_cache() does:
free_all(llist_del_all(&c->free_by_rcu_ttrace), ...);
include/linux/llist.h's concurrency table marks the del_first x del_all
cell 'L' (lock is needed) and the kerneldoc on llist_del_first says "If
multiple consumers are needed, please use llist_del_all or use lock
between consumers."
The non-draining path in do_call_rcu_ttrace() is incidentally covered by
the new extra GP because nodes end up in waiting_for_gp_ttrace, but the
draining direct-free and drain_mem_cache()'s free_all() still free without
any RCU deferral.
Does free_by_rcu_ttrace need the same treatment?
While the nodes sit in c->waiting_for_reclaim_gp they are invisible to
alloc_bulk()'s reuse loop, so for one extra normal RCU GP the allocator
must kmalloc() instead of reusing. This partially undoes the optimization
being fixed.
Is this expected? Should the commit message mention it?
The commit message says "Syzkaller repeatedly triggered" but carries no
Reported-by:, Closes: or Link: tag, and the splat is from a private 6.6.0+
tree rather than upstream syzbot.
Also, Hou Tao reported this exact hazard during review of 04fabf00b4d3 in
July 2023 (lore.kernel.org/rcu/957dd5cd-0855-1197-7045-4cb1590bd753@xxxxxxxxxxxxxxx).
Alexei's reply was that the probability was "lower than a bit flip due to
cosmic rays."
Could citing that thread make the analysis more credible and give proper
credit to Hou Tao?
---
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/32959494022