The patch titled
futex_handle_fault always fails
has been removed from the -mm tree. Its filename is
futex_handle_fault-always-fails.patch
This patch was dropped because it was merged into mainline or a subsystem tree
------------------------------------------------------
Subject: futex_handle_fault always fails
From: john stultz <johnstul@xxxxxxxxxx>
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@xxxxxxxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxx>
Acked-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
---
kernel/futex.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff -puN kernel/futex.c~futex_handle_fault-always-fails kernel/futex.c
--- a/kernel/futex.c~futex_handle_fault-always-fails
+++ a/kernel/futex.c
@@ -297,7 +297,7 @@ static int futex_handle_fault(unsigned l
struct vm_area_struct * vma;
struct mm_struct *mm = current->mm;
- if (attempt >= 2 || !(vma = find_vma(mm, address)) ||
+ if (attempt > 2 || !(vma = find_vma(mm, address)) ||
vma->vm_start > address || !(vma->vm_flags & VM_WRITE))
return -EFAULT;
@@ -747,8 +747,10 @@ retry:
*/
if (attempt++) {
if (futex_handle_fault((unsigned long)uaddr2,
- attempt))
+ attempt)) {
+ ret = -EFAULT;
goto out;
+ }
goto retry;
}
@@ -1322,9 +1324,10 @@ static int do_futex_lock_pi(u32 __user *
* still holding the mmap_sem.
*/
if (attempt++) {
- if (futex_handle_fault((unsigned long)uaddr, attempt))
+ if (futex_handle_fault((unsigned long)uaddr, attempt)) {
+ ret = -EFAULT;
goto out_unlock_release_sem;
-
+ }
goto retry_locked;
}
@@ -1506,9 +1509,10 @@ pi_faulted:
* still holding the mmap_sem.
*/
if (attempt++) {
- if (futex_handle_fault((unsigned long)uaddr, attempt))
+ if (futex_handle_fault((unsigned long)uaddr, attempt)) {
+ ret = -EFAULT;
goto out_unlock;
-
+ }
goto retry_locked;
}
_
Patches currently in -mm which might be from johnstul@xxxxxxxxxx are
origin.patch
avr32-switch-to-generic-timekeeping-framework.patch
i386-kill-references-to-xtime.patch
i386-detect-clock-skew-during-suspend.patch
simplify-update_times-avoid-jiffies-jiffies_64-aliasing-problem-2.patch
ntp-move-all-the-ntp-related-code-to-ntpc.patch
ntp-move-all-the-ntp-related-code-to-ntpc-fix.patch
ntp-add-ntp_update_frequency.patch
ntp-add-time_adj-to-tick-length.patch
ntp-add-time_freq-to-tick-length.patch
ntp-prescale-time_offset.patch
ntp-add-time_adjust-to-tick-length.patch
ntp-remove-time_tolerance.patch
ntp-convert-time_freq-to-nsec-value.patch
ntp-convert-to-the-ntp4-reference-model.patch
ntp-cleanup-defines-and-comments.patch
kernel-time-ntpc-possible-cleanups.patch
-
To unsubscribe from this list: send the line "unsubscribe mm-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
