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>

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>
Thomas Gleixner
1 parent bd1dbcc67c
Showing 1 changed file with 71 additions and 67 deletions Side-by-side Diff
kernel/futex.c
@@ -792,90 +792,94 @@
  * [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 (pid)
+				return -EINVAL;
 			/*
-			 * 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]
+			 * Take a ref on the state and return success. [4]
 			 */
-			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;
+			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
...	...	@@ -792,90 +792,94 @@
792	792	* [10] There is no transient state which leaves owner and user space
793	793	* TID out of sync.
794	794	*/
795		-static int
796		-lookup_pi_state(u32 uval, struct futex_hash_bucket *hb,
797		- union futex_key key, struct futex_pi_state *ps)
	795	+
	796	+/*
	797	+ * Validate that the existing waiter has a pi_state and sanity check
	798	+ * the pi_state against the user space value. If correct, attach to
	799	+ * it.
	800	+ */
	801	+static int attach_to_pi_state(u32 uval, struct futex_pi_state *pi_state,
	802	+ struct futex_pi_state **ps)
798	803	{
799		- struct futex_q *match = futex_top_waiter(hb, key);
800		- struct futex_pi_state *pi_state = NULL;
801		- struct task_struct *p;
802	804	pid_t pid = uval & FUTEX_TID_MASK;
803	805
804		- if (match) {
805		- /*
806		- * Sanity check the waiter before increasing the
807		- * refcount and attaching to it.
808		- */
809		- pi_state = match->pi_state;
810		- /*
811		- * Userspace might have messed up non-PI and PI
812		- * futexes [3]
813		- */
814		- if (unlikely(!pi_state))
815		- return -EINVAL;
	806	+ /*
	807	+ * Userspace might have messed up non-PI and PI futexes [3]
	808	+ */
	809	+ if (unlikely(!pi_state))
	810	+ return -EINVAL;
816	811
817		- WARN_ON(!atomic_read(&pi_state->refcount));
	812	+ WARN_ON(!atomic_read(&pi_state->refcount));
818	813
	814	+ /*
	815	+ * Handle the owner died case:
	816	+ */
	817	+ if (uval & FUTEX_OWNER_DIED) {
819	818	/*
820		- * Handle the owner died case:
	819	+ * exit_pi_state_list sets owner to NULL and wakes the
	820	+ * topmost waiter. The task which acquires the
	821	+ * pi_state->rt_mutex will fixup owner.
821	822	*/
822		- if (uval & FUTEX_OWNER_DIED) {
	823	+ if (!pi_state->owner) {
823	824	/*
824		- * exit_pi_state_list sets owner to NULL and
825		- * wakes the topmost waiter. The task which
826		- * acquires the pi_state->rt_mutex will fixup
827		- * owner.
	825	+ * No pi state owner, but the user space TID
	826	+ * is not 0. Inconsistent state. [5]
828	827	*/
829		- if (!pi_state->owner) {
830		- /*
831		- * No pi state owner, but the user
832		- * space TID is not 0. Inconsistent
833		- * state. [5]
834		- */
835		- if (pid)
836		- return -EINVAL;
837		- /*
838		- * Take a ref on the state and
839		- * return. [4]
840		- */
841		- goto out_state;
842		- }
843		-
	828	+ if (pid)
	829	+ return -EINVAL;
844	830	/*
845		- * If TID is 0, then either the dying owner
846		- * has not yet executed exit_pi_state_list()
847		- * or some waiter acquired the rtmutex in the
848		- * pi state, but did not yet fixup the TID in
849		- * user space.
850		- *
851		- * Take a ref on the state and return. [6]
	831	+ * Take a ref on the state and return success. [4]
852	832	*/
853		- if (!pid)
854		- goto out_state;
855		- } else {
856		- /*
857		- * If the owner died bit is not set,
858		- * then the pi_state must have an
859		- * owner. [7]
860		- */
861		- if (!pi_state->owner)
862		- return -EINVAL;
	833	+ goto out_state;
863	834	}
864	835
865	836	/*
866		- * Bail out if user space manipulated the
867		- * futex value. If pi state exists then the
868		- * owner TID must be the same as the user
869		- * space TID. [9/10]
	837	+ * If TID is 0, then either the dying owner has not
	838	+ * yet executed exit_pi_state_list() or some waiter
	839	+ * acquired the rtmutex in the pi state, but did not
	840	+ * yet fixup the TID in user space.
	841	+ *
	842	+ * Take a ref on the state and return success. [6]
870	843	*/
871		- if (pid != task_pid_vnr(pi_state->owner))
	844	+ if (!pid)
	845	+ goto out_state;
	846	+ } else {
	847	+ /*
	848	+ * If the owner died bit is not set, then the pi_state
	849	+ * must have an owner. [7]
	850	+ */
	851	+ if (!pi_state->owner)
872	852	return -EINVAL;
873		-
874		- out_state:
875		- atomic_inc(&pi_state->refcount);
876		- *ps = pi_state;
877		- return 0;
878	853	}
	854	+
	855	+ /*
	856	+ * Bail out if user space manipulated the futex value. If pi
	857	+ * state exists then the owner TID must be the same as the
	858	+ * user space TID. [9/10]
	859	+ */
	860	+ if (pid != task_pid_vnr(pi_state->owner))
	861	+ return -EINVAL;
	862	+out_state:
	863	+ atomic_inc(&pi_state->refcount);
	864	+ *ps = pi_state;
	865	+ return 0;
	866	+}
	867	+
	868	+static int
	869	+lookup_pi_state(u32 uval, struct futex_hash_bucket *hb,
	870	+ union futex_key key, struct futex_pi_state *ps)
	871	+{
	872	+ struct futex_q *match = futex_top_waiter(hb, key);
	873	+ struct futex_pi_state *pi_state = NULL;
	874	+ struct task_struct *p;
	875	+ pid_t pid = uval & FUTEX_TID_MASK;
	876	+
	877	+ /*
	878	+ * If there is a waiter on that futex, validate it and
	879	+ * attach to the pi_state when the validation succeeds.
	880	+ */
	881	+ if (match)
	882	+ return attach_to_pi_state(uval, match->pi_state, ps);
879	883
880	884	/*
881	885	* We are the first waiter - try to look up the real owner and attach