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

[Andrii Nakryiko]

On Mon, Oct 19, 2026 at 9:31 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> On 10/19, Andrii Nakryiko wrote:
> >
> > On Mon, Oct 19, 2026 at 3:51 AM Oleg Nesterov <oleg@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> Agreed, I have acked it too.
>
> Oleg.
>