All of lore.kernel.org
 help / color / mirror / Atom feed
From: tip-bot for Peter Zijlstra <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: mingo@kernel.org, peterz@infradead.org, tglx@linutronix.de,
	hpa@zytor.com, acme@redhat.com, torvalds@linux-foundation.org,
	alexander.shishkin@linux.intel.com, linux-kernel@vger.kernel.org,
	jolsa@redhat.com
Subject: [tip:perf/urgent] perf: Close install vs. exit race
Date: Thu, 25 Feb 2016 00:03:44 -0800	[thread overview]
Message-ID: <tip-84c4e620d35f49f486a900af214ad12276afb386@git.kernel.org> (raw)
In-Reply-To: <20160224174947.626853419@infradead.org>

Commit-ID:  84c4e620d35f49f486a900af214ad12276afb386
Gitweb:     http://git.kernel.org/tip/84c4e620d35f49f486a900af214ad12276afb386
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 24 Feb 2016 18:45:40 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 25 Feb 2016 08:42:32 +0100

perf: Close install vs. exit race

Consider the following scenario:

  CPU0					CPU1

  ctx = find_get_ctx();
					perf_event_exit_task_context()
  mutex_lock(&ctx->mutex);
  perf_install_in_context(ctx, ...);
    /* NO-OP */
  mutex_unlock(&ctx->mutex);

  ...

  perf_release()
    WARN_ON_ONCE(event->state != STATE_EXIT);

Since the event doesn't pass through perf_remove_from_context()
because perf_install_in_context() NO-OPs because the ctx is dead, and
perf_event_exit_task_context() will not observe the event because its
not attached yet, the event->state will not be set.

Solve this by revalidating ctx->task after we acquire ctx->mutex and
failing the event creation as a whole.

Tested-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: dvyukov@google.com
Cc: eranian@google.com
Cc: oleg@redhat.com
Cc: panand@redhat.com
Cc: sasha.levin@oracle.com
Cc: vince@deater.net
Link: http://lkml.kernel.org/r/20160224174947.626853419@infradead.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/core.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 0d58522..d7b0316 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2158,13 +2158,15 @@ perf_install_in_context(struct perf_event_context *ctx,
 	 */
 	raw_spin_lock_irq(&ctx->lock);
 	task = ctx->task;
+
 	/*
-	 * Worse, we cannot even rely on the ctx actually existing anymore. If
-	 * between find_get_context() and perf_install_in_context() the task
-	 * went through perf_event_exit_task() its dead and we should not be
-	 * adding new events.
+	 * If between ctx = find_get_context() and mutex_lock(&ctx->mutex) the
+	 * ctx gets destroyed, we must not install an event into it.
+	 *
+	 * This is normally tested for after we acquire the mutex, so this is
+	 * a sanity check.
 	 */
