linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: tip-bot for Thomas Gleixner <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, hpa@zytor.com, mingo@kernel.org,
	peterz@infradead.org, darren@dvhart.com, kees@outflux.net,
	tglx@linutronix.de, davidlohr@hp.com
Subject: [tip:locking/core] futex: Split out the waiter check from lookup_pi_state()
Date: Sat, 21 Jun 2014 13:33:43 -0700	[thread overview]
Message-ID: <tip-e60cbc5ceaa518d630ab8f35a7d05cee1c752648@git.kernel.org> (raw)
In-Reply-To: <20140611204237.180458410@linutronix.de>

Commit-ID:  e60cbc5ceaa518d630ab8f35a7d05cee1c752648
Gitweb:     http://git.kernel.org/tip/e60cbc5ceaa518d630ab8f35a7d05cee1c752648
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 11 Jun 2014 20:45:39 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sat, 21 Jun 2014 22:26:23 +0200

futex: Split out the waiter check from lookup_pi_state()

We want to be a bit more clever in futex_lock_pi_atomic() and separate
the possible states. Split out the waiter verification into a separate
function. No functional change.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Darren Hart <darren@dvhart.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Davidlohr Bueso <davidlohr@hp.com>
Cc: Kees Cook <kees@outflux.net>
Cc: wad@chromium.org
Link: http://lkml.kernel.org/r/20140611204237.180458410@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/futex.c | 138 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 71 insertions(+), 67 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index fff1ed9..db0c686 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -792,92 +792,96 @@ void exit_pi_state_list(struct task_struct *curr)
  * [10] There is no transient state which leaves owner and user space
  *	TID out of sync.
  */
