From patchwork Fri Feb 24 13:06:02 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wang Nan X-Patchwork-Id: 94477 Delivered-To: patch@linaro.org Received: by 10.140.20.99 with SMTP id 90csp681081qgi; Fri, 24 Feb 2017 05:08:11 -0800 (PST) X-Received: by 10.98.214.4 with SMTP id r4mr3357177pfg.185.1487941691396; Fri, 24 Feb 2017 05:08:11 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id j20si7348763pfj.127.2017.02.24.05.08.11; Fri, 24 Feb 2017 05:08:11 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751430AbdBXNHu (ORCPT + 25 others); Fri, 24 Feb 2017 08:07:50 -0500 Received: from szxga02-in.huawei.com ([45.249.212.188]:2903 "EHLO dggrg02-dlp.huawei.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1751291AbdBXNHS (ORCPT ); Fri, 24 Feb 2017 08:07:18 -0500 Received: from 172.30.72.56 (EHLO DGGEMM406-HUB.china.huawei.com) ([172.30.72.56]) by dggrg02-dlp.huawei.com (MOS 4.4.6-GA FastPath queued) with ESMTP id AIY57539; Fri, 24 Feb 2017 21:06:12 +0800 (CST) Received: from DGGEML403-HUB.china.huawei.com (10.3.17.33) by DGGEMM406-HUB.china.huawei.com (10.3.20.214) with Microsoft SMTP Server (TLS) id 14.3.301.0; Fri, 24 Feb 2017 21:06:10 +0800 Received: from linux-4hy3.site (10.107.193.248) by DGGEML403-HUB.china.huawei.com (10.3.17.33) with Microsoft SMTP Server id 14.3.301.0; Fri, 24 Feb 2017 21:06:02 +0800 From: Wang Nan To: Sasha Levin CC: , , , , Peter Zijlstra , Alexander Shishkin , Linus Torvalds , Thomas Gleixner , "Ingo Molnar" , Wang Nan Subject: [PATCH -stable 4.1 1/2] perf: Restructure perf syscall point of no return Date: Fri, 24 Feb 2017 13:06:02 +0000 Message-ID: <20170224130603.147418-2-wangnan0@huawei.com> X-Mailer: git-send-email 2.10.1 In-Reply-To: <20170224130603.147418-1-wangnan0@huawei.com> References: <20170224130603.147418-1-wangnan0@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.107.193.248] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090201.58B02FC5.0017, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: f73f73c3ca9732129fdec399b385d4e1 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Peter Zijlstra [ Upstream commit f55fc2a57cc9ca3b1bb4fb8eb25b6e1989e5b993 ] The exclusive_event_installable() stuff only works because its exclusive with the grouping bits. Rework the code such that there is a sane place to error out before we go do things we cannot undo. CVE 2017-6001 Signed-off-by: Peter Zijlstra (Intel) Cc: Alexander Shishkin Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: linux-kernel@vger.kernel.org Signed-off-by: Ingo Molnar Signed-off-by: Wang Nan --- kernel/events/core.c | 49 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 17 deletions(-) -- 2.10.1 diff --git a/kernel/events/core.c b/kernel/events/core.c index 6da64f0..77be116 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -8124,13 +8124,30 @@ SYSCALL_DEFINE5(perf_event_open, if (move_group) { gctx = group_leader->ctx; + mutex_lock_double(&gctx->mutex, &ctx->mutex); + } else { + mutex_lock(&ctx->mutex); + } + + /* + * Must be under the same ctx::mutex as perf_install_in_context(), + * because we need to serialize with concurrent event creation. + */ + if (!exclusive_event_installable(event, ctx)) { + /* exclusive and group stuff are assumed mutually exclusive */ + WARN_ON_ONCE(move_group); + + err = -EBUSY; + goto err_locked; + } + WARN_ON_ONCE(ctx->parent_ctx); + + if (move_group) { /* * 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); list_for_each_entry(sibling, &group_leader->sibling_list, @@ -8138,13 +8155,7 @@ SYSCALL_DEFINE5(perf_event_open, perf_remove_from_context(sibling, false); put_ctx(gctx); } - } else { - mutex_lock(&ctx->mutex); - } - - WARN_ON_ONCE(ctx->parent_ctx); - if (move_group) { /* * Wait for everybody to stop referencing the events through * the old lists, before installing it on new lists. @@ -8176,22 +8187,20 @@ SYSCALL_DEFINE5(perf_event_open, perf_event__state_init(group_leader); perf_install_in_context(ctx, group_leader, group_leader->cpu); get_ctx(ctx); - } - if (!exclusive_event_installable(event, ctx)) { - err = -EBUSY; - mutex_unlock(&ctx->mutex); - fput(event_file); - goto err_context; + /* + * Now that all events are installed in @ctx, nothing + * references @gctx anymore, so drop the last reference we have + * on it. + */ + put_ctx(gctx); } perf_install_in_context(ctx, event, event->cpu); perf_unpin_context(ctx); - if (move_group) { + if (move_group) mutex_unlock(&gctx->mutex); - put_ctx(gctx); - } mutex_unlock(&ctx->mutex); put_online_cpus(); @@ -8218,6 +8227,12 @@ SYSCALL_DEFINE5(perf_event_open, fd_install(event_fd, event_file); return event_fd; +err_locked: + if (move_group) + mutex_unlock(&gctx->mutex); + mutex_unlock(&ctx->mutex); +/* err_file: */ + fput(event_file); err_context: perf_unpin_context(ctx); put_ctx(ctx); From patchwork Fri Feb 24 13:06:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wang Nan X-Patchwork-Id: 94478 Delivered-To: patch@linaro.org Received: by 10.140.20.99 with SMTP id 90csp681177qgi; Fri, 24 Feb 2017 05:08:22 -0800 (PST) X-Received: by 10.99.141.195 with SMTP id z186mr3398838pgd.141.1487941702418; Fri, 24 Feb 2017 05:08:22 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id j20si7348763pfj.127.2017.02.24.05.08.19; Fri, 24 Feb 2017 05:08:22 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751484AbdBXNIR (ORCPT + 25 others); Fri, 24 Feb 2017 08:08:17 -0500 Received: from szxga02-in.huawei.com ([45.249.212.188]:2901 "EHLO dggrg02-dlp.huawei.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1751285AbdBXNHS (ORCPT ); Fri, 24 Feb 2017 08:07:18 -0500 Received: from 172.30.72.56 (EHLO DGGEMM406-HUB.china.huawei.com) ([172.30.72.56]) by dggrg02-dlp.huawei.com (MOS 4.4.6-GA FastPath queued) with ESMTP id AIY57540; Fri, 24 Feb 2017 21:06:13 +0800 (CST) Received: from DGGEML403-HUB.china.huawei.com (10.3.17.33) by DGGEMM406-HUB.china.huawei.com (10.3.20.214) with Microsoft SMTP Server (TLS) id 14.3.301.0; Fri, 24 Feb 2017 21:06:10 +0800 Received: from linux-4hy3.site (10.107.193.248) by DGGEML403-HUB.china.huawei.com (10.3.17.33) with Microsoft SMTP Server id 14.3.301.0; Fri, 24 Feb 2017 21:06:03 +0800 From: Wang Nan To: Sasha Levin 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 , Wang Nan Subject: [PATCH -stable 4.1 2/2] perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race Date: Fri, 24 Feb 2017 13:06:03 +0000 Message-ID: <20170224130603.147418-3-wangnan0@huawei.com> X-Mailer: git-send-email 2.10.1 In-Reply-To: <20170224130603.147418-1-wangnan0@huawei.com> References: <20170224130603.147418-1-wangnan0@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.107.193.248] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090201.58B02FC5.0100, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 252ebf62c7d8f8218e8df8cd89bf0e57 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Peter Zijlstra [ Upstream commit 321027c1fe77f892f4ea07846aeae08cefbbb290 ] 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. CVE-2017-6001 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 Signed-off-by: Wang Nan [ - Correct code context - Use group_flags instead of group_caps ] --- kernel/events/core.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 4 deletions(-) -- 2.10.1 diff --git a/kernel/events/core.c b/kernel/events/core.c index 77be116..3973df8 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -7900,6 +7900,37 @@ static int perf_event_set_clock(struct perf_event *event, clockid_t clk_id) return 0; } +/* + * 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 = READ_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 * @@ -8123,8 +8154,26 @@ SYSCALL_DEFINE5(perf_event_open, } if (move_group) { - gctx = group_leader->ctx; - mutex_lock_double(&gctx->mutex, &ctx->mutex); + 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; + } + } } else { mutex_lock(&ctx->mutex); } @@ -8200,7 +8249,7 @@ SYSCALL_DEFINE5(perf_event_open, perf_unpin_context(ctx); if (move_group) - mutex_unlock(&gctx->mutex); + perf_event_ctx_unlock(group_leader, gctx); mutex_unlock(&ctx->mutex); put_online_cpus(); @@ -8229,7 +8278,7 @@ SYSCALL_DEFINE5(perf_event_open, err_locked: if (move_group) - mutex_unlock(&gctx->mutex); + perf_event_ctx_unlock(group_leader, gctx); mutex_unlock(&ctx->mutex); /* err_file: */ fput(event_file);