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

[Oleg Nesterov <oleg@redhat.com>]

(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 <ming.jvle@gmail.com>
> ---
>  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
>