Re: [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure

["Hui Zhu" <hui.zhu@linux.dev>]

> 
> On Wed, Aug 05, 2026 at 12:04:05PM +0800, Hui Zhu wrote:
> 
> > 
> > From: Hui Zhu <zhuhui@kylinos.cn>
> >  
> >  This series fixes several use-after-free issues in the BPF trampoline
> >  multi-attach/detach error paths, where ftrace direct-call updates can
> >  fail and leave ftrace pointing at freed memory.
> > 
> hi,
> I need to stare at it bit more, but tbh I'm not sure the benefit of
> preventing hypothetical crash is worth the extra complexity on the
> detach side
> 
> IIUC we can't reproduce this error without instrumenting the code, right?
> 
> jirka

Hi Jiri,

You're right. I went through the failure paths and the realistic
triggers basically don't exist for a normal user:

The allocations are all GFP_KERNEL (reclaim + OOM handle them),
and bpf_jit_charge_modmem() lets CAP_BPF callers exceed the JIT
limit, so ENOMEM doesn't get there.
-E2BIG is attach-time, before cur_image is set, so no UAF.
SHARE_IPMODIFY -EAGAIN needs livepatch on the same function and
is retried in bpf_trampoline_update(); the multi path where it
could escape needs a second failure on the undo del, which doesn't
do ipmodify negotiation, so it doesn't reach the UAF either.
The rest is bugs or not user-driven.

So this is fault-injection territory, and I won't claim it's
a customer bug.

I'd like to drop patches 2 and 3 and the prog-side machinery
(pinned_prog + rollback + the trampoline leak).
And keep only the one-line image-side fix in patch 1: only free
old_image when it differs from cur_image.
It's obviously correct: if cur_image == old_image, ftrace is still
calling into it, so freeing it is wrong. And it costs almost nothing.

Would you prefer I proceed with just this single patch,
or drop the entire series instead?

Best,
Hui

> 
> > 
> > Patch 1 addresses two UAF scenarios in bpf_trampoline_multi_detach():
> >  the single-point unlink failure path (old_image == cur_image) and the
> >  batch ftrace update failure path. A new pinned_prog field in struct
> >  bpf_tramp_image keeps the bpf_prog alive while ftrace may still
> >  reference its image. bpf_trampoline_multi_detach() is made to return
> >  void, since callers cannot usefully react to failures, and
> >  bpf_trampoline_put() is taught to leak the trampoline when cur_image
> >  was left behind by a rollback, so ftrace keeps a valid target.
> >  
> >  Patch 2 fixes a similar UAF in bpf_trampoline_multi_attach() rollback:
> >  when the register-path undo fails, ftrace still calls into cur_image,
> >  so the prog is pinned on cur_image instead of being rolled back.
> >  
> >  Patch 3 fixes the common __bpf_trampoline_unlink_prog() path, covering
> >  both multi (bpf_trampoline_multi_detach) and non-multi
> >  (bpf_tracing_link_release, bpf_shim_tramp_link_release) callers.
> >  
> >  Hui Zhu (3):
> >  bpf: Fix UAF in bpf_trampoline_multi_detach on update failure
> >  bpf: Fix prog UAF in bpf_trampoline_multi_attach() register-path
> >  rollback
> >  bpf: Fix prog UAF in __bpf_trampoline_unlink_prog() on update failure
> >  
> >  include/linux/bpf.h | 20 +++--
> >  kernel/bpf/trampoline.c | 183 +++++++++++++++++++++++++++++++++++----
> >  kernel/trace/bpf_trace.c | 2 +-
> >  3 files changed, 183 insertions(+), 22 deletions(-)
> >  
> >  Changelog:
> >  v2:
> >  Folded v1's two detach patches into patch 1.
> >  According to the comments of Jiri Olsa, Pin the prog (pinned_prog) on
> >  cur_image so it stays alive while ftrace may still call into it.
> >  Make bpf_trampoline_multi_detach() return void.
> >  Fix the same UAF in standard (non-multi) trampolines.
> >  According to the comments of sashiko, Fix the prog UAF in
> >  bpf_trampoline_multi_attach() rollback.
> >  Leak the trampoline in bpf_trampoline_put() when cur_image is left
> >  by a rollback.
> >  
> >  -- 
> >  2.53.0
> >
>