From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754170AbdGUJmI (ORCPT ); Fri, 21 Jul 2017 05:42:08 -0400 Received: from terminus.zytor.com ([65.50.211.136]:58655 "EHLO terminus.zytor.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754028AbdGUJmG (ORCPT ); Fri, 21 Jul 2017 05:42:06 -0400 Date: Fri, 21 Jul 2017 02:37:28 -0700 From: tip-bot for Jiri Olsa Message-ID: Cc: linux-kernel@vger.kernel.org, peterz@infradead.org, acme@redhat.com, jolsa@kernel.org, alexander.shishkin@linux.intel.com, hpa@zytor.com, a.p.zijlstra@chello.nl, jolsa@redhat.com, torvalds@linux-foundation.org, ak@linux.intel.com, tglx@linutronix.de, mingo@kernel.org Reply-To: mingo@kernel.org, tglx@linutronix.de, ak@linux.intel.com, jolsa@redhat.com, torvalds@linux-foundation.org, a.p.zijlstra@chello.nl, hpa@zytor.com, alexander.shishkin@linux.intel.com, acme@redhat.com, jolsa@kernel.org, peterz@infradead.org, linux-kernel@vger.kernel.org In-Reply-To: <20170720141455.2106-1-jolsa@kernel.org> References: <20170720141455.2106-1-jolsa@kernel.org> To: linux-tip-commits@vger.kernel.org Subject: [tip:perf/urgent] perf/core: Fix locking for children siblings group read Git-Commit-ID: 2aeb1883547626d82c597cce2c99f0b9c62e2425 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: 2aeb1883547626d82c597cce2c99f0b9c62e2425 Gitweb: http://git.kernel.org/tip/2aeb1883547626d82c597cce2c99f0b9c62e2425 Author: Jiri Olsa AuthorDate: Thu, 20 Jul 2017 16:14:55 +0200 Committer: Ingo Molnar CommitDate: Fri, 21 Jul 2017 09:54:23 +0200 perf/core: Fix locking for children siblings group read We're missing ctx lock when iterating children siblings within the perf_read path for group reading. Following race and crash can happen: User space doing read syscall on event group leader: T1: perf_read lock event->ctx->mutex perf_read_group lock leader->child_mutex __perf_read_group_add(child) list_for_each_entry(sub, &leader->sibling_list, group_entry) ----> sub might be invalid at this point, because it could get removed via perf_event_exit_task_context in T2 Child exiting and cleaning up its events: T2: perf_event_exit_task_context lock ctx->mutex list_for_each_entry_safe(child_event, next, &child_ctx->event_list,... perf_event_exit_event(child) lock ctx->lock perf_group_detach(child) unlock ctx->lock ----> child is removed from sibling_list without any sync with T1 path above ... free_event(child) Before the child is removed from the leader's child_list, (and thus is omitted from perf_read_group processing), we need to ensure that perf_read_group touches child's siblings under its ctx->lock. Peter further notes: | One additional note; this bug got exposed by commit: | | ba5213ae6b88 ("perf/core: Correct event creation with PERF_FORMAT_GROUP") | | which made it possible to actually trigger this code-path. Tested-by: Andi Kleen Signed-off-by: Jiri Olsa Acked-by: Peter Zijlstra (Intel) Cc: Alexander Shishkin Cc: Arnaldo Carvalho de Melo Cc: Jiri Olsa Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Peter Zijlstra Cc: Thomas Gleixner Fixes: ba5213ae6b88 ("perf/core: Correct event creation with PERF_FORMAT_GROUP") Link: http://lkml.kernel.org/r/20170720141455.2106-1-jolsa@kernel.org Signed-off-by: Ingo Molnar --- kernel/events/core.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kernel/events/core.c b/kernel/events/core.c index c9cdbd3..c17c088 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -4372,7 +4372,9 @@ EXPORT_SYMBOL_GPL(perf_event_read_value); static int __perf_read_group_add(struct perf_event *leader, u64 read_format, u64 *values) { + struct perf_event_context *ctx = leader->ctx; struct perf_event *sub; + unsigned long flags; int n = 1; /* skip @nr */ int ret; @@ -4402,12 +4404,15 @@ static int __perf_read_group_add(struct perf_event *leader, if (read_format & PERF_FORMAT_ID) values[n++] = primary_event_id(leader); + raw_spin_lock_irqsave(&ctx->lock, flags); + list_for_each_entry(sub, &leader->sibling_list, group_entry) { values[n++] += perf_event_count(sub); if (read_format & PERF_FORMAT_ID) values[n++] = primary_event_id(sub); } + raw_spin_unlock_irqrestore(&ctx->lock, flags); return 0; }