From patchwork Tue Jan 9 13:42:42 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amit Pundir X-Patchwork-Id: 123959 Delivered-To: patch@linaro.org Received: by 10.140.22.227 with SMTP id 90csp4037086qgn; Tue, 9 Jan 2018 05:42:52 -0800 (PST) X-Google-Smtp-Source: ACJfBouUK7o1CN7zEJwG3UATETkOk8YgRlQZYF79qI/S82JxX4OswTLqJLFtHI/6XIOgew5aCh8/ X-Received: by 10.99.60.13 with SMTP id j13mr2287927pga.306.1515505372365; Tue, 09 Jan 2018 05:42:52 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1515505372; cv=none; d=google.com; s=arc-20160816; b=U3wofFjAXMjDoAUsRv7m3MZ3hvNmTqHXmkD10hsaMlxv6bRlb4+4LcZYQSHQgutw9I b51qfPpfk588T7v1WOXvdfEZMwF1G5gLfz1c4qb/SvMx+kVv5QDdhzC8UfDxz3iWmUl4 9OzlIFuP6T45j2KNEAzDoKRkzJFOQ5IZTPi61GwSzist44prrs22Po8a+cPQNNb5aTab WLrp8itcaJ5MtsgmPQHjViG2hgRaKvnVyC9+Nbr/PrdpITx+4dM7V8LU5BeJL+urcfnX UggfrxCZ9i/VSBHIMpAT65unftp7fRSYy44fGduESgronxcTc5va6nazb9hXosRjOxI7 nDkg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :dkim-signature:arc-authentication-results; bh=/GnS4KPZjbllGoqTvRH+93ncM49RgSu/iVm+4VqNurU=; b=wxzKQaj0sV55JNEMemnumH3oak/1NMknet+s7Tz9ML9t3Eh4/9rRe4EVVJSR/ll24F Ojjvca0da2rS5Oc6vIbfpQJQZiL0M4VYcqnX8kjtgO206or7vRm4sUyC/Xft0BDP5FtU lEmPvm4p5kaUgFH8PH78APwzD334t1Gwgfl4Y/j78qpNt9zFFToOjeuMrLfjB8DARTg2 i0fJsyRt8Tqd7cNafXjmU1dyE/Mn1yypvPZgmfaeQWt3sH8gAsWOfcpOiX58NWhPJ1QD kvCVffC/mPFj/QviFHU6SYlg92wJgxGQ7zWmTKUxidO0a7Xrtw+QNfUpjVoGsQrqeZje u76A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=UcrLC5mb; spf=pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id n13si9035458pgc.241.2018.01.09.05.42.52; Tue, 09 Jan 2018 05:42:52 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=UcrLC5mb; spf=pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751922AbeAINmv (ORCPT + 10 others); Tue, 9 Jan 2018 08:42:51 -0500 Received: from mail-pf0-f193.google.com ([209.85.192.193]:38973 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751644AbeAINmu (ORCPT ); Tue, 9 Jan 2018 08:42:50 -0500 Received: by mail-pf0-f193.google.com with SMTP id e11so4840473pff.6 for ; Tue, 09 Jan 2018 05:42:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=/GnS4KPZjbllGoqTvRH+93ncM49RgSu/iVm+4VqNurU=; b=UcrLC5mbcUPpdOw5abts4Iv/BgfXVmSNFxD2lgzhhQizkzBWd9OlthwAsukAEpRAQA DMR121xR5GbMZUDMVrKS4+1lmtU1riPgKZ7IWiZ4N7QXxsWgg0aoI37/xYesu5UhoPS8 rQmkLmrBv6SW3/0v1mb1roRl0dRXic/RjkoVY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=/GnS4KPZjbllGoqTvRH+93ncM49RgSu/iVm+4VqNurU=; b=HvHxdqR11cJ0iYlkf3BFYliQIG4Z55rRPtZ6DDTM0XZtDobF4Bp8M5tuIH1AtxTLkC +AW8R3iG4PFO+Iq1T8xrUMHRJ14Sh8FT0GfFsCHIxhGM4VIIKl0AQhjhs0PTPdIO3O6L uXYm5tsOm+Sft39ZNOYTa4rU0Y+oBvMQaGRXcttWGnobAPgbKJ8lymtdOhN8M5oqob2F GKMrAoVBRpxZr79VlOEzyXvHOqkyOhfdjydWIe0nJyR1uionvKtx7fLtNYWuTMm+GIRk T4VeQgmIt5XaJVO8girc7d/meNfpqOgMDLh8+5LzwiRQueBTMOeEo2NRSTdqmhAW10p3 DtvA== X-Gm-Message-State: AKGB3mKXDhi7xIpngRducLBDbD7rID12T0/l9rq7hEAcLdlga+tgF1p1 mPyy5+NOhPxlQhjB7yfzwFSW2g== X-Received: by 10.84.130.47 with SMTP id 44mr15411020plc.120.1515505369949; Tue, 09 Jan 2018 05:42:49 -0800 (PST) Received: from localhost.localdomain ([106.51.16.9]) by smtp.gmail.com with ESMTPSA id p10sm33267276pfl.32.2018.01.09.05.42.44 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 09 Jan 2018 05:42:48 -0800 (PST) From: Amit Pundir To: Greg KH , Stable Cc: Peter Zijlstra , Alexander Shishkin , Arnaldo Carvalho de Melo , Arnaldo Carvalho de Melo , Jiri Olsa , Kees Cook , Linus Torvalds , Min Chong , Stephane Eranian , Thomas Gleixner , Vince Weaver , Ingo Molnar , Ben Hutchings , Suren Baghdasaryan Subject: [PATCH for-3.18.y] perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race Date: Tue, 9 Jan 2018 19:12:42 +0530 Message-Id: <1515505362-26760-1-git-send-email-amit.pundir@linaro.org> X-Mailer: git-send-email 2.7.4 Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Peter Zijlstra commit 321027c1fe77f892f4ea07846aeae08cefbbb290 upstream. Di Shen reported a race between two concurrent sys_perf_event_open() calls where both try and move the same pre-existing software group into a hardware context. The problem is exactly that described in commit: f63a8daa5812 ("perf: Fix event->ctx locking") ... where, while we wait for a ctx->mutex acquisition, the event->ctx relation can have changed under us. That very same commit failed to recognise sys_perf_event_context() as an external access vector to the events and thereby didn't apply the established locking rules correctly. So while one sys_perf_event_open() call is stuck waiting on mutex_lock_double(), the other (which owns said locks) moves the group about. So by the time the former sys_perf_event_open() acquires the locks, the context we've acquired is stale (and possibly dead). Apply the established locking rules as per perf_event_ctx_lock_nested() to the mutex_lock_double() for the 'move_group' case. This obviously means we need to validate state after we acquire the locks. Reported-by: Di Shen (Keen Lab) Tested-by: John Dias Signed-off-by: Peter Zijlstra (Intel) Cc: Alexander Shishkin Cc: Arnaldo Carvalho de Melo Cc: Arnaldo Carvalho de Melo Cc: Jiri Olsa Cc: Kees Cook Cc: Linus Torvalds Cc: Min Chong Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Thomas Gleixner Cc: Vince Weaver Fixes: f63a8daa5812 ("perf: Fix event->ctx locking") Link: http://lkml.kernel.org/r/20170106131444.GZ3174@twins.programming.kicks-ass.net Signed-off-by: Ingo Molnar [bwh: Backported to 3.16: - Use ACCESS_ONCE() instead of READ_ONCE() - Test perf_event::group_flags instead of group_caps - Add the err_locked cleanup block, which we didn't need before - Adjust context] Signed-off-by: Ben Hutchings Signed-off-by: Suren Baghdasaryan Signed-off-by: Amit Pundir --- This upstream patch is featured in recent Android Security bulletin. Picked up this backported patch from android-3.18. Build tested on 3.18.91 kernel/events/core.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 4 deletions(-) -- 2.7.4 diff --git a/kernel/events/core.c b/kernel/events/core.c index 9b12efcefdf7..de3303aab7d6 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -7414,6 +7414,37 @@ static void mutex_lock_double(struct mutex *a, struct mutex *b) mutex_lock_nested(b, SINGLE_DEPTH_NESTING); } +/* + * Variation on perf_event_ctx_lock_nested(), except we take two context + * mutexes. + */ +static struct perf_event_context * +__perf_event_ctx_lock_double(struct perf_event *group_leader, + struct perf_event_context *ctx) +{ + struct perf_event_context *gctx; + +again: + rcu_read_lock(); + gctx = ACCESS_ONCE(group_leader->ctx); + if (!atomic_inc_not_zero(&gctx->refcount)) { + rcu_read_unlock(); + goto again; + } + rcu_read_unlock(); + + mutex_lock_double(&gctx->mutex, &ctx->mutex); + + if (group_leader->ctx != gctx) { + mutex_unlock(&ctx->mutex); + mutex_unlock(&gctx->mutex); + put_ctx(gctx); + goto again; + } + + return gctx; +} + /** * sys_perf_event_open - open a performance event, associate it to a task/cpu * @@ -7626,14 +7657,31 @@ SYSCALL_DEFINE5(perf_event_open, } if (move_group) { - gctx = group_leader->ctx; + gctx = __perf_event_ctx_lock_double(group_leader, ctx); + + /* + * Check if we raced against another sys_perf_event_open() call + * moving the software group underneath us. + */ + if (!(group_leader->group_flags & PERF_GROUP_SOFTWARE)) { + /* + * If someone moved the group out from under us, check + * if this new event wound up on the same ctx, if so + * its the regular !move_group case, otherwise fail. + */ + if (gctx != ctx) { + err = -EINVAL; + goto err_locked; + } else { + perf_event_ctx_unlock(group_leader, gctx); + move_group = 0; + } + } /* * See perf_event_ctx_lock() for comments on the details * of swizzling perf_event::ctx. */ - mutex_lock_double(&gctx->mutex, &ctx->mutex); - perf_remove_from_context(group_leader, false); /* @@ -7674,7 +7722,7 @@ SYSCALL_DEFINE5(perf_event_open, perf_unpin_context(ctx); if (move_group) { - mutex_unlock(&gctx->mutex); + perf_event_ctx_unlock(group_leader, gctx); put_ctx(gctx); } mutex_unlock(&ctx->mutex); @@ -7703,6 +7751,11 @@ SYSCALL_DEFINE5(perf_event_open, fd_install(event_fd, event_file); return event_fd; +err_locked: + if (move_group) + perf_event_ctx_unlock(group_leader, gctx); + mutex_unlock(&ctx->mutex); + fput(event_file); err_context: perf_unpin_context(ctx); put_ctx(ctx);