[PATCH v1] uprobe: Assign session ID before publishing consumer

[PATCH v1] uprobe: Assign session ID before publishing consumer

[Binbin Deng]

consumer_add() publishes a consumer on uprobe->consumers
before assigning its session ID.  handler_chain() traverses the consumer
list under the trace RCU read-side critical section and can therefore
observe the new consumer while uc->id still contains its initial value.

The race is reachable when an uprobe at the same inode:offset is already
active and a task hits its entry breakpoint while another CPU registers a
second session consumer.  handler_chain() may store the stale ID in the
return_instance consumer array.  By the time the function returns,
uc->id has been assigned a nonzero value, so return_consumer_find() fails
to match the saved ID and the session ret_handler is incorrectly skipped.

consumer_rwsem does not close this window because handler_chain() is an RCU
reader and does not take that rwsem.  Assign the ID before list_add_rcu()
publishes the consumer; the release publish of the list node then orders
the initialized ID before readers that traverse the list.

Found during a static-analysis-assisted audit: candidate sites were
surfaced by a Coccinelle rule, and the reader/writer paths were
confirmed by LLM-assisted (GLM-5.3) code inspection.  The issue was
not observed at runtime; it is identified by code inspection only.

Fixes: 4d756095d399 ("uprobe: Add support for session consumer")
Signed-off-by: Binbin Deng <18983559317@163.com>
---
 kernel/events/uprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 7709ea882477..83e6aef61181 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1032,8 +1032,8 @@ static void consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
 	static atomic64_t id;
 
 	down_write(&uprobe->consumer_rwsem);
-	list_add_rcu(&uc->cons_node, &uprobe->consumers);
 	uc->id = (__u64) atomic64_inc_return(&id);
+	list_add_rcu(&uc->cons_node, &uprobe->consumers);
 	up_write(&uprobe->consumer_rwsem);
 }
 
-- 
2.43.0

Re: [PATCH v1] uprobe: Assign session ID before publishing consumer

[sashiko-bot]

> consumer_add() publishes a consumer on uprobe->consumers
> before assigning its session ID.  handler_chain() traverses the consumer
> list under the trace RCU read-side critical section and can therefore
> observe the new consumer while uc->id still contains its initial value.
> 
> The race is reachable when an uprobe at the same inode:offset is already
> active and a task hits its entry breakpoint while another CPU registers a
> second session consumer.  handler_chain() may store the stale ID in the
> return_instance consumer array.  By the time the function returns,
> uc->id has been assigned a nonzero value, so return_consumer_find() fails
> to match the saved ID and the session ret_handler is incorrectly skipped.
> 
> consumer_rwsem does not close this window because handler_chain() is an RCU
> reader and does not take that rwsem.  Assign the ID before list_add_rcu()
> publishes the consumer; the release publish of the list node then orders
> the initialized ID before readers that traverse the list.
> 
> Found during a static-analysis-assisted audit: candidate sites were
> surfaced by a Coccinelle rule, and the reader/writer paths were
> confirmed by LLM-assisted (GLM-5.3) code inspection.  The issue was
> not observed at runtime; it is identified by code inspection only.
> 
> Fixes: 4d756095d399 ("uprobe: Add support for session consumer")
> Signed-off-by: Binbin Deng <18983559317@163.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019080709.477548-1-18983559317@163.com?part=1

Re: [PATCH v1] uprobe: Assign session ID before publishing consumer

[Oleg Nesterov]

On 10/19, Binbin Deng wrote:
>
> consumer_add() publishes a consumer on uprobe->consumers
> before assigning its session ID.  handler_chain() traverses the consumer
> list under the trace RCU read-side critical section and can therefore
> observe the new consumer while uc->id still contains its initial value.
>
> The race is reachable when an uprobe at the same inode:offset is already
> active and a task hits its entry breakpoint while another CPU registers a
> second session consumer.  handler_chain() may store the stale ID in the
> return_instance consumer array.  By the time the function returns,
> uc->id has been assigned a nonzero value, so return_consumer_find() fails
> to match the saved ID and the session ret_handler is incorrectly skipped.

I guess we don't really care, this doesn't differ from the case when
consumer_add() is called right after list_for_each_entry_rcu() in
handler_chain().

But the patch looks "obviously good" to me anyway, so

> @@ -1032,8 +1032,8 @@ static void consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
>  	static atomic64_t id;
>
>  	down_write(&uprobe->consumer_rwsem);
> -	list_add_rcu(&uc->cons_node, &uprobe->consumers);
>  	uc->id = (__u64) atomic64_inc_return(&id);
> +	list_add_rcu(&uc->cons_node, &uprobe->consumers);
>  	up_write(&uprobe->consumer_rwsem);
>  }

Acked-by: Oleg Nesterov <oleg@redhat.com>

Re: [PATCH v1] uprobe: Assign session ID before publishing consumer

[Andrii Nakryiko]

