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

[bot+bpf-ci@kernel.org]

[-- 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