From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965602AbdGTOO7 (ORCPT ); Thu, 20 Jul 2017 10:14:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52662 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933668AbdGTOO6 (ORCPT ); Thu, 20 Jul 2017 10:14:58 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 21A5CC0272DC Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=none smtp.mailfrom=jolsa@kernel.org DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 21A5CC0272DC From: Jiri Olsa To: Peter Zijlstra Cc: lkml , Ingo Molnar , Andi Kleen , Alexander Shishkin Subject: [PATCH] perf: Fix locking for children siblings group read Date: Thu, 20 Jul 2017 16:14:55 +0200 Message-Id: <20170720141455.2106-1-jolsa@kernel.org> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Thu, 20 Jul 2017 14:14:58 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. Tested-by: Andi Kleen Signed-off-by: Jiri Olsa --- kernel/events/core.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kernel/events/core.c b/kernel/events/core.c index 9747e422ab20..585955b41489 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -4365,7 +4365,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; @@ -4395,12 +4397,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; } -- 2.9.4