On Tue, Jul 26, 2022 at 09:33:56AM +0800, Yang Jihong wrote:
> commit 68e3c69803dada336893640110cb87221bb01dcf upstream.
>
> 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: 9bb5d40cd93c ("perf: Fix mmap() accounting hole")
> Reported-by: Yang Jihong <yangjihong1@xxxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> Tested-by: Yang Jihong <yangjihong1@xxxxxxxxxx>
> Link: https://lkml.kernel.org/r/YsQ3jm2GR38SW7uD@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> [YJH: backport to 4.19: adjusted context]
> Signed-off-by: Yang Jihong <yangjihong1@xxxxxxxxxx>
> ---
> kernel/events/core.c | 45 ++++++++++++++++++++++++++++++--------------
> 1 file changed, 31 insertions(+), 14 deletions(-)
Sasha already queued this up, thanks.
greg k-h
