aboutsummaryrefslogtreecommitdiffstats
path: root/kernel/futex.c
diff options
context:
space:
mode:
authorjohn stultz <johnstul@us.ibm.com>2006-08-14 02:24:24 -0400
committerGreg Kroah-Hartman <gregkh@suse.de>2006-08-14 15:54:29 -0400
commite579dcbf23604cb33c08b5c3c3ac06ca36e7c683 (patch)
tree293581684fc47260a4610e4a77510b459346e2aa /kernel/futex.c
parent6997a6faaa129a1c91775f7344c8d371a05178ea (diff)
[PATCH] futex_handle_fault always fails
We found this issue last week w/ the -RT kernel, but it seems the same issue is in mainline as well. Basically it is possible for futex_unlock_pi to return without actually freeing the lock. This is due to buggy logic in the use of futex_handle_fault() and its attempt argument in a failure case. Looking at futex.c the logic is as follows: 1) In futex_unlock_pi() we start w/ ret=0 and we go down to the first futex_atomic_cmpxchg_inatomic(), where we find uval==-EFAULT. We then jump to the pi_faulted label. 2) From pi_faulted: We increment attempt, unlock the sem and hit the retry label. 3) From the retry label, with ret still zero, we again hit EFAULT on the first futex_atomic_cmpxchg_inatomic(), and again goto the pi_faulted label. 4) Again from pi_faulted: we increment attempt and enter the conditional, where we call futex_handle_fault. 5) futex_handle_fault fails, and we goto the out_unlock_release_sem label. 6) From out_unlock_release_sem we return, and since ret is still zero, we return without error, while never actually unlocking the lock. Issue #1: at the first futex_atomic_cmpxchg_inatomic() we should probably be setting ret=-EFAULT before jumping to pi_faulted: However in our case this doesn't really affect anything, as the glibc we're using ignores the error value from futex_unlock_pi(). Issue #2: Look at futex_handle_fault(), its first conditional will return -EFAULT if attempt is >= 2. However, from the "if(attempt++) futex_handle_fault(attempt)" logic above, we'll *never* call futex_handle_fault when attempt is less then two. So we never get a chance to even try to fault the page in. The following patch addresses these two issues by 1) Always setting ret to -EFAULT if futex_handle_fault fails, and 2) Removing the = in futex_handle_fault's (attempt >= 2) check. I'm really not sure this is the right fix, but wanted to bring it up so folks knew the issue is alive and well in the current -git tree. From looking at the git logs the logic was first introduced (then later copied to other places) in the following commit almost a year ago: http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=4732efbeb997189d9f9b04708dc26bf8613ed721;hp=5b039e681b8c5f30aac9cc04385cc94be45d0823 Cc: Rusty Russell <rusty@rustcorp.com.au> Cc: Ingo Molnar <mingo@elte.hu> Acked-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Diffstat (limited to 'kernel/futex.c')
-rw-r--r--kernel/futex.c16
1 files changed, 10 insertions, 6 deletions
diff --git a/kernel/futex.c b/kernel/futex.c
index c2b2e0b83abf..d4633c588f33 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -297,7 +297,7 @@ static int futex_handle_fault(unsigned long address, int attempt)
297 struct vm_area_struct * vma; 297 struct vm_area_struct * vma;
298 struct mm_struct *mm = current->mm; 298 struct mm_struct *mm = current->mm;
299 299
300 if (attempt >= 2 || !(vma = find_vma(mm, address)) || 300 if (attempt > 2 || !(vma = find_vma(mm, address)) ||
301 vma->vm_start > address || !(vma->vm_flags & VM_WRITE)) 301 vma->vm_start > address || !(vma->vm_flags & VM_WRITE))
302 return -EFAULT; 302 return -EFAULT;
303 303
@@ -747,8 +747,10 @@ retry:
747 */ 747 */
748 if (attempt++) { 748 if (attempt++) {
749 if (futex_handle_fault((unsigned long)uaddr2, 749 if (futex_handle_fault((unsigned long)uaddr2,
750 attempt)) 750 attempt)) {
751 ret = -EFAULT;
751 goto out; 752 goto out;
753 }
752 goto retry; 754 goto retry;
753 } 755 }
754 756
@@ -1322,9 +1324,10 @@ static int do_futex_lock_pi(u32 __user *uaddr, int detect, int trylock,
1322 * still holding the mmap_sem. 1324 * still holding the mmap_sem.
1323 */ 1325 */
1324 if (attempt++) { 1326 if (attempt++) {
1325 if (futex_handle_fault((unsigned long)uaddr, attempt)) 1327 if (futex_handle_fault((unsigned long)uaddr, attempt)) {
1328 ret = -EFAULT;
1326 goto out_unlock_release_sem; 1329 goto out_unlock_release_sem;
1327 1330 }
1328 goto retry_locked; 1331 goto retry_locked;
1329 } 1332 }
1330 1333
@@ -1506,9 +1509,10 @@ pi_faulted:
1506 * still holding the mmap_sem. 1509 * still holding the mmap_sem.
1507 */ 1510 */
1508 if (attempt++) { 1511 if (attempt++) {
1509 if (futex_handle_fault((unsigned long)uaddr, attempt)) 1512 if (futex_handle_fault((unsigned long)uaddr, attempt)) {
1513 ret = -EFAULT;
1510 goto out_unlock; 1514 goto out_unlock;
1511 1515 }
1512 goto retry_locked; 1516 goto retry_locked;
1513 } 1517 }
1514 1518