From: Brian Silverman <[email protected]>

3.12-stable review patch.  If anyone has any objections, please let me know.

===============

commit 30a6b8031fe14031ab27c1fa3483cb9780e7f63c upstream.

free_pi_state and exit_pi_state_list both clean up futex_pi_state's.
exit_pi_state_list takes the hb lock first, and most callers of
free_pi_state do too. requeue_pi doesn't, which means free_pi_state
can free the pi_state out from under exit_pi_state_list. For example:

task A                            |  task B
exit_pi_state_list                |
  pi_state =                      |
      curr->pi_state_list->next   |
                                  |  futex_requeue(requeue_pi=1)
                                  |    // pi_state is the same as
                                  |    // the one in task A
                                  |    free_pi_state(pi_state)
                                  |      list_del_init(&pi_state->list)
                                  |      kfree(pi_state)
  list_del_init(&pi_state->list)  |

Move the free_pi_state calls in requeue_pi to before it drops the hb
locks which it's already holding.

[ tglx: Removed a pointless free_pi_state() call and the hb->lock held
        debugging. The latter comes via a seperate patch ]

Signed-off-by: Brian Silverman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Mike Galbraith <[email protected]>
Link: 
http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
---
 kernel/futex.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index e4b9b60e25b1..bd0bc06772f6 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -490,8 +490,14 @@ static struct futex_pi_state * alloc_pi_state(void)
        return pi_state;
 }
 
+/*
+ * Must be called with the hb lock held.
+ */
 static void free_pi_state(struct futex_pi_state *pi_state)
 {
+       if (!pi_state)
+               return;
+
        if (!atomic_dec_and_test(&pi_state->refcount))
                return;
 
@@ -1405,15 +1411,6 @@ static int futex_requeue(u32 __user *uaddr1, unsigned 
int flags,
        }
 
 retry:
-       if (pi_state != NULL) {
-               /*
-                * We will have to lookup the pi_state again, so free this one
-                * to keep the accounting correct.
-                */
-               free_pi_state(pi_state);
-               pi_state = NULL;
-       }
-
        ret = get_futex_key(uaddr1, flags & FLAGS_SHARED, &key1, VERIFY_READ);
        if (unlikely(ret != 0))
                goto out;
@@ -1501,6 +1498,8 @@ retry_private:
                case 0:
                        break;
                case -EFAULT:
+                       free_pi_state(pi_state);
+                       pi_state = NULL;
                        double_unlock_hb(hb1, hb2);
                        put_futex_key(&key2);
                        put_futex_key(&key1);
@@ -1510,6 +1509,8 @@ retry_private:
                        goto out;
                case -EAGAIN:
                        /* The owner was exiting, try again. */
+                       free_pi_state(pi_state);
+                       pi_state = NULL;
                        double_unlock_hb(hb1, hb2);
                        put_futex_key(&key2);
                        put_futex_key(&key1);
@@ -1586,6 +1587,7 @@ retry_private:
        }
 
 out_unlock:
+       free_pi_state(pi_state);
        double_unlock_hb(hb1, hb2);
 
        /*
@@ -1602,8 +1604,6 @@ out_put_keys:
 out_put_key1:
        put_futex_key(&key1);
 out:
-       if (pi_state != NULL)
-               free_pi_state(pi_state);
        return ret ? ret : task_count;
 }
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to