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

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

[Keke Ming]

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.

Free the partially copied utask before returning -ENOMEM.

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

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

[sashiko-bot]

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?

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. 

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

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

[Masami Hiramatsu]

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>

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

[Keke Ming]

Thanks, agreed.

> Hmm, this seems another bug here.

I chose method 2 (clearing ri->extra_consumers when dup_return_instance())
in this patch set. Please let me know if you prefer the first approach instead.

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

[Keke Ming]

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.

Free the partially copied utask before returning -ENOMEM.

Also, dup_return_instance() copies the return_instance before fixing
up extra_consumers.

Here, if no deep copy is needed, clear the copied extra_consumers
pointer.

Signed-off-by: Keke Ming <ming.jvle@gmail.com>
---
v2 changes:
- Clear ri->extra_consumers = NULL when old->cons_cnt <= 1 in
  dup_return_instance(), fixing a potential double free during
  cleanup.
- Keep the dup_utask() error cleanup from v1.

 kernel/events/uprobes.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index b25531331902..73a6620c5701 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -2118,6 +2118,8 @@ static struct return_instance *dup_return_instance(struct return_instance *old)
 			kfree(ri);
 			return NULL;
 		}
+	} else {
+		ri->extra_consumers = NULL;
 	}
 
 	return ri;
@@ -2140,8 +2142,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

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

[sashiko-bot]

> 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.
> 
> Free the partially copied utask before returning -ENOMEM.
> 
> Also, dup_return_instance() copies the return_instance before fixing
> up extra_consumers.
> 
> Here, if no deep copy is needed, clear the copied extra_consumers
> pointer.
> 
> Signed-off-by: Keke Ming <ming.jvle@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822150906.528208-1-ming.jvle@gmail.com?part=1

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

[Oleg Nesterov]

(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
>

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

[Keke Ming]

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 <oleg@redhat.com> 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 <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
> >
>

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

[Oleg Nesterov]

On 08/24, Keke Ming wrote:
>
> > 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.

Hmm. It is not that I think "it is not enough", I think this is pointless
whatever we do. Please see below.

But you know what? I am afraid I am totally confused again, this happens
more and more often. So please correct me.

> Would it make sense to keep the cleanup

But again, why do you think it makes any sense to keep the cleanup
you propose?

> and additionally send
> SIGKILL to the new child when the  uprobe state copy fails?

Or SIGILL like the rest of uprobes.c does...

Yes, this is what I meant. But this is only the first step to cleanup
this logic. I'll try to write another email tomorrow.

Lets suppose we change uprobe_copy_process() to kill the child. Then
why do we need to call uprobe_free_utask() in dup_utask() or do anything
else in copy_process() paths?

The child won't return to userspace, it will exit and call uprobe_free_utask()
itself.

> Also, should the existing "dup xol area" failure path be handled
> the same way?

Yes sure. But perhaps needs another discussion.

Oleg.

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

[Keke Ming]

On 08/24, Oleg Nesterov wrote:

> Hmm. It is not that I think "it is not enough", I think this is pointless
> whatever we do.

Well, I didn't mean to insist on the cleanup, I also think it may
be redundant. If the child is bound to exit, manually calling
uprobe_free_utask(t) in dup_utask() is pointless, as the normal
exit path will handle it.

> Yes, this is what I meant. But this is only the first step to cleanup
> this logic. I'll try to write another email tomorrow.

Of course, I'm all ears.

Here is my current understanding. One thing I am not sure about
is the signal semantics. If the copied uprobe state is incomplete,
the child should not return to user mode with that state.

Option A:

```
if (dup_utask(t, utask)) {
  uprobe_warn(t, "dup ret instances");
  goto kill_child;
}
...
if (!area) {
  // whether we also need to kill it is open to further discussion.
  uprobe_warn(t, "dup xol area");
  goto kill_child;
}
...
kill_child:
  // I was thinking about SIGKILL here because it cannot be ignored
  // Of course, you can correct me
  send_sig(SIGKILL, t, 1);
```

Option B:

> Note that uprobe_copy_process() warns but returns "void", too late
> to abort copy_process().

Looking at copy_process(), uprobe_copy_process() is currently called
after the "No more failure paths" point and after the child has already
been made visible through the task list and pid links.

Would the longer term fix be to move the uprobe state copy earlier,
before the "No more failure paths" point, and then add the needed
bad_fork cleanup for  p->utask?

Keke.

On Mon, Aug 24, 2026 at 3:41 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 08/24, Keke Ming wrote:
> >
> > > 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.
>
> Hmm. It is not that I think "it is not enough", I think this is pointless
> whatever we do. Please see below.
>
> But you know what? I am afraid I am totally confused again, this happens
> more and more often. So please correct me.
>
> > Would it make sense to keep the cleanup
>
> But again, why do you think it makes any sense to keep the cleanup
> you propose?
>
> > and additionally send
> > SIGKILL to the new child when the  uprobe state copy fails?
>
> Or SIGILL like the rest of uprobes.c does...
>
> Yes, this is what I meant. But this is only the first step to cleanup
> this logic. I'll try to write another email tomorrow.
>
> Lets suppose we change uprobe_copy_process() to kill the child. Then
> why do we need to call uprobe_free_utask() in dup_utask() or do anything
> else in copy_process() paths?
>
> The child won't return to userspace, it will exit and call uprobe_free_utask()
> itself.
>
> > Also, should the existing "dup xol area" failure path be handled
> > the same way?
>
> Yes sure. But perhaps needs another discussion.
>
> Oleg.
>

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

[Oleg Nesterov]

On 08/24, Keke Ming wrote:
>
> On 08/24, Oleg Nesterov wrote:
>
> > Hmm. It is not that I think "it is not enough", I think this is pointless
> > whatever we do.
>
> Well, I didn't mean to insist on the cleanup, I also think it may
> be redundant. If the child is bound to exit, manually calling
> uprobe_free_utask(t) in dup_utask() is pointless, as the normal
> exit path will handle it.

Yes, but the same is true if the child is not bound to exit... IOW,
I still can't understand your motivation for this patch...

> > But this is only the first step to cleanup
> > this logic. I'll try to write another email tomorrow.
>
> Of course, I'm all ears.

You have already mentioned "Option B" below ;) that is what I meant.

