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

[Alexei Starovoitov]

On Wed, Sep 2, 2026 at 3:15 AM Florent Revest <florent.revest@xxxxxxxxx> wrote:
>
> On Wed Sep 2, 2026 at 5:57 AM UTC, Alexei Starovoitov wrote:
> > On Mon, Aug 31, 2026 at 1:58 PM Florent Revest <florent.revest@xxxxxxxxx> wrote:
> > >
> > > On Mon Aug 31, 2026 at 2:43 AM UTC, Kumar Kartikeya Dwivedi wrote:
> > > > 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:
> > > > >>
> > > > >> 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.
> > >
> > > How about having old trampolines skip freed programs instead of keeping
> > > them alive ?
> >
> > Isn't it doing it now?
> > That was the whole point of patching nop to jmp in a trampoline.
>
> Just to be sure we're talking about the same thing, you're referring to the
> "ip_after_call" patching done in bpf_tramp_image_put, right ?
>
> My understanding is that this only helps in the case where a task is in the
> traced function while a prog is detached. It'd make it skip all fexit progs.

Right. That's the one I meant.

> But if a task is sleeping in a fexit.s prog for example, it is already past
> ip_after_call, so this patching would not prevent it from calling a second,
> freed, fexit prog lined up after it in the trampoline, no ? The same situation
> should happen with a sleeping fentry.s prog followed by a fentry prog.

Hmm. You mean like fexit prog A called a sleepable kfunc and
another prog B is attached to the same trampoline.

Then the trampoline needs to be freed. ip_after_call patches jmp in,
and trampoline proceeds to free progs A and B.
B is freed right away.
Eventually A's kfunc returns from sleep
and jmps back into a trampoline
which is not freed yet, since percpu_ref keeps it,
but prog B is gone, so it attempts to execute a freed prog B ?


> I have a reproducer for those scenarios that crashes bpf-next. I will make it a
> selftest in v2 like Jiri suggested.

Please narrow down the reproducer (sounds like it's a random stress test
at this point) to specific steps.

If my guess above is correct the reproducer will be short
and deterministic.