On Sat, 22 Aug 2026 06:00:14 +0000
[email protected] 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 <[email protected]>
>
> 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/[email protected]?part=1
>
--
Masami Hiramatsu (Google) <[email protected]>
