From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@redhat.com>,
linux-kernel@vger.kernel.org, vince@deater.net,
eranian@google.com, Arnaldo Carvalho de Melo <acme@infradead.org>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>
Subject: [PATCH v2 2/5] perf: Fix aux_mmap_count vs aux_refcount order
Date: Tue, 6 Sep 2016 16:23:50 +0300 [thread overview]
Message-ID: <20160906132353.19887-3-alexander.shishkin@linux.intel.com> (raw)
In-Reply-To: <20160906132353.19887-1-alexander.shishkin@linux.intel.com>
The order of accesses to ring buffer's aux_mmap_count and aux_refcount
has to be preserved across the users, namely perf_mmap_close() and
perf_aux_output_begin(), otherwise the inversion can result in the latter
holding the last reference to the aux buffer and subsequently free'ing
it in atomic context, triggering a warning.
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 257 at kernel/events/ring_buffer.c:541 __rb_free_aux+0x11a/0x130
> CPU: 0 PID: 257 Comm: stopbug Not tainted 4.8.0-rc1+ #2596
> Call Trace:
> [<ffffffff810f3e0b>] __warn+0xcb/0xf0
> [<ffffffff810f3f3d>] warn_slowpath_null+0x1d/0x20
> [<ffffffff8121182a>] __rb_free_aux+0x11a/0x130
> [<ffffffff812127a8>] rb_free_aux+0x18/0x20
> [<ffffffff81212913>] perf_aux_output_begin+0x163/0x1e0
> [<ffffffff8100c33a>] bts_event_start+0x3a/0xd0
> [<ffffffff8100c42d>] bts_event_add+0x5d/0x80
> [<ffffffff81203646>] event_sched_in.isra.104+0xf6/0x2f0
> [<ffffffff8120652e>] group_sched_in+0x6e/0x190
> [<ffffffff8120694e>] ctx_sched_in+0x2fe/0x5f0
> [<ffffffff81206ca0>] perf_event_sched_in+0x60/0x80
> [<ffffffff81206d1b>] ctx_resched+0x5b/0x90
> [<ffffffff81207281>] __perf_event_enable+0x1e1/0x240
> [<ffffffff81200639>] event_function+0xa9/0x180
> [<ffffffff81202000>] ? perf_cgroup_attach+0x70/0x70
> [<ffffffff8120203f>] remote_function+0x3f/0x50
> [<ffffffff811971f3>] flush_smp_call_function_queue+0x83/0x150
> [<ffffffff81197bd3>] generic_smp_call_function_single_interrupt+0x13/0x60
> [<ffffffff810a6477>] smp_call_function_single_interrupt+0x27/0x40
> [<ffffffff81a26ea9>] call_function_single_interrupt+0x89/0x90
> [<ffffffff81120056>] finish_task_switch+0xa6/0x210
> [<ffffffff81120017>] ? finish_task_switch+0x67/0x210
> [<ffffffff81a1e83d>] __schedule+0x3dd/0xb50
> [<ffffffff81a1efe5>] schedule+0x35/0x80
> [<ffffffff81128031>] sys_sched_yield+0x61/0x70
> [<ffffffff81a25be5>] entry_SYSCALL_64_fastpath+0x18/0xa8
> ---[ end trace 6235f556f5ea83a9 ]---
This patch puts the checks in perf_aux_output_begin() in the same order
as that of perf_mmap_close().
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
kernel/events/ring_buffer.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index ae9b90dc9a..71c9194b1f 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -330,15 +330,19 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
if (!rb)
return NULL;
- if (!rb_has_aux(rb) || !atomic_inc_not_zero(&rb->aux_refcount))
- goto err;
-
/*
- * If rb::aux_mmap_count is zero (and rb_has_aux() above went through),
- * the aux buffer is in perf_mmap_close(), about to get freed.
+ * If aux_mmap_count is zero, the aux buffer is in perf_mmap_close(),
+ * about to get freed, so we leave immediately.
+ *
+ * Checking rb::aux_mmap_count and rb::refcount has to be done in
+ * the same order, see perf_mmap_close. Otherwise we end up freeing
+ * aux pages in this path, which is a bug, because in_atomic().
*/
if (!atomic_read(&rb->aux_mmap_count))
- goto err_put;
+ goto err;
+
+ if (!rb_has_aux(rb) || !atomic_inc_not_zero(&rb->aux_refcount))
+ goto err;
/*
* Nesting is not supported for AUX area, make sure nested
--
2.9.3
next prev parent reply other threads:[~2016-09-06 13:28 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-06 13:23 [PATCH v2 0/5] perf, bts: Fallout from the fuzzer for perf/urgent Alexander Shishkin
2016-09-06 13:23 ` [PATCH v2 1/5] perf: Fix a race between mmap_close and set_output of AUX events Alexander Shishkin
2016-09-10 12:38 ` [tip:perf/core] perf/core: Fix a race between mmap_close() and set_output() " tip-bot for Alexander Shishkin
2016-09-06 13:23 ` Alexander Shishkin [this message]
2016-09-10 12:38 ` [tip:perf/core] perf/core: Fix aux_mmap_count vs aux_refcount order tip-bot for Alexander Shishkin
2016-09-06 13:23 ` [PATCH v2 3/5] perf/x86/intel/bts: Fix confused ordering of PMU callbacks Alexander Shishkin
2016-09-10 12:39 ` [tip:perf/core] " tip-bot for Alexander Shishkin
2016-09-06 13:23 ` [PATCH v2 4/5] perf/x86/intel/bts: Fix BTS PMI detection Alexander Shishkin
2016-09-10 12:39 ` [tip:perf/core] " tip-bot for Alexander Shishkin
2016-09-20 13:12 ` [PATCH] perf/x86/intel/bts: don't dereference ds unconditionally Sebastian Andrzej Siewior
2016-09-20 13:44 ` Alexander Shishkin
2016-09-20 13:54 ` Alexander Shishkin
2016-09-20 14:13 ` [tip:perf/urgent] perf/x86/intel/bts: Make sure debug store is valid tip-bot for Sebastian Andrzej Siewior
2016-09-06 13:23 ` [PATCH v2 5/5] perf/x86/intel/bts: Kill a silly warning Alexander Shishkin
2016-09-10 12:40 ` [tip:perf/core] " tip-bot for Alexander Shishkin
2016-09-06 17:19 ` [PATCH v2 0/5] perf, bts: Fallout from the fuzzer for perf/urgent Ingo Molnar
2016-09-07 0:13 ` Vince Weaver
2016-09-07 15:20 ` Alexander Shishkin
2016-09-07 15:36 ` Vince Weaver
2016-09-07 16:38 ` Peter Zijlstra
2016-09-07 18:33 ` Alexander Shishkin
2016-09-08 3:36 ` Vince Weaver
2016-09-08 8:51 ` Alexander Shishkin
2016-09-08 12:54 ` Vince Weaver
2016-09-08 6:21 ` Ingo Molnar
2016-09-08 6:23 ` Ingo Molnar
2016-09-08 8:43 ` Alexander Shishkin
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=20160906132353.19887-3-alexander.shishkin@linux.intel.com \
--to=alexander.shishkin@linux.intel.com \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@infradead.org \
--cc=eranian@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=vince@deater.net \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).