| [2557] | 1 | From 9ad5dabd87e8dd5506529e12e4e8c7b25fb88d7a Mon Sep 17 00:00:00 2001 | 
|---|
|  | 2 | From: Thomas Gleixner <tglx@linutronix.de> | 
|---|
|  | 3 | Date: Tue, 3 Jun 2014 12:27:07 +0000 | 
|---|
|  | 4 | Subject: [PATCH 3/4] futex: Always cleanup owner tid in unlock_pi | 
|---|
|  | 5 |  | 
|---|
|  | 6 | commit 13fbca4c6ecd96ec1a1cfa2e4f2ce191fe928a5e upstream. | 
|---|
|  | 7 |  | 
|---|
|  | 8 | If the owner died bit is set at futex_unlock_pi, we currently do not | 
|---|
|  | 9 | cleanup the user space futex.  So the owner TID of the current owner | 
|---|
|  | 10 | (the unlocker) persists.  That's observable inconsistant state, | 
|---|
|  | 11 | especially when the ownership of the pi state got transferred. | 
|---|
|  | 12 |  | 
|---|
|  | 13 | Clean it up unconditionally. | 
|---|
|  | 14 |  | 
|---|
|  | 15 | Signed-off-by: Thomas Gleixner <tglx@linutronix.de> | 
|---|
|  | 16 | Cc: Kees Cook <keescook@chromium.org> | 
|---|
|  | 17 | Cc: Will Drewry <wad@chromium.org> | 
|---|
|  | 18 | Cc: Darren Hart <dvhart@linux.intel.com> | 
|---|
|  | 19 | Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> | 
|---|
|  | 20 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 
|---|
|  | 21 | --- | 
|---|
|  | 22 | kernel/futex.c |   40 ++++++++++++++++++---------------------- | 
|---|
|  | 23 | 1 file changed, 18 insertions(+), 22 deletions(-) | 
|---|
|  | 24 |  | 
|---|
|  | 25 | diff --git a/kernel/futex.c b/kernel/futex.c | 
|---|
|  | 26 | index 8c1e6d0..9720c42 100644 | 
|---|
|  | 27 | --- a/kernel/futex.c | 
|---|
|  | 28 | +++ b/kernel/futex.c | 
|---|
|  | 29 | @@ -903,6 +903,7 @@ static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *this) | 
|---|
|  | 30 | struct task_struct *new_owner; | 
|---|
|  | 31 | struct futex_pi_state *pi_state = this->pi_state; | 
|---|
|  | 32 | u32 uninitialized_var(curval), newval; | 
|---|
|  | 33 | +       int ret = 0; | 
|---|
|  | 34 |  | 
|---|
|  | 35 | if (!pi_state) | 
|---|
|  | 36 | return -EINVAL; | 
|---|
|  | 37 | @@ -926,23 +927,19 @@ static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *this) | 
|---|
|  | 38 | new_owner = this->task; | 
|---|
|  | 39 |  | 
|---|
|  | 40 | /* | 
|---|
|  | 41 | -        * We pass it to the next owner. (The WAITERS bit is always | 
|---|
|  | 42 | -        * kept enabled while there is PI state around. We must also | 
|---|
|  | 43 | -        * preserve the owner died bit.) | 
|---|
|  | 44 | +        * We pass it to the next owner. The WAITERS bit is always | 
|---|
|  | 45 | +        * kept enabled while there is PI state around. We cleanup the | 
|---|
|  | 46 | +        * owner died bit, because we are the owner. | 
|---|
|  | 47 | */ | 
|---|
|  | 48 | -       if (!(uval & FUTEX_OWNER_DIED)) { | 
|---|
|  | 49 | -               int ret = 0; | 
|---|
|  | 50 | - | 
|---|
|  | 51 | -               newval = FUTEX_WAITERS | task_pid_vnr(new_owner); | 
|---|
|  | 52 | +       newval = FUTEX_WAITERS | task_pid_vnr(new_owner); | 
|---|
|  | 53 |  | 
|---|
|  | 54 | -               if (cmpxchg_futex_value_locked(&curval, uaddr, uval, newval)) | 
|---|
|  | 55 | -                       ret = -EFAULT; | 
|---|
|  | 56 | -               else if (curval != uval) | 
|---|
|  | 57 | -                       ret = -EINVAL; | 
|---|
|  | 58 | -               if (ret) { | 
|---|
|  | 59 | -                       raw_spin_unlock(&pi_state->pi_mutex.wait_lock); | 
|---|
|  | 60 | -                       return ret; | 
|---|
|  | 61 | -               } | 
|---|
|  | 62 | +       if (cmpxchg_futex_value_locked(&curval, uaddr, uval, newval)) | 
|---|
|  | 63 | +               ret = -EFAULT; | 
|---|
|  | 64 | +       else if (curval != uval) | 
|---|
|  | 65 | +               ret = -EINVAL; | 
|---|
|  | 66 | +       if (ret) { | 
|---|
|  | 67 | +               raw_spin_unlock(&pi_state->pi_mutex.wait_lock); | 
|---|
|  | 68 | +               return ret; | 
|---|
|  | 69 | } | 
|---|
|  | 70 |  | 
|---|
|  | 71 | raw_spin_lock_irq(&pi_state->owner->pi_lock); | 
|---|
|  | 72 | @@ -2187,9 +2184,10 @@ retry: | 
|---|
|  | 73 | /* | 
|---|
|  | 74 | * To avoid races, try to do the TID -> 0 atomic transition | 
|---|
|  | 75 | * again. If it succeeds then we can return without waking | 
|---|
|  | 76 | -        * anyone else up: | 
|---|
|  | 77 | +        * anyone else up. We only try this if neither the waiters nor | 
|---|
|  | 78 | +        * the owner died bit are set. | 
|---|
|  | 79 | */ | 
|---|
|  | 80 | -       if (!(uval & FUTEX_OWNER_DIED) && | 
|---|
|  | 81 | +       if (!(uval & ~FUTEX_TID_MASK) && | 
|---|
|  | 82 | cmpxchg_futex_value_locked(&uval, uaddr, vpid, 0)) | 
|---|
|  | 83 | goto pi_faulted; | 
|---|
|  | 84 | /* | 
|---|
|  | 85 | @@ -2221,11 +2219,9 @@ retry: | 
|---|
|  | 86 | /* | 
|---|
|  | 87 | * No waiters - kernel unlocks the futex: | 
|---|
|  | 88 | */ | 
|---|
|  | 89 | -       if (!(uval & FUTEX_OWNER_DIED)) { | 
|---|
|  | 90 | -               ret = unlock_futex_pi(uaddr, uval); | 
|---|
|  | 91 | -               if (ret == -EFAULT) | 
|---|
|  | 92 | -                       goto pi_faulted; | 
|---|
|  | 93 | -       } | 
|---|
|  | 94 | +       ret = unlock_futex_pi(uaddr, uval); | 
|---|
|  | 95 | +       if (ret == -EFAULT) | 
|---|
|  | 96 | +               goto pi_faulted; | 
|---|
|  | 97 |  | 
|---|
|  | 98 | out_unlock: | 
|---|
|  | 99 | spin_unlock(&hb->lock); | 
|---|
|  | 100 | -- | 
|---|
|  | 101 | 1.7.10.4 | 
|---|
|  | 102 |  | 
|---|