Re: [PATCH bpf] bpf: Keep progs alive until the trampoline image calling them is freed

[Kumar Kartikeya Dwivedi]

On Sun Aug 30, 2026 at 3:21 PM CEST, Alexei Starovoitov wrote:
> On Sun, Aug 30, 2026 at 3:41 AM Kumar Kartikeya Dwivedi
> <memxor@xxxxxxxxx> wrote:
>>
>> On Wed Aug 19, 2026 at 2:22 PM CEST, Florent Revest (Anthropic) wrote:
>> > bpf_tramp_image_put() makes sure a trampoline image is not freed while
>> > a task may still be running in it (call_rcu_tasks() + im->pcref), but
>> > nothing similar is done for the progs called by that image. Since
>> > commit e21aa341785c ("bpf: Fix fexit trampoline."), detach patches the
>> > return path so that a task still in the original function skips the
>> > fexit progs when it comes back, and counts on the prog's own RCU flavor
>> > to cover a task that is inside a prog. On that basis the last prog
>> > reference is dropped right away and the prog is freed after a single
>> > RCU / RCU tasks trace grace period.
>> >
>> > That leaves out a task in the trampoline glue itself: between two
>> > progs, or already past the patched jump but not yet in the first fexit
>> > prog's enter helper. On !PREEMPT kernels this is a few instructions
>> > that cannot be preempted, so it did not matter. With CONFIG_PREEMPTION
>>
>> I guess it would make sense to highlight why it may not have mattered. I think
>> the real reason was that on !PREEMPT kernels, the execution in the trampoline
>> image counted as (implicit) RCU read section which caused program free path to
>> wait for someone executing the trampoline image? For non-sleepable progs, they
>> wait for RCU grace period already, for sleepable, RCU tasks trace has implicit
>> RCU grace period wait as well, hence this never showed up on !PREEMPT.
>>
>> > a task can sit there, in no RCU read section of any flavor and holding
>> > only im->pcref, for longer than it takes to free the prog it is about
>> > to call:
>> >
>> >   CPU 0                               CPU 1
>> >   in image I, orig_call() returned
>> >   [preempted before lsm.s prog A]
>> >                                       bpf_tracing_link_release()
>> >                                        -> bpf_tramp_image_put(I)
>> >                                       bpf_link_dealloc()
>> >                                        bpf_prog_put(A), last ref
>> >                                       tasks trace GP, A's text freed
>> >   __bpf_prog_enter_sleepable(A)
>> >   call A->bpf_func
>> >
>> > On x86 this is an int3 in poisoned bpf_prog_pack memory:
>> >
>> >   Oops: int3: 0000 [#1] SMP NOPTI
>> >   CPU: 18 UID: 0 PID: 94573 Comm: x169 Not tainted 6.18.44 #1 PREEMPT(lazy)
>> >   RIP: 0010:0xffffffffc0601d8d
>> >   Call Trace:
>> >    <TASK>
>> >    ? bpf_trampoline_6442515411+0x1a4/0x21b
>> >    bpf_lsm_bprm_committed_creds+0x5/0x10
>> >    security_bprm_committed_creds+0x5f/0x70
>> >    begin_new_exec+0x2d6/0x410
>> >    ...
>> >
>> > We hit this in production on preemptible kernels when progs attached
>> > through trampolines got detached while their hooks were busy. Adding
>> > grace periods before the prog free would not help with sleepable progs:
>> > neither RCU tasks nor RCU tasks trace waits for a task that slept in a
>> > prog and then got preempted in the gap after it.
>> >
>> > Fix it by having the image take a reference on every prog it calls, in
>> > bpf_tramp_image_alloc(), and drop them in bpf_tramp_image_free(). A
>> > detached prog now stays loaded until the old image is gone, which
>> > reverts a deliberate choice of commit e21aa341785c ("bpf: Fix fexit
>> > trampoline."). Detached fexit progs still stop being called right away
>> > since the return path is patched.
>> >
>> > Fixes: e21aa341785c ("bpf: Fix fexit trampoline.")
>> > Assisted-by: Claude:unspecified
>> > Signed-off-by: Florent Revest (Anthropic) <florent.revest@xxxxxxxxx>
>> > ---
>>
>> Overall, looks good to me. Thanks for the fix!
>>
>> Acked-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
>>
>> Note for whoever applies this: please add Reported-by: tag for Sechang as well.
>> Optionally, wordsmith the commit log with the suggestion above.
>
> Hold on. I don't think we can proceed with this fix.
> It defeats the point of fexit jmp patching and keeps progs
> pinned until a sleepable kernel function that were attached to
> will return. Which means that the tracing prog attached to "unlucky" kernel
> function that sleeps for an hour will stay pinned for an hour.
> Let's think of a different way of fixing the race.

I don't have background on the original commit being fixed, but is that really
realistic? Or worrisome even if it happens in practice, since worst case the
program refcounts remains raised for that duration?

We have similar worst case for programs too (e.g. using bpf_copy_from_user on
user controlled buffer in, say, LSM progs). At least here we won't be extending
any RCU flavored GP.

That said I will think about alternative fixes in the meantime, if we accept the
premise that we don't want to pin program references in the image and keep their
lifetimes decoupled.

>
> pw-bot: cr