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

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Sat, 22 Aug 2026 06:00:14 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Double free of `extra_consumers` during `fork()` due to missing pointer nullification in `dup_return_instance()` / `ri_pool_push()`.
> --
> 
> --- Patch [1]: [PATCH] uprobes: Free utask on dup_return_instance() failure ---
> commit 52bd4caeb5b9686b25fe707609034e9397c6100a
> Author: Keke Ming <ming.jvle@gmail.com>
> 
> uprobes: Free utask on dup_return_instance() failure
> 
> 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. This patch frees the partially
> copied utask before returning -ENOMEM.
> 
> > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> > index 7709ea8824778..7547929ab294d 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);
> 
> [Severity: Critical]
> This is a pre-existing issue, but can dup_return_instance() leave a dangling
> pointer that results in a double free of extra_consumers when
> uprobe_free_utask() is called here?

Hmm, this seems another bug here.

> 
> If a multi-consumer uretprobe is hit, extra_consumers is allocated. When this
> returns, it is pushed to the reuse pool:
> 
> kernel/events/uprobes.c:ri_pool_push() {
>     ri->cons_cnt = 0;
>     ri->next = utask->ri_pool;
>     utask->ri_pool = ri;
> }
> 
> This resets cons_cnt to 0 but retains the extra_consumers heap pointer.
> 
> If the thread then hits a single-consumer uretprobe, alloc_return_instance()
> reuses the pooled object. push_consumer() increments cons_cnt to 1 and uses
> the inline consumer, leaving the old extra_consumers pointer untouched.
> 
> When the task calls fork(), dup_return_instance() does a shallow copy:
> 
> kernel/events/uprobes.c:dup_return_instance() {
>     ri = kmemdup(old, sizeof(*ri), GFP_KERNEL);
>     if (!ri)
>         return NULL;
> 
>     if (unlikely(old->cons_cnt > 1)) {
>         ri->extra_consumers = kmemdup(old->extra_consumers, ...
>     ...
> }
> 
> Since old->cons_cnt == 1, it skips deep-copying or clearing extra_consumers.
> Both the parent and child tasks now share the exact same heap pointer. 

Hmm, so either clearing extra_consumers when returning ri to the pool
(with releasing extra_consumers) or clearing new ri->extra_consumers when
dup_return_instance(), will fix this.

Thanks,

> 
> When the tasks exit, or if dup_return_instance() fails and triggers the new
> cleanup path in this patch, ri_free() will execute kfree(ri->extra_consumers)
> in both contexts, freeing the same memory twice.
> 
> >  			return -ENOMEM;
> > +		}
> >  
> >  		/* if uprobe is non-NULL, we'll have an extra refcount for uprobe */
> >  		uprobe = hprobe_expire(&o->hprobe, true);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260822054620.452262-1-ming.jvle@gmail.com?part=1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>