[linux] 07/07: perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race (CVE-2017-6001)
debian-kernel at lists.debian.org
debian-kernel at lists.debian.org
Tue Feb 21 21:40:58 UTC 2017
This is an automated email from the git hooks/post-receive script.
benh pushed a commit to branch wheezy-security
in repository linux.
commit cc5047b03a57287ef0169278443e8ee9efd2ae80
Author: Ben Hutchings <ben at decadent.org.uk>
Date: Tue Feb 21 21:38:49 2017 +0000
perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race (CVE-2017-6001)
...plus a dependency
---
debian/changelog | 3 +
...ix-concurrent-sys_perf_event_open-vs.-mov.patch | 153 +++++++++++++++++++++
.../bugfix/all/perf-do-not-double-free.patch | 48 +++++++
debian/patches/series | 2 +
4 files changed, 206 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index 34fd6cd..72d18f5 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -13,6 +13,9 @@ linux (3.2.84-2) UNRELEASED; urgency=high
* perf: Fix event->ctx locking (CVE-2016-6786, CVE-2016-6787)
* fbdev: color map copying bounds checking (CVE-2016-8405)
* USB: serial: kl5kusb105: fix line-state error handling (CVE-2017-5549)
+ * perf: Do not double free (dependency of the following fix)
+ * perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race
+ (CVE-2017-6001)
-- Salvatore Bonaccorso <carnil at debian.org> Sat, 18 Feb 2017 18:26:58 +0100
diff --git a/debian/patches/bugfix/all/perf-core-fix-concurrent-sys_perf_event_open-vs.-mov.patch b/debian/patches/bugfix/all/perf-core-fix-concurrent-sys_perf_event_open-vs.-mov.patch
new file mode 100644
index 0000000..fd0c3ca
--- /dev/null
+++ b/debian/patches/bugfix/all/perf-core-fix-concurrent-sys_perf_event_open-vs.-mov.patch
@@ -0,0 +1,153 @@
+From: Peter Zijlstra <peterz at infradead.org>
+Date: Wed, 11 Jan 2017 21:09:50 +0100
+Subject: perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race
+Origin: https://git.kernel.org/linus/321027c1fe77f892f4ea07846aeae08cefbbb290
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-6001
+
+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 <joaodias at google.com>
+Signed-off-by: Peter Zijlstra (Intel) <peterz at infradead.org>
+Cc: Alexander Shishkin <alexander.shishkin at linux.intel.com>
+Cc: Arnaldo Carvalho de Melo <acme at kernel.org>
+Cc: Arnaldo Carvalho de Melo <acme at redhat.com>
+Cc: Jiri Olsa <jolsa at redhat.com>
+Cc: Kees Cook <keescook at chromium.org>
+Cc: Linus Torvalds <torvalds at linux-foundation.org>
+Cc: Min Chong <mchong at google.com>
+Cc: Peter Zijlstra <peterz at infradead.org>
+Cc: Stephane Eranian <eranian at google.com>
+Cc: Thomas Gleixner <tglx at linutronix.de>
+Cc: Vince Weaver <vincent.weaver at 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 at kernel.org>
+[bwh: Backported to 3.2:
+ - 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 <ben at decadent.org.uk>
+---
+ kernel/events/core.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++----
+ 1 file changed, 54 insertions(+), 4 deletions(-)
+
+--- a/kernel/events/core.c
++++ b/kernel/events/core.c
+@@ -6479,6 +6479,37 @@ static void mutex_lock_double(struct mut
+ 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
+ *
+@@ -6669,14 +6700,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);
+
+ /*
+@@ -6718,7 +6766,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);
+@@ -6745,6 +6793,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);
diff --git a/debian/patches/bugfix/all/perf-do-not-double-free.patch b/debian/patches/bugfix/all/perf-do-not-double-free.patch
new file mode 100644
index 0000000..f74cb9e
--- /dev/null
+++ b/debian/patches/bugfix/all/perf-do-not-double-free.patch
@@ -0,0 +1,48 @@
+From: Peter Zijlstra <peterz at infradead.org>
+Date: Wed, 24 Feb 2016 18:45:41 +0100
+Subject: perf: Do not double free
+Origin: https:/.git.kernel.org/linus/130056275ade730e7a79c110212c8815202773ee
+
+In case of: err_file: fput(event_file), we'll end up calling
+perf_release() which in turn will free the event.
+
+Do not then free the event _again_.
+
+Tested-by: Alexander Shishkin <alexander.shishkin at linux.intel.com>
+Signed-off-by: Peter Zijlstra (Intel) <peterz at infradead.org>
+Reviewed-by: Alexander Shishkin <alexander.shishkin at linux.intel.com>
+Cc: Arnaldo Carvalho de Melo <acme at redhat.com>
+Cc: Jiri Olsa <jolsa at redhat.com>
+Cc: Linus Torvalds <torvalds at linux-foundation.org>
+Cc: Peter Zijlstra <peterz at infradead.org>
+Cc: Thomas Gleixner <tglx at linutronix.de>
+Cc: dvyukov at google.com
+Cc: eranian at google.com
+Cc: oleg at redhat.com
+Cc: panand at redhat.com
+Cc: sasha.levin at oracle.com
+Cc: vince at deater.net
+Link: http://lkml.kernel.org/r/20160224174947.697350349@infradead.org
+Signed-off-by: Ingo Molnar <mingo at kernel.org>
+[bwh: Backported to 3.2: adjust context]
+Signed-off-by: Ben Hutchings <ben at decadent.org.uk>
+---
+ kernel/events/core.c | 7 ++++++-
+ 1 file changed, 6 insertions(+), 1 deletion(-)
+
+--- a/kernel/events/core.c
++++ b/kernel/events/core.c
+@@ -6749,7 +6749,12 @@ err_context:
+ perf_unpin_context(ctx);
+ put_ctx(ctx);
+ err_alloc:
+- free_event(event);
++ /*
++ * If event_file is set, the fput() above will have called ->release()
++ * and that will take care of freeing the event.
++ */
++ if (!event_file)
++ free_event(event);
+ err_task:
+ if (task)
+ put_task_struct(task);
diff --git a/debian/patches/series b/debian/patches/series
index bc7bf4f..6d5c5c1 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1134,6 +1134,8 @@ bugfix/all/lockdep-silence-warning-if-config_lockdep-isn-t-set.patch
bugfix/all/perf-fix-event-ctx-locking.patch
bugfix/all/fbdev-color-map-copying-bounds-checking.patch
bugfix/all/usb-serial-kl5kusb105-fix-line-state-error-handling.patch
+bugfix/all/perf-do-not-double-free.patch
+bugfix/all/perf-core-fix-concurrent-sys_perf_event_open-vs.-mov.patch
# ABI maintenance
debian/perf-hide-abi-change-in-3.2.30.patch
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/kernel/linux.git
More information about the Kernel-svn-changes
mailing list