On Mon, Oct 19, 2026 at 3:51 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 10/19, Binbin Deng wrote:
> >
> > consumer_add() publishes a consumer on uprobe->consumers
> > before assigning its session ID.  handler_chain() traverses the consumer
> > list under the trace RCU read-side critical section and can therefore
> > observe the new consumer while uc->id still contains its initial value.
> >
> > The race is reachable when an uprobe at the same inode:offset is already
> > active and a task hits its entry breakpoint while another CPU registers a
> > second session consumer.  handler_chain() may store the stale ID in the
> > return_instance consumer array.  By the time the function returns,
> > uc->id has been assigned a nonzero value, so return_consumer_find() fails
> > to match the saved ID and the session ret_handler is incorrectly skipped.
>
> I guess we don't really care, this doesn't differ from the case when
> consumer_add() is called right after list_for_each_entry_rcu() in
> handler_chain().

that uc->id is important for "uprobe session" semantics, which seems a
bit different situation compared to barely missing consumer_add() in
handler_chain(), but regardless, the fix looks good, we should apply
it

Acked-by: Andrii Nakryiko <andrii@kernel.org>

>
> But the patch looks "obviously good" to me anyway, so
>
> > @@ -1032,8 +1032,8 @@ static void consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
> >       static atomic64_t id;
> >
> >       down_write(&uprobe->consumer_rwsem);
> > -     list_add_rcu(&uc->cons_node, &uprobe->consumers);
> >       uc->id = (__u64) atomic64_inc_return(&id);
> > +     list_add_rcu(&uc->cons_node, &uprobe->consumers);
> >       up_write(&uprobe->consumer_rwsem);
> >  }
>
> Acked-by: Oleg Nesterov <oleg@redhat.com>
>

Re: [PATCH v1] uprobe: Assign session ID before publishing consumer

[Oleg Nesterov]

On 10/19, Andrii Nakryiko wrote:
>
> On Mon, Oct 19, 2026 at 3:51 AM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > On 10/19, Binbin Deng wrote:
> > >
> > > consumer_add() publishes a consumer on uprobe->consumers
> > > before assigning its session ID.  handler_chain() traverses the consumer
> > > list under the trace RCU read-side critical section and can therefore
> > > observe the new consumer while uc->id still contains its initial value.
> > >
> > > The race is reachable when an uprobe at the same inode:offset is already
> > > active and a task hits its entry breakpoint while another CPU registers a
> > > second session consumer.  handler_chain() may store the stale ID in the
> > > return_instance consumer array.  By the time the function returns,
> > > uc->id has been assigned a nonzero value, so return_consumer_find() fails
> > > to match the saved ID and the session ret_handler is incorrectly skipped.
> >
> > I guess we don't really care, this doesn't differ from the case when
> > consumer_add() is called right after list_for_each_entry_rcu() in
> > handler_chain().
>
> that uc->id is important for "uprobe session" semantics,

Yes, but

> which seems a
> bit different situation compared to barely missing consumer_add() in
> handler_chain(),

I must have missed something... If handler_chain() misses consumer_add()
then push_consumer(uc->id) won't be called.  This means that then later
handle_uretprobe_chain() -> return_consumer_find(uc->id) will equally fail?

> but regardless, the fix looks good, we should apply it
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>

Agreed, I have acked it too.

Oleg.

Re: [PATCH v1] uprobe: Assign session ID before publishing consumer

[Andrii Nakryiko]

On Mon, Oct 19, 2026 at 9:31 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 10/19, Andrii Nakryiko wrote:
> >
> > On Mon, Oct 19, 2026 at 3:51 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > >
> > > On 10/19, Binbin Deng wrote:
> > > >
> > > > consumer_add() publishes a consumer on uprobe->consumers
> > > > before assigning its session ID.  handler_chain() traverses the consumer
> > > > list under the trace RCU read-side critical section and can therefore
> > > > observe the new consumer while uc->id still contains its initial value.
> > > >
> > > > The race is reachable when an uprobe at the same inode:offset is already
> > > > active and a task hits its entry breakpoint while another CPU registers a
> > > > second session consumer.  handler_chain() may store the stale ID in the
> > > > return_instance consumer array.  By the time the function returns,
> > > > uc->id has been assigned a nonzero value, so return_consumer_find() fails
> > > > to match the saved ID and the session ret_handler is incorrectly skipped.
> > >
> > > I guess we don't really care, this doesn't differ from the case when
> > > consumer_add() is called right after list_for_each_entry_rcu() in
> > > handler_chain().
> >
> > that uc->id is important for "uprobe session" semantics,
>
> Yes, but
>
> > which seems a
> > bit different situation compared to barely missing consumer_add() in
> > handler_chain(),
>
> I must have missed something... If handler_chain() misses consumer_add()
> then push_consumer(uc->id) won't be called.  This means that then later
> handle_uretprobe_chain() -> return_consumer_find(uc->id) will equally fail?

yes, you are right, and we are in violent agreement :)

>
> > but regardless, the fix looks good, we should apply it
> >
> > Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> Agreed, I have acked it too.
>
> Oleg.
>