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

[Keke Ming]

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, this is what I meant. 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.

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.

Option A:

```
if (dup_utask(t, utask)) {
  uprobe_warn(t, "dup ret instances");
  goto kill_child;
}
...
if (!area) {
  // whether we also need to kill it is open to further discussion.
  uprobe_warn(t, "dup xol area");
  goto kill_child;
}
...
kill_child:
  // I was thinking about SIGKILL here because it cannot be ignored
  // Of course, you can correct me
  send_sig(SIGKILL, t, 1);
```

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?

Keke.

On Mon, Aug 24, 2026 at 3:41 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> On 08/24, Keke Ming wrote:
> >
> > > Note that uprobe_copy_process() warns but returns "void", too late
> > > to abort copy_process().
> >
> > You are right that freeing the partial utask is not enough.
>
> Hmm. It is not that I think "it is not enough", I think this is pointless
> whatever we do. Please see below.
>
> But you know what? I am afraid I am totally confused again, this happens
> more and more often. So please correct me.
>
> > Would it make sense to keep the cleanup
>
> But again, why do you think it makes any sense to keep the cleanup
> you propose?
>
> > and additionally send
> > SIGKILL to the new child when the  uprobe state copy fails?
>
> Or SIGILL like the rest of uprobes.c does...
>
> Yes, this is what I meant. But this is only the first step to cleanup
> this logic. I'll try to write another email tomorrow.
>
> Lets suppose we change uprobe_copy_process() to kill the child. Then
> why do we need to call uprobe_free_utask() in dup_utask() or do anything
> else in copy_process() paths?
>
> The child won't return to userspace, it will exit and call uprobe_free_utask()
> itself.
>
> > Also, should the existing "dup xol area" failure path be handled
> > the same way?
>
> Yes sure. But perhaps needs another discussion.
>
> Oleg.
>