• Peter Zijlstra's avatar
    perf/core: Fix data race between perf_event_set_output() and perf_mmap_close() · 68e3c698
    Peter Zijlstra authored
    Yang Jihing reported a race between perf_event_set_output() and
    perf_mmap_close():
    
    	CPU1					CPU2
    
    	perf_mmap_close(e2)
    	  if (atomic_dec_and_test(&e2->rb->mmap_count)) // 1 - > 0
    	    detach_rest = true
    
    						ioctl(e1, IOC_SET_OUTPUT, e2)
    						  perf_event_set_output(e1, e2)
    
    	  ...
    	  list_for_each_entry_rcu(e, &e2->rb->event_list, rb_entry)
    	    ring_buffer_attach(e, NULL);
    	    // e1 isn't yet added and
    	    // therefore not detached
    
    						    ring_buffer_attach(e1, e2->rb)
    						      list_add_rcu(&e1->rb_entry,
    								   &e2->rb->event_list)
    
    After this; e1 is attached to an unmapped rb and a subsequent
    perf_mmap() will loop forever more:
    
    	again:
    		mutex_lock(&e->mmap_mutex);
    		if (event->rb) {
    			...
    			if (!atomic_inc_not_zero(&e->rb->mmap_count)) {
    				...
    				mutex_unlock(&e->mmap_mutex);
    				goto again;
    			}
    		}
    
    The loop in perf_mmap_close() holds e2->mmap_mutex, while the attach
    in perf_event_set_output() holds e1->mmap_mutex. As such there is no
    serialization to avoid this race.
    
    Change perf_event_set_output() to take both e1->mmap_mutex and
    e2->mmap_mutex to alleviate that problem. Additionally, have the loop
    in perf_mmap() detach the rb directly, this avoids having to wait for
    the concurrent perf_mmap_close() to get around to doing it to make
    progress.
    
    Fixes: 9bb5d40c ("perf: Fix mmap() accounting hole")
    Reported-by: default avatarYang Jihong <yangjihong1@huawei.com>
    Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
    Tested-by: default avatarYang Jihong <yangjihong1@huawei.com>
    Link: https://lkml.kernel.org/r/YsQ3jm2GR38SW7uD@worktop.programming.kicks-ass.net
    68e3c698
core.c 322 KB