Re: [PATCH] uprobes: Free utask on dup_return_instance() failure
flat view
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Date: 2026-08-23 12:02:11
Also in:
linux-perf-users, sashiko-reviews
On Sat, 22 Aug 2026 06:00:14 +0000 sashiko-bot@kernel.org wrote:
quoted hunk ↗ jump to hunk
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 [off-list ref] 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.quoted
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.quoted
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) [off-list ref]