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

[Alexei Starovoitov]

On Wed, Sep 2, 2026 at 4:25 PM Florent Revest <florent.revest@xxxxxxxxx> wrote:
>
> On Wed Sep 2, 2026 at 10:06 PM UTC, Alexei Starovoitov wrote:
> > 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:
> > > > >
> > > > > 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 ?
>
> Yep, exactly!
>
> > > 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.
>
> Indeed. When I originally sent this patch my repro was a stress test but when I
> found out about the above scenario I could make a short and deterministic repro
>
> My only question then is, what kind of fix would you prefer in v2: keeping refs
> to progs while the trampoline is alive ? (like in this v1, but I think you
> nacked this approach already ?)

I'm worried about holding fexit progs potentially forever.
There are kernel funcs that enter and never return.
Attaching to them will pin fexit progs forever.
While right now it's only the trampoline that gets pinned
which is 4k.

> or some other creative solution like having
> __bpf_prog_enter*() skip each freed prog individually ?

I don't want to add more overhead to __bpf_prog_enter*() wrappers either.

> Or maybe something
> else ?

Let's extend ip_after_call concept.
Instead of using a single NOP after the call to skip fexit progs
let's add NOPs in front of every invoke_bpf() including fentry and fmod_ret.

Then on tramp destroy patch all nop-s to jmp epilogue.

llm-s helped me realize that fentry has the same issue.
fentry prog A can sleep
while non-sleepable fentry prog B can detach and things will crash.

better ideas?