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

["Florent Revest" <florent.revest@linux.dev>]

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@linux.dev> 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@linux.dev> 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 ?) or some other creative solution like having
__bpf_prog_enter*() skip each freed prog individually ? Or maybe something
else ?