Commit 13fbca4c6ecd96ec1a1cfa2e4f2ce191fe928a5e

Authored by Thomas Gleixner
Committed by Linus Torvalds
1 parent b3eaa9fc5c

futex: Always cleanup owner tid in unlock_pi

If the owner died bit is set at futex_unlock_pi, we currently do not
cleanup the user space futex.  So the owner TID of the current owner
(the unlocker) persists.  That's observable inconsistant state,
especially when the ownership of the pi state got transferred.

Clean it up unconditionally.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Kees Cook <keescook@chromium.org>
Cc: Will Drewry <wad@chromium.org>
Cc: Darren Hart <dvhart@linux.intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Showing 1 changed file with 18 additions and 22 deletions Side-by-side Diff

... ... @@ -1052,6 +1052,7 @@
1052 1052 struct task_struct *new_owner;
1053 1053 struct futex_pi_state *pi_state = this->pi_state;
1054 1054 u32 uninitialized_var(curval), newval;
  1055 + int ret = 0;
1055 1056  
1056 1057 if (!pi_state)
1057 1058 return -EINVAL;
1058 1059  
1059 1060  
... ... @@ -1075,23 +1076,19 @@
1075 1076 new_owner = this->task;
1076 1077  
1077 1078 /*
1078   - * We pass it to the next owner. (The WAITERS bit is always
1079   - * kept enabled while there is PI state around. We must also
1080   - * preserve the owner died bit.)
  1079 + * We pass it to the next owner. The WAITERS bit is always
  1080 + * kept enabled while there is PI state around. We cleanup the
  1081 + * owner died bit, because we are the owner.
1081 1082 */
1082   - if (!(uval & FUTEX_OWNER_DIED)) {
1083   - int ret = 0;
  1083 + newval = FUTEX_WAITERS | task_pid_vnr(new_owner);
1084 1084  
1085   - newval = FUTEX_WAITERS | task_pid_vnr(new_owner);
1086   -
1087   - if (cmpxchg_futex_value_locked(&curval, uaddr, uval, newval))
1088   - ret = -EFAULT;
1089   - else if (curval != uval)
1090   - ret = -EINVAL;
1091   - if (ret) {
1092   - raw_spin_unlock(&pi_state->pi_mutex.wait_lock);
1093   - return ret;
1094   - }
  1085 + if (cmpxchg_futex_value_locked(&curval, uaddr, uval, newval))
  1086 + ret = -EFAULT;
  1087 + else if (curval != uval)
  1088 + ret = -EINVAL;
  1089 + if (ret) {
  1090 + raw_spin_unlock(&pi_state->pi_mutex.wait_lock);
  1091 + return ret;
1095 1092 }
1096 1093  
1097 1094 raw_spin_lock_irq(&pi_state->owner->pi_lock);
1098 1095  
... ... @@ -2351,9 +2348,10 @@
2351 2348 /*
2352 2349 * To avoid races, try to do the TID -> 0 atomic transition
2353 2350 * again. If it succeeds then we can return without waking
2354   - * anyone else up:
  2351 + * anyone else up. We only try this if neither the waiters nor
  2352 + * the owner died bit are set.
2355 2353 */
2356   - if (!(uval & FUTEX_OWNER_DIED) &&
  2354 + if (!(uval & ~FUTEX_TID_MASK) &&
2357 2355 cmpxchg_futex_value_locked(&uval, uaddr, vpid, 0))
2358 2356 goto pi_faulted;
2359 2357 /*
... ... @@ -2383,11 +2381,9 @@
2383 2381 /*
2384 2382 * No waiters - kernel unlocks the futex:
2385 2383 */
2386   - if (!(uval & FUTEX_OWNER_DIED)) {
2387   - ret = unlock_futex_pi(uaddr, uval);
2388   - if (ret == -EFAULT)
2389   - goto pi_faulted;
2390   - }
  2384 + ret = unlock_futex_pi(uaddr, uval);
  2385 + if (ret == -EFAULT)
  2386 + goto pi_faulted;
2391 2387  
2392 2388 out_unlock:
2393 2389 spin_unlock(&hb->lock);