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);
next prev parent reply other threads:[~2018-01-15 14:34 UTC|newest]
Thread overview: 50+ 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 38/46] USB: serial: cp210x: add IDs for LifeScan OneTouch Verio IQ 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-15 16:28 ` [PATCH 3.18 00/46] 3.18.92-stable review kernelci.org bot
2018-01-16 14:28 ` 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 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.