From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936454AbcHJTsO (ORCPT ); Wed, 10 Aug 2016 15:48:14 -0400 Received: from terminus.zytor.com ([198.137.202.10]:57918 "EHLO mail.zytor.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S938502AbcHJSpj (ORCPT ); Wed, 10 Aug 2016 14:45:39 -0400 Date: Wed, 10 Aug 2016 10:56:17 -0700 From: tip-bot for Peter Zijlstra Message-ID: Cc: alexander.shishkin@linux.intel.com, vincent.weaver@maine.edu, tglx@linutronix.de, eranian@google.com, acme@redhat.com, jolsa@redhat.com, linux-kernel@vger.kernel.org, vegard.nossum@gmail.com, peterz@infradead.org, kan.liang@intel.com, mingo@kernel.org, hpa@zytor.com, davidcc@google.com, torvalds@linux-foundation.org Reply-To: mingo@kernel.org, kan.liang@intel.com, peterz@infradead.org, vegard.nossum@gmail.com, linux-kernel@vger.kernel.org, torvalds@linux-foundation.org, davidcc@google.com, hpa@zytor.com, tglx@linutronix.de, alexander.shishkin@linux.intel.com, vincent.weaver@maine.edu, jolsa@redhat.com, acme@redhat.com, eranian@google.com In-Reply-To: <20160804123724.GN6862@twins.programming.kicks-ass.net> References: <20160804123724.GN6862@twins.programming.kicks-ass.net> To: linux-tip-commits@vger.kernel.org Subject: [tip:perf/core] perf/core: Fix sideband list-iteration vs. event ordering NULL pointer deference crash Git-Commit-ID: 0b8f1e2e26bfc6b9abe3f0f3faba2cb0eecb9fb9 X-Mailer: tip-git-log-daemon Robot-ID: Robot-Unsubscribe: Contact to get blacklisted from these emails MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Commit-ID: 0b8f1e2e26bfc6b9abe3f0f3faba2cb0eecb9fb9 Gitweb: http://git.kernel.org/tip/0b8f1e2e26bfc6b9abe3f0f3faba2cb0eecb9fb9 Author: Peter Zijlstra AuthorDate: Thu, 4 Aug 2016 14:37:24 +0200 Committer: Ingo Molnar CommitDate: Wed, 10 Aug 2016 13:05:51 +0200 perf/core: Fix sideband list-iteration vs. event ordering NULL pointer deference crash Vegard Nossum reported that perf fuzzing generates a NULL pointer dereference crash: > Digging a bit deeper into this, it seems the event itself is getting > created by perf_event_open() and it gets added to the pmu_event_list > through: > > perf_event_open() > - perf_event_alloc() > - account_event() > - account_pmu_sb_event() > - attach_sb_event() > > so at this point the event is being attached but its ->ctx is still > NULL. It seems like ->ctx is set just a bit later in > perf_event_open(), though. > > But before that, __schedule() comes along and creates a stack trace > similar to the one above: > > __schedule() > - __perf_event_task_sched_out() > - perf_iterate_sb() > - perf_iterate_sb_cpu() > - event_filter_match() > - perf_cgroup_match() > - __get_cpu_context() > - (dereference ctx which is NULL) > > So I guess the question is... should the event be attached (= put on > the list) before ->ctx gets set? Or should the cgroup code check for a > NULL ->ctx? The latter seems like the simplest solution. Moving the list-add later creates a bit of a mess. Reported-by: Vegard Nossum Tested-by: Vegard Nossum Tested-by: Vince Weaver Signed-off-by: Peter Zijlstra (Intel) Cc: Alexander Shishkin Cc: Arnaldo Carvalho de Melo Cc: David Carrillo-Cisneros Cc: Jiri Olsa Cc: Kan Liang Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Thomas Gleixner Fixes: f2fb6bef9251 ("perf/core: Optimize side-band event delivery") Link: http://lkml.kernel.org/r/20160804123724.GN6862@twins.programming.kicks-ass.net Signed-off-by: Ingo Molnar --- kernel/events/core.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/kernel/events/core.c b/kernel/events/core.c index a19550d..87d02b8 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -1716,8 +1716,8 @@ static inline int pmu_filter_match(struct perf_event *event) static inline int event_filter_match(struct perf_event *event) { - return (event->cpu == -1 || event->cpu == smp_processor_id()) - && perf_cgroup_match(event) && pmu_filter_match(event); + return (event->cpu == -1 || event->cpu == smp_processor_id()) && + perf_cgroup_match(event) && pmu_filter_match(event); } static void @@ -1737,8 +1737,8 @@ event_sched_out(struct perf_event *event, * maintained, otherwise bogus information is return * via read() for time_enabled, time_running: */ - if (event->state == PERF_EVENT_STATE_INACTIVE - && !event_filter_match(event)) { + if (event->state == PERF_EVENT_STATE_INACTIVE && + !event_filter_match(event)) { delta = tstamp - event->tstamp_stopped; event->tstamp_running += delta; event->tstamp_stopped = tstamp; @@ -2236,10 +2236,15 @@ perf_install_in_context(struct perf_event_context *ctx, lockdep_assert_held(&ctx->mutex); - event->ctx = ctx; if (event->cpu != -1) event->cpu = cpu; + /* + * Ensures that if we can observe event->ctx, both the event and ctx + * will be 'complete'. See perf_iterate_sb_cpu(). + */ + smp_store_release(&event->ctx, ctx); + if (!task) { cpu_function_call(cpu, __perf_install_in_context, event); return; @@ -5969,6 +5974,14 @@ static void perf_iterate_sb_cpu(perf_iterate_f output, void *data) struct perf_event *event; list_for_each_entry_rcu(event, &pel->list, sb_list) { + /* + * Skip events that are not fully formed yet; ensure that + * if we observe event->ctx, both event and ctx will be + * complete enough. See perf_install_in_context(). + */ + if (!smp_load_acquire(&event->ctx)) + continue; + if (event->state < PERF_EVENT_STATE_INACTIVE) continue; if (!event_filter_match(event))