Re: [PATCH] uprobes: Free utask on dup_return_instance() failure

[Andrii Nakryiko]

On Mon, Aug 24, 2026 at 8:57 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> On 08/24, Keke Ming wrote:
> >
> > On 08/24, Oleg Nesterov wrote:
> >
> > > Hmm. It is not that I think "it is not enough", I think this is pointless
> > > whatever we do.
> >
> > Well, I didn't mean to insist on the cleanup, I also think it may
> > be redundant. If the child is bound to exit, manually calling
> > uprobe_free_utask(t) in dup_utask() is pointless, as the normal
> > exit path will handle it.
>
> Yes, but the same is true if the child is not bound to exit... IOW,
> I still can't understand your motivation for this patch...
>
> > > But this is only the first step to cleanup
> > > this logic. I'll try to write another email tomorrow.
> >
> > Of course, I'm all ears.
>
> You have already mentioned "Option B" below ;) that is what I meant.
>
> > Here is my current understanding. One thing I am not sure about
> > is the signal semantics. If the copied uprobe state is incomplete,
> > the child should not return to user mode with that state.
>
> Perhaps... But consider
>
>         func_which_can_be_ret_probed(void)
>         {
>                 if (!fork) {
>                         printf("CHILD\n");
>                         exit(0);
>                 }
>         }
>
> > Option A:
> ...
> > ...
> > kill_child:
> >   // I was thinking about SIGKILL here because it cannot be ignored
> >   // Of course, you can correct me
> >   send_sig(SIGKILL, t, 1);
>
> force_exit_sig(SIGILL) can't be ignored too. But I am fine either way.

Current SIGILL sending is already pretty bad behavior that bites us
periodically, instead of doubling down on killing the user space
process because something about installing uprobe goes wrong, let's
think about a bit less destructive way to do this. Mark uprobe for a
particular process as defunct or "detaching it", or something along
those lines. But not just kill innocent processes.

>
> So yes, perhaps this makes sense. Although IMO this all is not that
> important; GFP_KERNEL shouldn't fail "in practice" and at least
> uprobe_copy_process() warns in this case...
>
> > Option B:
> >
> > > Note that uprobe_copy_process() warns but returns "void", too late
> > > to abort copy_process().
> >
> > Looking at copy_process(), uprobe_copy_process() is currently called
> > after the "No more failure paths" point and after the child has already
> > been made visible through the task list and pid links.
> >
> > Would the longer term fix be to move the uprobe state copy earlier,
> > before the "No more failure paths" point, and then add the needed
> > bad_fork cleanup for  p->utask?
>
> Agreed, pronably better than Option A. But in therory dup_xol_work()
> can still fail after fork().
>
> Oleg.
>