-	if (task == TASK_TOMBSTONE) {
+	if (WARN_ON_ONCE(task == TASK_TOMBSTONE)) {
 		raw_spin_unlock_irq(&ctx->lock);
 		return;
 	}
@@ -8389,10 +8391,19 @@ SYSCALL_DEFINE5(perf_event_open,
 	if (move_group) {
 		gctx = group_leader->ctx;
 		mutex_lock_double(&gctx->mutex, &ctx->mutex);
+		if (gctx->task == TASK_TOMBSTONE) {
+			err = -ESRCH;
+			goto err_locked;
+		}
 	} else {
 		mutex_lock(&ctx->mutex);
 	}
 
+	if (ctx->task == TASK_TOMBSTONE) {
+		err = -ESRCH;
+		goto err_locked;
+	}
+
 	if (!perf_event_validate_size(event)) {
 		err = -E2BIG;
 		goto err_locked;
@@ -8563,12 +8574,14 @@ perf_event_create_kernel_counter(struct perf_event_attr *attr, int cpu,
 
 	WARN_ON_ONCE(ctx->parent_ctx);
 	mutex_lock(&ctx->mutex);
+	if (ctx->task == TASK_TOMBSTONE) {
+		err = -ESRCH;
+		goto err_unlock;
+	}
+
 	if (!exclusive_event_installable(event, ctx)) {
-		mutex_unlock(&ctx->mutex);
-		perf_unpin_context(ctx);
-		put_ctx(ctx);
 		err = -EBUSY;
-		goto err_free;
+		goto err_unlock;
 	}
 
 	perf_install_in_context(ctx, event, cpu);
@@ -8577,6 +8590,10 @@ perf_event_create_kernel_counter(struct perf_event_attr *attr, int cpu,
 
 	return event;
 
+err_unlock:
+	mutex_unlock(&ctx->mutex);
+	perf_unpin_context(ctx);
+	put_ctx(ctx);
 err_free:
 	free_event(event);
 err:

  reply	other threads:[~2016-02-25  8:04 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-24 17:45 [PATCH 00/12] perf: more fixes Peter Zijlstra
2016-02-24 17:45 ` [PATCH 01/12] perf: Close install vs exit race Peter Zijlstra
2016-02-25  8:03   ` tip-bot for Peter Zijlstra [this message]
2016-02-24 17:45 ` [PATCH 02/12] perf: Do not double free Peter Zijlstra
2016-02-25  8:04   ` [tip:perf/urgent] " tip-bot for Peter Zijlstra
2016-02-24 17:45 ` [PATCH 03/12] perf: Allow perf_release() with !event->ctx Peter Zijlstra
2016-02-25  8:04   ` [tip:perf/urgent] " tip-bot for Peter Zijlstra
2016-02-24 17:45 ` [PATCH 04/12] perf: Only update context time when active Peter Zijlstra
2016-02-25  8:04   ` [tip:perf/urgent] " tip-bot for Peter Zijlstra
2016-02-24 17:45 ` [PATCH 05/12] perf: Fix cloning Peter Zijlstra
2016-02-25  8:05   ` [tip:perf/urgent] " tip-bot for Peter Zijlstra
2016-02-24 17:45 ` [PATCH 06/12] perf: Fix race between event install and jump_labels Peter Zijlstra
2016-02-25  8:05   ` [tip:perf/urgent] " tip-bot for Peter Zijlstra
2016-02-24 17:45 ` [PATCH 07/12] perf: Cure event->pending_disable race Peter Zijlstra
2016-02-25  8:06   ` [tip:perf/urgent] " tip-bot for Peter Zijlstra
2016-02-24 17:45 ` [PATCH 08/12] perf: Introduce EVENT_TIME Peter Zijlstra
2016-02-25  8:06   ` [tip:perf/urgent] perf: Fix ctx time tracking by introducing EVENT_TIME tip-bot for Peter Zijlstra
2016-02-24 17:45 ` [PATCH 09/12] perf: Fix scaling vs enable_on_exec Peter Zijlstra
2016-02-25  8:07   ` [tip:perf/urgent] perf: Fix scaling vs. perf_event_enable_on_exec() tip-bot for Peter Zijlstra
2016-02-24 17:45 ` [PATCH 10/12] perf: Fix scaling vs perf_event_enable Peter Zijlstra
2016-02-25  8:07   ` [tip:perf/urgent] perf: Fix scaling vs. perf_event_enable() tip-bot for Peter Zijlstra
2016-02-24 17:45 ` [PATCH 11/12] perf: Fix scaling vs perf_install_in_context Peter Zijlstra
2016-02-25  8:07   ` [tip:perf/urgent] perf: Fix scaling vs. perf_install_in_context() tip-bot for Peter Zijlstra
2016-02-24 17:45 ` [PATCH 12/12] perf: Robustify task_function_call() Peter Zijlstra
2016-02-25  8:08   ` [tip:perf/urgent] " tip-bot for Peter Zijlstra
2016-03-10 14:39 ` [PATCH 00/12] perf: more fixes Peter Zijlstra
2016-03-10 14:44   ` Vince Weaver
2016-03-11 14:23     ` Peter Zijlstra
2016-03-11 15:41       ` Borislav Petkov
2016-03-21  9:49       ` [tip:perf/urgent] perf/x86/ibs: Fix IBS throttle tip-bot for Peter Zijlstra
2016-03-15 15:38     ` [PATCH 00/12] perf: more fixes Peter Zijlstra
2016-03-16 22:59       ` Peter Zijlstra
2016-03-16 23:10         ` Peter Zijlstra
2016-03-17 11:54         ` Borislav Petkov
2016-03-11 10:12   ` Alexander Shishkin
2016-03-21  9:48   ` [tip:perf/urgent] perf/core: Fix the unthrottle logic tip-bot for Peter Zijlstra

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=tip-84c4e620d35f49f486a900af214ad12276afb386@git.kernel.org \
    --to=tipbot@zytor.com \
    --cc=acme@redhat.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /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.