> Here is my current understanding. One thing I am not sure about
> is the signal semantics. If the copied uprobe state is incomplete,
> the child should not return to user mode with that state.

Perhaps... But consider

	func_which_can_be_ret_probed(void)
	{
		if (!fork) {
			printf("CHILD\n");
			exit(0);
		}
	}

> Option A:
...
> ...
> kill_child:
>   // I was thinking about SIGKILL here because it cannot be ignored
>   // Of course, you can correct me
>   send_sig(SIGKILL, t, 1);

force_exit_sig(SIGILL) can't be ignored too. But I am fine either way.

So yes, perhaps this makes sense. Although IMO this all is not that
important; GFP_KERNEL shouldn't fail "in practice" and at least
uprobe_copy_process() warns in this case...

> Option B:
>
> > Note that uprobe_copy_process() warns but returns "void", too late
> > to abort copy_process().
>
> Looking at copy_process(), uprobe_copy_process() is currently called
> after the "No more failure paths" point and after the child has already
> been made visible through the task list and pid links.
>
> Would the longer term fix be to move the uprobe state copy earlier,
> before the "No more failure paths" point, and then add the needed
> bad_fork cleanup for  p->utask?

Agreed, pronably better than Option A. But in therory dup_xol_work()
can still fail after fork().

Oleg.

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

[Andrii Nakryiko]

