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 <[email protected]>
