stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, John Dias <joaodias@google.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Jiri Olsa <jolsa@redhat.com>, Kees Cook <keescook@chromium.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Min Chong <mchong@google.com>,
	Stephane Eranian <eranian@google.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vince Weaver <vincent.weaver@maine.edu>,
	Ingo Molnar <mingo@kernel.org>,
	Ben Hutchings <ben@decadent.org.uk>,
	Suren Baghdasaryan <surenb@google.com>,
	Amit Pundir <amit.pundir@linaro.org>
Subject: [PATCH 3.18 16/46] perf/core: Fix concurrent sys_perf_event_open() vs. move_group race
Date: Mon, 15 Jan 2018 13:33:24 +0100	[thread overview]
Message-ID: <20180115123329.237393846@linuxfoundation.org> (raw)
In-Reply-To: <20180115123327.303455538@linuxfoundation.org>

3.18-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Peter Zijlstra <peterz@infradead.org>

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 <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>
[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 <ben@decadent.org.uk>
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
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(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7414,6 +7414,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
  *
@@ -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);

  parent reply	other threads:[~2018-01-15 12:36 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-15 12:33 [PATCH 3.18 00/46] 3.18.92-stable review Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 01/46] kernel/acct.c: fix the acct->needcheck check in check_free_space() Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 02/46] crypto: n2 - cure use after free Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 03/46] fscache: Fix the default for fscache_maybe_release_page() Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 04/46] kernel/signal.c: protect the traced SIGNAL_UNKILLABLE tasks from SIGKILL Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 05/46] kernel/signal.c: protect the SIGNAL_UNKILLABLE tasks from !sig_kernel_only() signals Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 06/46] kernel/signal.c: remove the no longer needed SIGNAL_UNKILLABLE check in complete_signal() Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 07/46] Input: elantech - add new icbody type 15 Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 08/46] can: gs_usb: fix return value of the "set_bittiming" callback Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 09/46] IB/srpt: Disable RDMA access by the initiator Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 10/46] MIPS: Factor out NT_PRFPREG regset access helpers Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 11/46] MIPS: Guard against any partial write attempt with PTRACE_SETREGSET Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 12/46] MIPS: Consistently handle buffer counter " Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 13/46] MIPS: Fix an FCSR access API regression with NT_PRFPREG and MSA Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 14/46] MIPS: Disallow outsized PTRACE_SETREGSET NT_PRFPREG regset accesses Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 15/46] MIPS: Also verify sizeof `elf_fpreg_t with PTRACE_SETREGSET Greg Kroah-Hartman
2018-01-15 12:33 ` Greg Kroah-Hartman [this message]
2018-01-15 12:33 ` [PATCH 3.18 17/46] kvm: vmx: Scrub hardware GPRs at VM-exit Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 18/46] x86/acpi: Handle SCI interrupts above legacy space gracefully Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 19/46] ALSA: pcm: Remove incorrect snd_BUG_ON() usages Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 20/46] ALSA: pcm: Add missing error checks in OSS emulation plugin builder Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 21/46] ALSA: pcm: Abort properly at pending signal in OSS read/write loops Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 22/46] ALSA: pcm: Allow aborting mutex lock at " Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 23/46] ALSA: aloop: Release cable upon open error path Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 24/46] ALSA: aloop: Fix inconsistent format due to incomplete rule Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 25/46] ALSA: aloop: Fix racy hw constraints adjustment Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 26/46] x86/acpi: Reduce code duplication in mp_override_legacy_irq() Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 27/46] 8021q: fix a memory leak for VLAN 0 device Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 28/46] RDS: Heap OOB write in rds_message_alloc_sgs() Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 29/46] RDS: null pointer dereference in rds_atomic_free_op Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 30/46] sh_eth: fix TSU resource handling Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 31/46] sh_eth: fix SH7757 GEther initialization Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 32/46] net: stmmac: enable EEE in MII, GMII or RGMII only Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 33/46] crypto: algapi - fix NULL dereference in crypto_remove_spawns() Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 34/46] x86/microcode/intel: Extend BDW late-loading with a revision check Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 35/46] iscsi-target: Make TASK_REASSIGN use proper se_cmd->cmd_kref Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 36/46] target: Avoid early CMD_T_PRE_EXECUTE failures during ABORT_TASK Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 37/46] [PATCH] Revert "can: kvaser_usb: free buf in error paths" Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 39/46] USB: serial: cp210x: add new device ID ELV ALC 8xxx Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 40/46] usb: misc: usb3503: make sure reset is low for at least 100us Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 41/46] USB: fix usbmon BUG trigger Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 42/46] usbip: remove kernel addresses from usb device and urb debug msgs Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 43/46] staging: android: ashmem: fix a race condition in ASHMEM_SET_SIZE ioctl Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 44/46] Bluetooth: Prevent stack info leak from the EFS element Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 45/46] uas: ignore UAS for Norelsys NS1068(X) chips Greg Kroah-Hartman
2018-01-15 12:33 ` [PATCH 3.18 46/46] e1000e: Fix e1000_check_for_copper_link_ich8lan return value Greg Kroah-Hartman
2018-01-16 14:28 ` [PATCH 3.18 00/46] 3.18.92-stable review Guenter Roeck
2018-01-16 20:31 ` Shuah Khan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180115123329.237393846@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=amit.pundir@linaro.org \
    --cc=ben@decadent.org.uk \
    --cc=eranian@google.com \
    --cc=joaodias@google.com \
    --cc=jolsa@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchong@google.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=vincent.weaver@maine.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).