On Mon, Aug 24, 2026 at 8:57 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 08/24, Keke Ming wrote:
> >
> > On 08/24, Oleg Nesterov wrote:
> >
> > > Hmm. It is not that I think "it is not enough", I think this is pointless
> > > whatever we do.
> >
> > Well, I didn't mean to insist on the cleanup, I also think it may
> > be redundant. If the child is bound to exit, manually calling
> > uprobe_free_utask(t) in dup_utask() is pointless, as the normal
> > exit path will handle it.
>
> Yes, but the same is true if the child is not bound to exit... IOW,
> I still can't understand your motivation for this patch...
>
> > > But this is only the first step to cleanup
> > > this logic. I'll try to write another email tomorrow.
> >
> > Of course, I'm all ears.
>
> You have already mentioned "Option B" below ;) that is what I meant.
>
> > Here is my current understanding. One thing I am not sure about
> > is the signal semantics. If the copied uprobe state is incomplete,
> > the child should not return to user mode with that state.
>
> Perhaps... But consider
>
>         func_which_can_be_ret_probed(void)
>         {
>                 if (!fork) {
>                         printf("CHILD\n");
>                         exit(0);
>                 }
>         }
>
> > Option A:
> ...
> > ...
> > kill_child:
> >   // I was thinking about SIGKILL here because it cannot be ignored
> >   // Of course, you can correct me
> >   send_sig(SIGKILL, t, 1);
>
> force_exit_sig(SIGILL) can't be ignored too. But I am fine either way.

Current SIGILL sending is already pretty bad behavior that bites us
periodically, instead of doubling down on killing the user space
process because something about installing uprobe goes wrong, let's
think about a bit less destructive way to do this. Mark uprobe for a
particular process as defunct or "detaching it", or something along
those lines. But not just kill innocent processes.

>
> So yes, perhaps this makes sense. Although IMO this all is not that
> important; GFP_KERNEL shouldn't fail "in practice" and at least
> uprobe_copy_process() warns in this case...
>
> > Option B:
> >
> > > Note that uprobe_copy_process() warns but returns "void", too late
> > > to abort copy_process().
> >
> > Looking at copy_process(), uprobe_copy_process() is currently called
> > after the "No more failure paths" point and after the child has already
> > been made visible through the task list and pid links.
> >
> > Would the longer term fix be to move the uprobe state copy earlier,
> > before the "No more failure paths" point, and then add the needed
> > bad_fork cleanup for  p->utask?
>
> Agreed, pronably better than Option A. But in therory dup_xol_work()
> can still fail after fork().
>
> Oleg.
>

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

[Oleg Nesterov]

On 08/24, Andrii Nakryiko wrote:
>
> On Mon, Aug 24, 2026 at 8:57 AM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > force_exit_sig(SIGILL) can't be ignored too. But I am fine either way.
>
> Current SIGILL sending is already pretty bad behavior that bites us
> periodically, instead of doubling down on killing the user space
> process because something about installing uprobe goes wrong, let's
> think about a bit less destructive way to do this. Mark uprobe for a
> particular process as defunct or "detaching it", or something along
> those lines. But not just kill innocent processes.

Well. Yes, SIGILL sending acts as BUG_ON(), this is not nice and
probably we can improve the current code. And, just in case, as for
uprobe_copy_process() in particular I agree that Option B (make fork()
fail) is better.

But what, say, uprobe_handle_trampoline() can do if ->return_instances
is NULL? Either we have a kernel bug, or the probed task does something
wrong. In both cases it is better to kill the task with pr_warn() and
generate the core dump.

Oleg.

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

[Andrii Nakryiko]

On Tue, Aug 25, 2026 at 5:27 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 08/24, Andrii Nakryiko wrote:
> >
> > On Mon, Aug 24, 2026 at 8:57 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > >
> > > force_exit_sig(SIGILL) can't be ignored too. But I am fine either way.
> >
> > Current SIGILL sending is already pretty bad behavior that bites us
> > periodically, instead of doubling down on killing the user space
> > process because something about installing uprobe goes wrong, let's
> > think about a bit less destructive way to do this. Mark uprobe for a
> > particular process as defunct or "detaching it", or something along
> > those lines. But not just kill innocent processes.
>
> Well. Yes, SIGILL sending acts as BUG_ON(), this is not nice and

The problem is that this is actually not a bug but a very real and
expected behavior with user-space using fibers or coroutines. Any
suggestions how we can make this less violent, as it's actually quite
dangerous and kills production applications when someone (unknowingly)
tries to uretprobe trace them...