-static int
-lookup_pi_state(u32 uval, struct futex_hash_bucket *hb,
-		union futex_key *key, struct futex_pi_state **ps)
+
+/*
+ * Validate that the existing waiter has a pi_state and sanity check
+ * the pi_state against the user space value. If correct, attach to
+ * it.
+ */
+static int attach_to_pi_state(u32 uval, struct futex_pi_state *pi_state,
+			      struct futex_pi_state **ps)
 {
-	struct futex_q *match = futex_top_waiter(hb, key);
-	struct futex_pi_state *pi_state = NULL;
-	struct task_struct *p;
 	pid_t pid = uval & FUTEX_TID_MASK;
 
-	if (match) {
-		/*
-		 * Sanity check the waiter before increasing the
-		 * refcount and attaching to it.
-		 */
-		pi_state = match->pi_state;
-		/*
-		 * Userspace might have messed up non-PI and PI
-		 * futexes [3]
-		 */
-		if (unlikely(!pi_state))
-			return -EINVAL;
+	/*
+	 * Userspace might have messed up non-PI and PI futexes [3]
+	 */
+	if (unlikely(!pi_state))
+		return -EINVAL;
 
-		WARN_ON(!atomic_read(&pi_state->refcount));
+	WARN_ON(!atomic_read(&pi_state->refcount));
 
+	/*
+	 * Handle the owner died case:
+	 */
+	if (uval & FUTEX_OWNER_DIED) {
 		/*
-		 * Handle the owner died case:
+		 * exit_pi_state_list sets owner to NULL and wakes the
+		 * topmost waiter. The task which acquires the
+		 * pi_state->rt_mutex will fixup owner.
 		 */
-		if (uval & FUTEX_OWNER_DIED) {
+		if (!pi_state->owner) {
 			/*
-			 * exit_pi_state_list sets owner to NULL and
-			 * wakes the topmost waiter. The task which
-			 * acquires the pi_state->rt_mutex will fixup
-			 * owner.
+			 * No pi state owner, but the user space TID
+			 * is not 0. Inconsistent state. [5]
 			 */
-			if (!pi_state->owner) {
-				/*
-				 * No pi state owner, but the user
-				 * space TID is not 0. Inconsistent
-				 * state. [5]
-				 */
-				if (pid)
-					return -EINVAL;
-				/*
-				 * Take a ref on the state and
-				 * return. [4]
-				 */
-				goto out_state;
-			}
-
-			/*
-			 * If TID is 0, then either the dying owner
-			 * has not yet executed exit_pi_state_list()
-			 * or some waiter acquired the rtmutex in the
-			 * pi state, but did not yet fixup the TID in
-			 * user space.
-			 *
-			 * Take a ref on the state and return. [6]
-			 */
-			if (!pid)
-				goto out_state;
-		} else {
+			if (pid)
+				return -EINVAL;
 			/*
-			 * If the owner died bit is not set,
-			 * then the pi_state must have an
-			 * owner. [7]
+			 * Take a ref on the state and return success. [4]
 			 */
-			if (!pi_state->owner)
-				return -EINVAL;
+			goto out_state;
 		}
 
 		/*
-		 * Bail out if user space manipulated the
-		 * futex value. If pi state exists then the
-		 * owner TID must be the same as the user
-		 * space TID. [9/10]
+		 * If TID is 0, then either the dying owner has not
+		 * yet executed exit_pi_state_list() or some waiter
+		 * acquired the rtmutex in the pi state, but did not
+		 * yet fixup the TID in user space.
+		 *
+		 * Take a ref on the state and return success. [6]
 		 */
-		if (pid != task_pid_vnr(pi_state->owner))
+		if (!pid)
+			goto out_state;
+	} else {
+		/*
+		 * If the owner died bit is not set, then the pi_state
+		 * must have an owner. [7]
+		 */
+		if (!pi_state->owner)
 			return -EINVAL;
-
-	out_state:
-		atomic_inc(&pi_state->refcount);
-		*ps = pi_state;
-		return 0;
 	}
 
 	/*
+	 * Bail out if user space manipulated the futex value. If pi
+	 * state exists then the owner TID must be the same as the
+	 * user space TID. [9/10]
+	 */
+	if (pid != task_pid_vnr(pi_state->owner))
+		return -EINVAL;
+out_state:
+	atomic_inc(&pi_state->refcount);
+	*ps = pi_state;
+	return 0;
+}
+
+static int
+lookup_pi_state(u32 uval, struct futex_hash_bucket *hb,
+		union futex_key *key, struct futex_pi_state **ps)
+{
+	struct futex_q *match = futex_top_waiter(hb, key);
+	struct futex_pi_state *pi_state = NULL;
+	struct task_struct *p;
+	pid_t pid = uval & FUTEX_TID_MASK;
+
+	/*
+	 * If there is a waiter on that futex, validate it and
+	 * attach to the pi_state when the validation succeeds.
+	 */
+	if (match)
+		return attach_to_pi_state(uval, match->pi_state, ps);
+
+	/*
 	 * We are the first waiter - try to look up the real owner and attach
 	 * the new pi_state to it, but bail out when TID = 0 [1]
 	 */

  parent reply	other threads:[~2014-06-21 20:34 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-11 20:45 [patch 0/5] futex: More robustness tweaks Thomas Gleixner
2014-06-11 20:45 ` [patch 1/5] futex: Make unlock_pi more robust Thomas Gleixner
2014-06-16 16:18   ` Darren Hart
2014-06-16 22:15     ` Thomas Gleixner
2014-06-16 22:28       ` Thomas Gleixner
2014-06-16 22:49         ` Darren Hart
2014-06-16 22:39       ` Darren Hart
2014-06-21 20:33   ` [tip:locking/core] " tip-bot for Thomas Gleixner
2014-06-11 20:45 ` [patch 3/5] futex: Split out the waiter check from lookup_pi_state() Thomas Gleixner
2014-06-16 18:12   ` Darren Hart
2014-06-21 20:33   ` tip-bot for Thomas Gleixner [this message]
2014-06-11 20:45 ` [patch 2/5] futex: Use futex_top_waiter() in lookup_pi_state() Thomas Gleixner
2014-06-16 16:51   ` Darren Hart
2014-06-21 20:33   ` [tip:locking/core] " tip-bot for Thomas Gleixner
2014-06-11 20:45 ` [patch 4/5] futex: Split out the first waiter attachment from lookup_pi_state() Thomas Gleixner
2014-06-16 18:19   ` Darren Hart
2014-06-21 20:33   ` [tip:locking/core] " tip-bot for Thomas Gleixner
2014-06-11 20:45 ` [patch 5/5] futex: Simplify futex_lock_pi_atomic() and make it more robust Thomas Gleixner
2014-06-13  5:46   ` Darren Hart
2014-06-13  8:34     ` Thomas Gleixner
2014-06-13  9:36       ` Thomas Gleixner
2014-06-13  9:44         ` [patch V2 " Thomas Gleixner
2014-06-13 20:51           ` Davidlohr Bueso
2014-06-16 20:36           ` Darren Hart
2014-06-17  7:20             ` Thomas Gleixner
2014-06-21 20:34           ` [tip:locking/core] " tip-bot for Thomas Gleixner

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-e60cbc5ceaa518d630ab8f35a7d05cee1c752648@git.kernel.org \
    --to=tipbot@zytor.com \
    --cc=darren@dvhart.com \
    --cc=davidlohr@hp.com \
    --cc=hpa@zytor.com \
    --cc=kees@outflux.net \
    --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 \
    /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).