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

[Florent Revest]

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.

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.

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