All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -stable 4.1 0/2] perf/core: Fix CVE-2017-6001
@ 2017-02-24 13:06 Wang Nan
  2017-02-24 13:06 ` [PATCH -stable 4.1 1/2] perf: Restructure perf syscall point of no return Wang Nan
  2017-02-24 13:06 ` [PATCH -stable 4.1 2/2] perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race Wang Nan
  0 siblings, 2 replies; 3+ messages in thread
From: Wang Nan @ 2017-02-24 13:06 UTC (permalink / raw)
  To: Sasha Levin; +Cc: thunder.leizhen, fanjinke1, lizefan, linux-kernel, Wang Nan

These two patch are needed for stable 4.1. They fix CVE-2017-6001.

Peter Zijlstra (2):
  perf: Restructure perf syscall point of no return
  perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race

 kernel/events/core.c | 102 +++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 83 insertions(+), 19 deletions(-)

-- 
2.10.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH -stable 4.1 1/2] perf: Restructure perf syscall point of no return
  2017-02-24 13:06 [PATCH -stable 4.1 0/2] perf/core: Fix CVE-2017-6001 Wang Nan
@ 2017-02-24 13:06 ` Wang Nan
  2017-02-24 13:06 ` [PATCH -stable 4.1 2/2] perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race Wang Nan
  1 sibling, 0 replies; 3+ messages in thread
From: Wang Nan @ 2017-02-24 13:06 UTC (permalink / raw)
  To: Sasha Levin
  Cc: thunder.leizhen, fanjinke1, lizefan, linux-kernel,
	Peter Zijlstra, Alexander Shishkin, Linus Torvalds,
	Thomas Gleixner, Ingo Molnar, Wang Nan

From: Peter Zijlstra <peterz@infradead.org>

[ 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) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 kernel/events/core.c | 49 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 17 deletions(-)

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);
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH -stable 4.1 2/2] perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race
  2017-02-24 13:06 [PATCH -stable 4.1 0/2] perf/core: Fix CVE-2017-6001 Wang Nan
  2017-02-24 13:06 ` [PATCH -stable 4.1 1/2] perf: Restructure perf syscall point of no return Wang Nan
@ 2017-02-24 13:06 ` Wang Nan
  1 sibling, 0 replies; 3+ messages in thread
From: Wang Nan @ 2017-02-24 13:06 UTC (permalink / raw)
  To: Sasha Levin
  Cc: thunder.leizhen, fanjinke1, lizefan, linux-kernel,
	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

From: Peter Zijlstra <peterz@infradead.org>

[ 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 <joaodias@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Min Chong <mchong@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
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 <mingo@kernel.org>
Signed-off-by: Wang Nan <wangnan0@huawei.com>
[ - Correct code context
  - Use group_flags instead of group_caps
]
---
 kernel/events/core.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 53 insertions(+), 4 deletions(-)

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);
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-02-24 13:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-24 13:06 [PATCH -stable 4.1 0/2] perf/core: Fix CVE-2017-6001 Wang Nan
2017-02-24 13:06 ` [PATCH -stable 4.1 1/2] perf: Restructure perf syscall point of no return Wang Nan
2017-02-24 13:06 ` [PATCH -stable 4.1 2/2] perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race Wang Nan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.