| From 30a6b8031fe14031ab27c1fa3483cb9780e7f63c Mon Sep 17 00:00:00 2001 |
| From: Brian Silverman <bsilver16384@gmail.com> |
| Date: Sat, 25 Oct 2014 20:20:37 -0400 |
| Subject: futex: Fix a race condition between REQUEUE_PI and task death |
| |
| From: Brian Silverman <bsilver16384@gmail.com> |
| |
| 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 <bsilver16384@gmail.com> |
| Cc: austin.linux@gmail.com |
| Cc: darren@dvhart.com |
| Cc: peterz@infradead.org |
| Link: http://lkml.kernel.org/r/1414282837-23092-1-git-send-email-bsilver16384@gmail.com |
| Signed-off-by: Thomas Gleixner <tglx@linutronix.de> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| kernel/futex.c | 22 +++++++++++----------- |
| 1 file changed, 11 insertions(+), 11 deletions(-) |
| |
| --- a/kernel/futex.c |
| +++ b/kernel/futex.c |
| @@ -641,8 +641,14 @@ static struct futex_pi_state * alloc_pi_ |
| 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; |
| |
| @@ -1521,15 +1527,6 @@ static int futex_requeue(u32 __user *uad |
| } |
| |
| 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; |
| @@ -1619,6 +1616,8 @@ retry_private: |
| case 0: |
| break; |
| case -EFAULT: |
| + free_pi_state(pi_state); |
| + pi_state = NULL; |
| double_unlock_hb(hb1, hb2); |
| hb_waiters_dec(hb2); |
| put_futex_key(&key2); |
| @@ -1634,6 +1633,8 @@ retry_private: |
| * exit to complete. |
| * - The user space value changed. |
| */ |
| + free_pi_state(pi_state); |
| + pi_state = NULL; |
| double_unlock_hb(hb1, hb2); |
| hb_waiters_dec(hb2); |
| put_futex_key(&key2); |
| @@ -1710,6 +1711,7 @@ retry_private: |
| } |
| |
| out_unlock: |
| + free_pi_state(pi_state); |
| double_unlock_hb(hb1, hb2); |
| hb_waiters_dec(hb2); |
| |
| @@ -1727,8 +1729,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; |
| } |
| |