Thanks for the feedback. > 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. The child may still be unsafe to run after uprobe_copy_process() fails. Would it make sense to keep the cleanup and additionally send SIGKILL to the new child when the uprobe state copy fails? Also, should the existing "dup xol area" failure path be handled the same way? At this point dup_utask() has already succeeded, so the child has inherited the uprobe task state. However, we fail to get the xol_area needed for the later setup, so the child's uprobe state may still be incomplete. Keke. On Sun, Aug 23, 2026 at 11:38 PM Oleg Nesterov <[email protected]> wrote: > > (Add Andrii and Jiri) > > On 08/22, Keke Ming wrote: > > > > dup_utask() installs the new uprobe_task in t->utask before copying > > return_instances. If dup_return_instance() fails, the partially copied > > utask is left attached to the child task. > > Yes, > > > Free the partially copied utask before returning -ENOMEM. > > But why? > > Note that uprobe_copy_process() warns but returns "void", too late > to abort copy_process(). > > Perhaps we should change uprobe_copy_process() to kill the new child > on failure, it will likely crash anyway. > > But I don't think this patch can make the things any better. > > Oleg. > > > Signed-off-by: Keke Ming <[email protected]> > > --- > > kernel/events/uprobes.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > > index b25531331902..0c8a664a0fe5 100644 > > --- a/kernel/events/uprobes.c > > +++ b/kernel/events/uprobes.c > > @@ -2140,8 +2140,10 @@ static int dup_utask(struct task_struct *t, struct > > uprobe_task *o_utask) > > p = &n_utask->return_instances; > > for (o = o_utask->return_instances; o; o = o->next) { > > n = dup_return_instance(o); > > - if (!n) > > + if (!n) { > > + uprobe_free_utask(t); > > return -ENOMEM; > > + } > > > > /* if uprobe is non-NULL, we'll have an extra refcount for > > uprobe */ > > uprobe = hprobe_expire(&o->hprobe, true); > > -- > > 2.43.0 > > >
