From: Jiri Olsa <jolsa@kernel.org> To: Peter Zijlstra <a.p.zijlstra@chello.nl>, Arnaldo Carvalho de Melo <acme@kernel.org> Cc: lkml <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@kernel.org>, Namhyung Kim <namhyung@kernel.org>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Michael Petlan <mpetlan@redhat.com>, Wade Mealing <wmealing@redhat.com> Subject: [PATCH] perf: Fix race in perf_mmap_close function Date: Thu, 10 Sep 2020 12:41:53 +0200 [thread overview] Message-ID: <20200910104153.1672460-1-jolsa@kernel.org> (raw) There's a possible race in perf_mmap_close when checking ring buffer's mmap_count refcount value. The problem is that the mmap_count check is not atomic because we call atomic_dec and atomic_read separately. perf_mmap_close: ... atomic_dec(&rb->mmap_count); ... if (atomic_read(&rb->mmap_count)) goto out_put; <ring buffer detach> free_uid out_put: ring_buffer_put(rb); /* could be last */ The race can happen when we have two (or more) events sharing same ring buffer and they go through atomic_dec and then they both see 0 as refcount value later in atomic_read. Then both will go on and execute code which is meant to be run just once. The code that detaches ring buffer is probably fine to be executed more than once, but the problem is in calling free_uid, which will later on demonstrate in related crashes and refcount warnings, like: refcount_t: addition on 0; use-after-free. ... RIP: 0010:refcount_warn_saturate+0x6d/0xf ... Call Trace: prepare_creds+0x190/0x1e0 copy_creds+0x35/0x172 copy_process+0x471/0x1a80 _do_fork+0x83/0x3a0 __do_sys_wait4+0x83/0x90 __do_sys_clone+0x85/0xa0 do_syscall_64+0x5b/0x1e0 entry_SYSCALL_64_after_hwframe+0x44/0xa9 Using atomic decrease and check instead of separated calls. This fixes CVE-2020-14351. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- kernel/events/core.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/kernel/events/core.c b/kernel/events/core.c index 7ed5248f0445..29313cc54d9e 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -5903,8 +5903,6 @@ static void perf_mmap_close(struct vm_area_struct *vma) mutex_unlock(&event->mmap_mutex); } - atomic_dec(&rb->mmap_count); - if (!atomic_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex)) goto out_put; @@ -5912,7 +5910,7 @@ static void perf_mmap_close(struct vm_area_struct *vma) mutex_unlock(&event->mmap_mutex); /* If there's still other mmap()s of this buffer, we're done. */ - if (atomic_read(&rb->mmap_count)) + if (!atomic_dec_and_test(&rb->mmap_count)) goto out_put; /* -- 2.26.2
next reply other threads:[~2020-09-10 10:42 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-10 10:41 Jiri Olsa [this message] 2020-09-10 13:48 ` Namhyung Kim 2020-09-10 14:47 ` Jiri Olsa 2020-09-11 3:05 ` Namhyung Kim 2020-09-11 7:49 ` Jiri Olsa 2020-09-14 12:48 ` Namhyung Kim 2020-09-14 20:59 ` Jiri Olsa 2020-09-15 15:35 ` Michael Petlan 2020-09-16 11:53 ` [PATCHv2] " Jiri Olsa 2020-09-16 13:54 ` peterz 2020-09-16 14:38 ` Jiri Olsa 2020-09-16 14:05 ` peterz 2020-10-12 11:45 ` [tip: perf/core] perf/core: Fix race in the perf_mmap_close() function tip-bot2 for Jiri Olsa
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200910104153.1672460-1-jolsa@kernel.org \ --to=jolsa@kernel.org \ --cc=a.p.zijlstra@chello.nl \ --cc=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=mingo@kernel.org \ --cc=mpetlan@redhat.com \ --cc=namhyung@kernel.org \ --cc=wmealing@redhat.com \ --subject='Re: [PATCH] perf: Fix race in perf_mmap_close function' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).