[PATCH] bpf: Fix use-after-free in __bpf_trace_run()

[PATCH] bpf: Fix use-after-free in __bpf_trace_run()

[Qing Wang]

A use-after-free issue reported from syzbot exists in __bpf_trace_run(O).

BUG: KASAN: slab-use-after-free in __bpf_trace_run kernel/trace/bpf_trace.c:2075 [inline]
    -> struct bpf_prog *prog = link->link.prog;

The link(struct bpf_raw_tp_link) was freed when link->link.prog.

The root cause is that: both bpf_raw_tp_link and tp_probes are RCU-managed,
but there is no synchronization between their lifecycles. The link can be
freed via call_rcu() before tp_probes is freed via
call_srcu()/call_rcu_tasks_trace(), causing the link's lifetime to be
shorter than tp_probes. This allows __bpf_trace_run() to access the freed
link when a tracepoint fires.

Fix by calling tracepoint_synchronize_unregister() to ensure tp_probes
is freed before allowing the link to be freed.

The issue was introduced by commit d4dfc5700e86 ("bpf:
pass whole link instead of prog when triggering raw tracepoint"),
which changed tracepoint callbacks to receive bpf_raw_tp_link pointers
instead of bpf_prog pointers.

Prior to this commit, this issue did not occur because the bpf_prog was
directly used and protected by reference counting.

Fixes: d4dfc5700e86 ("bpf: pass whole link instead of prog when triggering raw tracepoint")
Reported-by: syzbot+b4c5ad098c821bf8d8bc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b4c5ad098c821bf8d8bc
Tested-by: syzbot+b4c5ad098c821bf8d8bc@syzkaller.appspotmail.com
Signed-off-by: Qing Wang <wangqing7171@gmail.com>
---
 kernel/bpf/syscall.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 0378e83b4099..fc93dd544162 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3783,6 +3783,12 @@ static void bpf_raw_tp_link_release(struct bpf_link *link)
 
 	bpf_probe_unregister(raw_tp->btp, raw_tp);
 	bpf_put_raw_tracepoint(raw_tp->btp);
+
+	/*
+	 * Ensure tp_probes is freed before the link. This prevents
+	 * use-after-free in __bpf_trace_run() when a tracepoint fires.
+	 */
+	tracepoint_synchronize_unregister();
 }
 
 static void bpf_raw_tp_link_dealloc(struct bpf_link *link)
-- 
2.34.1

Re: [PATCH] bpf: Fix use-after-free in __bpf_trace_run()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2316 bytes --]

> Fix by calling tracepoint_synchronize_unregister() to ensure tp_probes
> is freed before allowing the link to be freed.

tracepoint_synchronize_unregister() calls synchronize_rcu_tasks_trace()
and synchronize_srcu(&tracepoint_srcu), which wait for in-flight
readers to complete. They do not ensure that call_srcu() or
call_rcu_tasks_trace() callbacks (the ones that actually kfree
tp_probes in rcu_free_old_probes()) have been invoked -- that
would require srcu_barrier()/rcu_barrier().

The fix is correct because after tracepoint_synchronize_unregister()
returns, no tracepoint callback can still be executing with a reference
to the link through the old tp_probes array. Would it be more precise
to say the synchronization ensures all in-flight tracepoint callbacks
have completed, rather than that tp_probes is freed?

> The link(struct bpf_raw_tp_link) was freed when link->link.prog.

This sentence is difficult to parse. Perhaps something like "The
link (struct bpf_raw_tp_link) was freed before accessing
link->link.prog"?

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 274039e36465..b179e96ebf2b 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3783,6 +3783,12 @@ static void bpf_raw_tp_link_release(struct bpf_link *link)
>
>  	bpf_probe_unregister(raw_tp->btp, raw_tp);
>  	bpf_put_raw_tracepoint(raw_tp->btp);
> +
> +	/*
> +	 * Ensure tp_probes is freed before the link. This prevents
> +	 * use-after-free in __bpf_trace_run() when a tracepoint fires.
> +	 */

Same observation as above about the commit message. The comment says
"Ensure tp_probes is freed before the link" but
tracepoint_synchronize_unregister() ensures all in-flight tracepoint
callbacks have completed, not that the tp_probes freeing callback
has run. The UAF prevention comes from readers finishing, not from
a freeing order.

Something like "Wait for all in-flight tracepoint callbacks to
complete so the link is no longer reachable through tp_probes" would
more accurately describe the guarantee.

> +	tracepoint_synchronize_unregister();
>  }


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