> probably we can improve the current code. And, just in case, as for
> uprobe_copy_process() in particular I agree that Option B (make fork()
> fail) is better.
>
> But what, say, uprobe_handle_trampoline() can do if ->return_instances
> is NULL? Either we have a kernel bug, or the probed task does something
> wrong. In both cases it is better to kill the task with pr_warn() and
> generate the core dump.

I don't remember all the code paths, but just shooting in the dark
there, we can always poison a pointer to distinguish or something? or
just assume kernel bugs do not happen and ignore NULL. Or something
along those lines.

>
> Oleg.
>

SIGILL and uretprobes, WAS: [PATCH] uprobes: Free utask on dup_return_instance() failure

[Andrii Nakryiko]

On Tue, Aug 25, 2026 at 9:38 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Aug 25, 2026 at 5:27 AM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > On 08/24, Andrii Nakryiko wrote:
> > >
> > > On Mon, Aug 24, 2026 at 8:57 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > > >
> > > > force_exit_sig(SIGILL) can't be ignored too. But I am fine either way.
> > >
> > > Current SIGILL sending is already pretty bad behavior that bites us
> > > periodically, instead of doubling down on killing the user space
> > > process because something about installing uprobe goes wrong, let's
> > > think about a bit less destructive way to do this. Mark uprobe for a
> > > particular process as defunct or "detaching it", or something along
> > > those lines. But not just kill innocent processes.
> >
> > Well. Yes, SIGILL sending acts as BUG_ON(), this is not nice and
>
> The problem is that this is actually not a bug but a very real and
> expected behavior with user-space using fibers or coroutines. Any
> suggestions how we can make this less violent, as it's actually quite
> dangerous and kills production applications when someone (unknowingly)
> tries to uretprobe trace them...
>

IIUC, this SIGILL and coroutines/fibers issue is due to
uprobe_handle_trampoline not finding utrask->return_instances, because
a bit earlier we threw out pending uretprobes due to
arch_uretprobe_is_alive() returned false due to that `regs->sp <=
ret->stack` check (for x86-64).

So the question I hope you can help reason about is a) should we just
ignore utask->return_instances == NULL case and count it as uretprobe
miss and b) should we flip it around, and just not add a new pending
uretprobe if current stack doesn't satisfy this invariant.

In practice this would mean that with fibers we'll get some uretprobes
matched, others dropped, but at least this will be discoverable due to
misses and it won't be killing innocent applications.

And at any rate, we have a per-process limit on number of pending
uretprobes, so there is no risk of having unbounded amount of memory
taken by erratically behaving application breaking uretprobes.

WDYT?

> > probably we can improve the current code. And, just in case, as for
> > uprobe_copy_process() in particular I agree that Option B (make fork()
> > fail) is better.
> >
> > But what, say, uprobe_handle_trampoline() can do if ->return_instances
> > is NULL? Either we have a kernel bug, or the probed task does something
> > wrong. In both cases it is better to kill the task with pr_warn() and
> > generate the core dump.
>
> I don't remember all the code paths, but just shooting in the dark
> there, we can always poison a pointer to distinguish or something? or
> just assume kernel bugs do not happen and ignore NULL. Or something
> along those lines.
>
> >
> > Oleg.
> >

Re: SIGILL and uretprobes, WAS: [PATCH] uprobes: Free utask on dup_return_instance() failure

[Oleg Nesterov]

On 08/25, Andrii Nakryiko wrote:
>
> On Tue, Aug 25, 2026 at 9:38 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Tue, Aug 25, 2026 at 5:27 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > >
> > > On 08/24, Andrii Nakryiko wrote:
> > > >
> > > > On Mon, Aug 24, 2026 at 8:57 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > > > >
> > > > > force_exit_sig(SIGILL) can't be ignored too. But I am fine either way.
> > > >
> > > > Current SIGILL sending is already pretty bad behavior that bites us
> > > > periodically, instead of doubling down on killing the user space
> > > > process because something about installing uprobe goes wrong, let's
> > > > think about a bit less destructive way to do this. Mark uprobe for a
> > > > particular process as defunct or "detaching it", or something along
> > > > those lines. But not just kill innocent processes.
> > >
> > > Well. Yes, SIGILL sending acts as BUG_ON(), this is not nice and
> >
> > The problem is that this is actually not a bug but a very real and
> > expected behavior with user-space using fibers or coroutines. Any
> > suggestions how we can make this less violent, as it's actually quite
> > dangerous and kills production applications when someone (unknowingly)
> > tries to uretprobe trace them...
> >
>
> IIUC, this SIGILL and coroutines/fibers issue is due to
> uprobe_handle_trampoline not finding utrask->return_instances, because
> a bit earlier we threw out pending uretprobes due to
> arch_uretprobe_is_alive() returned false due to that `regs->sp <=
> ret->stack` check (for x86-64).

