From: Brian Niebuhr <[email protected]>

__lll_timedlock_wait has a bug that is exposed in these conditions:

1. Thread 1 acquires the lock
2. Thread 2 attempts to acquire the lock and waits via futex syscall
3. Thread 1 unlocks the lock and wakes the waiting thread
4. Thread 1 re-aquires the lock before thread 2 gets the CPU

What happens in this case is that the futex value is set to '1' since
Thread 1 reaquired the lock through the fast path (since it had just been
released).  The problem is that Thread 2 is in the loop in
__lll_timedlock_wait and it expects the futex value to be '2'.
atomic_compare_and_exchange_bool_acq only sets the futex value to '2' if
it is already set to '0', and since Thread 1 reaquired the lock, the
futex remains set to '1'.

When Thread 2 attempts to wait on the futex, the operating system returns
-EWOULDBLOCK since the futex value is not '2'.  This causes a busy wait
condition where Thread 2 continuously attempts to wait on the futex and
the kernel immediately returns -EWOULDBLOCK.  This continues until Thread
1 releases the lock.

The fix is to use atomic_exchange_acq instead of
atomic_compare_and_exchange_bool_acq which will force the futex value to
'2' on each loop iteration.  This change makes uClibc line up with
glibc's implementation.

Signed-off-by: Brian Niebuhr <[email protected]>
---
 libpthread/nptl/sysdeps/unix/sysv/linux/arm/lowlevellock.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/libpthread/nptl/sysdeps/unix/sysv/linux/arm/lowlevellock.c 
b/libpthread/nptl/sysdeps/unix/sysv/linux/arm/lowlevellock.c
index af864b3..4f7e890 100644
--- a/libpthread/nptl/sysdeps/unix/sysv/linux/arm/lowlevellock.c
+++ b/libpthread/nptl/sysdeps/unix/sysv/linux/arm/lowlevellock.c
@@ -60,10 +60,7 @@ __lll_timedlock_wait (int *futex, const struct timespec 
*abstime, int private)
     return EINVAL;
 
   /* Upgrade the lock.  */
-  if (atomic_exchange_acq (futex, 2) == 0)
-    return 0;
-
-  do
+  while (atomic_exchange_acq (futex, 2) != 0)
     {
       struct timeval tv;
 
@@ -86,7 +83,6 @@ __lll_timedlock_wait (int *futex, const struct timespec 
*abstime, int private)
       // XYZ: Lost the lock to check whether it was private.
       lll_futex_timed_wait (futex, 2, &rt, private);
     }
-  while (atomic_compare_and_exchange_bool_acq (futex, 2, 0) != 0);
 
   return 0;
 }
-- 
1.8.3.2

_______________________________________________
uClibc mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/uclibc

Reply via email to