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

[Oleg Nesterov <oleg@redhat.com>]

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>