Well, unless I am totally confused this is another thing...

Yes, arch_uretprobe_is_alive() sucks, it assumes the single stack.

And I do not know how to fix this problem... uprobe_handle_trampoline()
must fix regs->ip correctly somehow, otherwise it is pointless to contunue.

Note also that the current instruction_pointer_set(ri->orig_ret_vaddr)
can be wrong if the probed task switches its stack.

And we have the same problems with sigaltstack().

ATM, I don't see any solution, and I don't really understand what do
you propose...

Oleg.

Re: SIGILL and uretprobes, WAS: [PATCH] uprobes: Free utask on dup_return_instance() failure

[Andrii Nakryiko]

On Wed, Aug 26, 2026 at 4:49 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 08/25, Andrii Nakryiko wrote:
> >
> > On Tue, Aug 25, 2026 at 9:38 AM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Tue, Aug 25, 2026 at 5:27 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > > >
> > > > On 08/24, Andrii Nakryiko wrote:
> > > > >
> > > > > On Mon, Aug 24, 2026 at 8:57 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > > > > >
> > > > > > force_exit_sig(SIGILL) can't be ignored too. But I am fine either way.
> > > > >
> > > > > Current SIGILL sending is already pretty bad behavior that bites us
> > > > > periodically, instead of doubling down on killing the user space
> > > > > process because something about installing uprobe goes wrong, let's
> > > > > think about a bit less destructive way to do this. Mark uprobe for a
> > > > > particular process as defunct or "detaching it", or something along
> > > > > those lines. But not just kill innocent processes.
> > > >
> > > > Well. Yes, SIGILL sending acts as BUG_ON(), this is not nice and
> > >
> > > The problem is that this is actually not a bug but a very real and
> > > expected behavior with user-space using fibers or coroutines. Any
> > > suggestions how we can make this less violent, as it's actually quite
> > > dangerous and kills production applications when someone (unknowingly)
> > > tries to uretprobe trace them...
> > >
> >
> > IIUC, this SIGILL and coroutines/fibers issue is due to
> > uprobe_handle_trampoline not finding utrask->return_instances, because
> > a bit earlier we threw out pending uretprobes due to
> > arch_uretprobe_is_alive() returned false due to that `regs->sp <=
> > ret->stack` check (for x86-64).
>
> Well, unless I am totally confused this is another thing...
>
> Yes, arch_uretprobe_is_alive() sucks, it assumes the single stack.
>
> And I do not know how to fix this problem... uprobe_handle_trampoline()
> must fix regs->ip correctly somehow, otherwise it is pointless to contunue.
>

Argh, I completely forgot about regs->ip fix up, sorry.

> Note also that the current instruction_pointer_set(ri->orig_ret_vaddr)
> can be wrong if the probed task switches its stack.
>
> And we have the same problems with sigaltstack().
>
> ATM, I don't see any solution, and I don't really understand what do
> you propose...
>

Well, conceptually, I'd like us to try to find correct return_instance
instead of assuming the last one in the list is the one we want.
Whether that's technically sound, is a good question, I haven't done a
very thorough home work on this, sorry.

But we do have all pending return instances, and they record original
stack pointer, maybe we can do something with that? On user function
return, stack pointer should match what we had when we installed
return_instance, isn't that right? So in normal/happy case we'll do
what we do today, but if we find our that return_instance is "not
valid", we keep searching across all pending return instances. That's
going to be slower for sure, but at least it a) won't kill the app and
b) will be immensely useful.

> Oleg.
>