[PATCH] Fix race between sem_post and semaphore destruction [BZ #12674]

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

Summary of the race:

T1: enter sem_post and increment value
T2: enter_sem_wait and decrement value
T2: return from sem_wait and destroy the semaphore
T1: Check value of semaphore->nwaiters and find an invalid value

The idea for the fix is adapted from Rich Felker's fix for the same
race in musl.  The fix is to prevent sem_post from accessing nwaiters
after it has incremented the value since the state of the semaphore is
not known beyond this point.  This is fairly easy to achieve using
Rich's idea.  One may set the value to a special value of -1 to
indicate waiters.  That way, sem_post can inspect the old value and
call futex_wake if it is necessary.

Rich used this condition as a primary check and the waiter count as a
secondary check, but I think the secondary check is not required in
glibc.  The only time the secondary check would theoretically be
required is when the old value came up as zero *and* there were
waiters to wake up.  This happens only momentarily when an exiting
waiter decrements nwaiters and resets the semaphore value if it is -1
and that operation races with a new waiter entering and losing the
race, thus keeping the value as 0.  This is momentary because the
futex call on such a value will fail with EWOULDBLOCK since it expects
the value to be -1 and not 0.  After this failure, the waiter fixes up
the value to -1 and goes back to wait.

This requires two other changes:

- The type of value is now int instead of unsigned int.  This should
  not break the ABI since we don't expose the sign of the value.  In
  fact, the only place the value is seen is with sem_getvalue, where
  it is int.  And speaking of sem_getvalue...

- sem_getvalue is patched to lie about the actual value if it sees the
  -1 and return 0.

Siddhesh

	[BZ #12674]
	* nptl/sem_getvalue.c (__new_sem_getvalue): Return 0 for
	negative semaphore value.
	* nptl/sysdeps/unix/sysv/linux/internaltypes.h (struct
	new_sem): Change type of VALUE to int.
	* nptl/sysdeps/unix/sysv/linux/sem_post.c (__new_sem_post):
	Avoid accessing semaphore after incrementing it.
	* sysdeps/unix/sysv/linux/i386/i486/sem_post.S
	(__new_sem_post): Likewise.
	* sysdeps/unix/sysv/linux/x86_64/sem_post.S (__new_sem_post):
	Likewise.
	* nptl/sysdeps/unix/sysv/linux/sem_timedwait.c
	(do_futex_timed_wait): Set expected value of futex to -1.
	(sem_timedwait): Set expected value of semaphore to -1.
	* sysdeps/unix/sysv/linux/i386/i486/sem_timedwait.S
	(sem_wait_cleanup): Reset semaphore value when there are no
	waiters.
	(sem_timedwait): Set expected value of semaphore to -1.
	* sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S
	(sem_wait_cleanup): Reset semaphore value when there are no
	waiters.
	(sem_wait_cleanup2): Likewise.
	(sem_timedwait): Set expected value of semaphore to -1.
	* nptl/sysdeps/unix/sysv/linux/sem_wait.c
	(__sem_wait_cleanup): Reset semaphore value when there are no
	waiters.
	(do_futex_wait): Set expected value of futex to -1.
	(__new_sem_wait): Set expected value of semaphore to -1.
	* sysdeps/unix/sysv/linux/i386/i486/sem_wait.S
	(sem_wait_cleanup): Reset semaphore value when there are no
	waiters.
	(__new_sem_wait): Set expected value of semaphore to -1.
	* sysdeps/unix/sysv/linux/x86_64/sem_wait.S
	(sem_wait_cleanup): Reset semaphore value when there are no
	waiters.
	(__new_sem_wait): Set expected value of semaphore to -1.


diff --git a/nptl/sem_getvalue.c b/nptl/sem_getvalue.c
index a4ab41f..d9b70fd 100644
--- a/nptl/sem_getvalue.c
+++ b/nptl/sem_getvalue.c
@@ -22,15 +22,15 @@
 
 
 int
-__new_sem_getvalue (sem, sval)
-     sem_t *sem;
-     int *sval;
+__new_sem_getvalue (sem_t *sem, int *sval)
 {
   struct new_sem *isem = (struct new_sem *) sem;
 
   /* XXX Check for valid SEM parameter.  */
 
   *sval = isem->value;
+  if (*sval < 0)
+    *sval = 0;
 
   return 0;
 }
diff --git a/nptl/sysdeps/unix/sysv/linux/internaltypes.h b/nptl/sysdeps/unix/sysv/linux/internaltypes.h
index d127f68..5eea097 100644
--- a/nptl/sysdeps/unix/sysv/linux/internaltypes.h
+++ b/nptl/sysdeps/unix/sysv/linux/internaltypes.h
@@ -141,7 +141,7 @@ struct pthread_key_struct
 /* Semaphore variable structure.  */
 struct new_sem
 {
-  unsigned int value;
+  int value;
   int private;
   unsigned long int nwaiters;
 };
diff --git a/nptl/sysdeps/unix/sysv/linux/sem_post.c b/nptl/sysdeps/unix/sysv/linux/sem_post.c
index 4906adf..0ff1699 100644
--- a/nptl/sysdeps/unix/sysv/linux/sem_post.c
+++ b/nptl/sysdeps/unix/sysv/linux/sem_post.c
@@ -30,24 +30,35 @@ int
 __new_sem_post (sem_t *sem)
 {
   struct new_sem *isem = (struct new_sem *) sem;
+  int incr, is_private = isem->private;
 
   __typeof (isem->value) cur;
   do
     {
       cur = isem->value;
+      incr = 1 + (cur < 0);
       if (isem->value == SEM_VALUE_MAX)
 	{
 	  __set_errno (EOVERFLOW);
 	  return -1;
 	}
     }
-  while (atomic_compare_and_exchange_bool_rel (&isem->value, cur + 1, cur));
+  while (atomic_compare_and_exchange_bool_rel (&isem->value, cur + incr, cur));
 
   atomic_full_barrier ();
-  if (isem->nwaiters > 0)
+  /* This is always a sufficient condition to detect waiters.  This is because
+     once there is either a waiter or a poster, the value is always non-zero at
+     this point, either because sem_post set it to a positive value or sem_wait
+     set it to a negative value.  There is a transient condition whe it could
+     be 0 with a waiter.  This happens when a waiter is cancelled and another
+     waiter arrives, where a race condition causes the value to be 0 before the
+     futex_wait is called.  That is fixed immediately since the futex_wait will
+     return immediately with EWOULDBLOCK, fix the value and go back to
+     sleep in futex_wait.  */
+  if (cur < 0)
     {
       int err = lll_futex_wake (&isem->value, 1,
-				isem->private ^ FUTEX_PRIVATE_FLAG);
+				is_private ^ FUTEX_PRIVATE_FLAG);
       if (__builtin_expect (err, 0) < 0)
 	{
 	  __set_errno (-err);
diff --git a/nptl/sysdeps/unix/sysv/linux/sem_timedwait.c b/nptl/sysdeps/unix/sysv/linux/sem_timedwait.c
index 7dfe51d..1e74c40 100644
--- a/nptl/sysdeps/unix/sysv/linux/sem_timedwait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sem_timedwait.c
@@ -38,7 +38,7 @@ do_futex_timed_wait (struct new_sem *isem, struct timespec *rt)
 {
   int err, oldtype = __pthread_enable_asynccancel ();
 
-  err = lll_futex_timed_wait (&isem->value, 0, rt,
+  err = lll_futex_timed_wait (&isem->value, -1, rt,
 			      isem->private ^ FUTEX_PRIVATE_FLAG);
 
   __pthread_disable_asynccancel (oldtype);
@@ -60,6 +60,8 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
       return -1;
     }
 
+  /* If the value is zero, set it to -1 to indicate waiters.  */
+  atomic_compare_and_exchange_val_acq (&isem->value, -1, 0);
   atomic_increment (&isem->nwaiters);
 
   pthread_cleanup_push (__sem_wait_cleanup, isem);
@@ -106,11 +108,17 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
 	  err = 0;
 	  break;
 	}
+
+      /* Still not time to wake up.  Go back to sleep.  */
+      atomic_compare_and_exchange_val_acq (&isem->value, -1, 0);
     }
 
   pthread_cleanup_pop (0);
 
-  atomic_decrement (&isem->nwaiters);
+  /* Reset semaphore value to zero if we are the last waiter.  The reset will
+     actually happen only when we exit due to an error.  */
+  if (atomic_decrement_and_test (&isem->nwaiters))
+    atomic_compare_and_exchange_val_acq (&isem->value, 0, -1);
 
   return err;
 }
diff --git a/nptl/sysdeps/unix/sysv/linux/sem_wait.c b/nptl/sysdeps/unix/sysv/linux/sem_wait.c
index b12babb..4d3f91b 100644
--- a/nptl/sysdeps/unix/sysv/linux/sem_wait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sem_wait.c
@@ -34,7 +34,12 @@ __sem_wait_cleanup (void *arg)
 {
   struct new_sem *isem = (struct new_sem *) arg;
 
-  atomic_decrement (&isem->nwaiters);
+  /* Decrement nwaiters and reset value if there are no other waiters.  This
+     could race with the futex_wait call in another waiter and cause it to wake
+     up when it shouldn't, but that is OK since it will go right back to sleep
+     when it sees that the semaphore value is not what it wants.  */
+  if (atomic_decrement_and_test (&isem->nwaiters))
+    atomic_compare_and_exchange_val_acq (&isem->value, 0, -1);
 }
 
 /* This is in a seperate function in order to make sure gcc
@@ -46,7 +51,7 @@ do_futex_wait (struct new_sem *isem)
 {
   int err, oldtype = __pthread_enable_asynccancel ();
 
-  err = lll_futex_wait (&isem->value, 0, isem->private ^ FUTEX_PRIVATE_FLAG);
+  err = lll_futex_wait (&isem->value, -1, isem->private ^ FUTEX_PRIVATE_FLAG);
 
   __pthread_disable_asynccancel (oldtype);
   return err;
@@ -61,6 +66,12 @@ __new_sem_wait (sem_t *sem)
   if (atomic_decrement_if_positive (&isem->value) > 0)
     return 0;
 
+  /* If we are the only know waiter right now, indicate that by setting the
+     value to -1.  This is useful to avoid access to nwaiters in sem_post when
+     the sole waiter exits and destroys the semaphore before sem_post has a
+     chance to test the value of nwaiters.  */
+  atomic_compare_and_exchange_val_acq (&isem->value, -1, 0);
+
   atomic_increment (&isem->nwaiters);
 
   pthread_cleanup_push (__sem_wait_cleanup, isem);
@@ -80,11 +91,17 @@ __new_sem_wait (sem_t *sem)
 	  err = 0;
 	  break;
 	}
+
+      /* Still not time to wake up.  Go back to sleep.  */
+      atomic_compare_and_exchange_val_acq (&isem->value, -1, 0);
     }
 
   pthread_cleanup_pop (0);
 
-  atomic_decrement (&isem->nwaiters);
+  /* Reset semaphore value to zero if we are the last waiter.  The reset will
+     actually happen only when we exit due to an error.  */
+  if (atomic_decrement_and_test (&isem->nwaiters))
+    atomic_compare_and_exchange_val_acq (&isem->value, -1, 0);
 
   return err;
 }
diff --git a/sysdeps/unix/sysv/linux/i386/i486/sem_post.S b/sysdeps/unix/sysv/linux/i386/i486/sem_post.S
index bc091a0..82435ab 100644
--- a/sysdeps/unix/sysv/linux/i386/i486/sem_post.S
+++ b/sysdeps/unix/sysv/linux/i386/i486/sem_post.S
@@ -35,6 +35,7 @@ __new_sem_post:
 	cfi_offset(%ebx, -8)
 
 	movl	8(%esp), %ebx
+	movl	PRIVATE(%ebx), %ecx
 
 #if VALUE == 0
 	movl	(%ebx), %eax
@@ -43,8 +44,13 @@ __new_sem_post:
 #endif
 0:	cmpl	$SEM_VALUE_MAX, %eax
 	je	3f
+
+	/* Add 2 to the value if it is negative, or else add 1.  */
 	leal	1(%eax), %edx
-	LOCK
+	testl   %eax, %eax
+	jns     6f
+	addl    $1, %edx
+6:	LOCK
 #if VALUE == 0
 	cmpxchgl %edx, (%ebx)
 #else
@@ -52,11 +58,12 @@ __new_sem_post:
 #endif
 	jnz	0b
 
-	cmpl	$0, NWAITERS(%ebx)
-	je	2f
+	/* Test the old semaphore value again.  Don't do the syscall if the
+	   sign bit is not set, i.e. the value is not negative.  */
+	testl	%eax, %eax
+	jns	2f
 
-	movl	$FUTEX_WAKE, %ecx
-	orl	PRIVATE(%ebx), %ecx
+	orl	$FUTEX_WAKE, %ecx
 	movl	$1, %edx
 	movl	$SYS_futex, %eax
 	ENTER_KERNEL
diff --git a/sysdeps/unix/sysv/linux/i386/i486/sem_timedwait.S b/sysdeps/unix/sysv/linux/i386/i486/sem_timedwait.S
index 94d052a..575359b 100644
--- a/sysdeps/unix/sysv/linux/i386/i486/sem_timedwait.S
+++ b/sysdeps/unix/sysv/linux/i386/i486/sem_timedwait.S
@@ -39,7 +39,7 @@ sem_timedwait:
 
 	movl	(%ecx), %eax
 2:	testl	%eax, %eax
-	je	1f
+	jle	1f
 
 	leal	-1(%eax), %edx
 	LOCK
@@ -87,17 +87,25 @@ sem_timedwait:
 	addl	$1000000000, %edx
 	subl	$1, %ecx
 5:	testl	%ecx, %ecx
-	movl	$ETIMEDOUT, %esi
-	js	6f		/* Time is already up.  */
+	movl	$-ETIMEDOUT, %esi
+	movl	28(%esp), %ebx	/* Load semaphore address.  */
+	js	10f		/* Time is already up.  */
 
 	movl	%ecx, (%esp)	/* Store relative timeout.  */
 	movl	%edx, 4(%esp)
 
+	/* If value is 0, set it to -1.  */
+	movl	%eax, %edx
+	movl	$-1, %ecx
+	xorl	%eax, %eax
+	LOCK
+	cmpxchgl %ecx, (%ebx)
+	movl	%edx, %eax
+
 .LcleanupSTART:
 	call	__pthread_enable_asynccancel
 	movl	%eax, 8(%esp)
 
-	movl	28(%esp), %ebx	/* Load semaphore address.  */
 #if FUTEX_WAIT == 0
 	movl	PRIVATE(%ebx), %ecx
 #else
@@ -105,7 +113,7 @@ sem_timedwait:
 	orl	PRIVATE(%ebx), %ecx
 #endif
 	movl	%esp, %esi
-	xorl	%edx, %edx
+	movl	$-1, %edx
 	movl	$SYS_futex, %eax
 	ENTER_KERNEL
 	movl	%eax, %esi
@@ -117,11 +125,11 @@ sem_timedwait:
 	testl	%esi, %esi
 	je	9f
 	cmpl	$-EWOULDBLOCK, %esi
-	jne	3f
+	jne	10f
 
 9:	movl	(%ebx), %eax
 8:	testl	%eax, %eax
-	je	7b
+	jle	7b
 
 	leal	-1(%eax), %ecx
 	LOCK
@@ -130,10 +138,23 @@ sem_timedwait:
 
 	xorl	%eax, %eax
 
-	LOCK
+10:	LOCK
 	decl	NWAITERS(%ebx)
+	jnz	11f
+
+	movl	%eax, %edx
+	movl	$-1, %eax
+	xorl	%ecx, %ecx
+	LOCK
+	cmpxchgl %ecx, (%ebx)
+	movl	%edx, %eax
+	/* Set errno if the kernel returned an error.  We moved the syscall
+	   return value into %esi earlier.  */
+	test	%esi, %esi
+	jnz	6f
+
+11:	addl	$12, %esp
 
-10:	addl	$12, %esp
 .Ladd_esp:
 	popl	%ebx
 .Lpop_ebx:
@@ -144,11 +165,7 @@ sem_timedwait:
 	ret
 
 .Lafter_ret:
-3:	negl	%esi
-6:
-	movl	28(%esp), %ebx	/* Load semaphore address.  */
-	LOCK
-	decl	NWAITERS(%ebx)
+6:	negl	%esi
 .Lerrno_exit:
 #ifdef PIC
 	SETUP_PIC_REG(bx)
@@ -167,15 +184,23 @@ sem_timedwait:
 #endif
 
 	orl	$-1, %eax
-	jmp	10b
+	jmp	11b
 	.size	sem_timedwait,.-sem_timedwait
 
 
 	.type	sem_wait_cleanup,@function
 sem_wait_cleanup:
+	movl	%eax, %edx
 	LOCK
 	decl	NWAITERS(%ebx)
-	movl	%eax, (%esp)
+	jnz	11f
+
+	movl	$-1, %eax
+	xorl	%ecx, %ecx
+	LOCK
+	cmpxchgl %ecx, (%ebx)
+
+11:	movl	%edx, (%esp)
 .LcallUR:
 	call	_Unwind_Resume@PLT
 	hlt
diff --git a/sysdeps/unix/sysv/linux/i386/i486/sem_wait.S b/sysdeps/unix/sysv/linux/i386/i486/sem_wait.S
index 14d616f..db7637f 100644
--- a/sysdeps/unix/sysv/linux/i386/i486/sem_wait.S
+++ b/sysdeps/unix/sysv/linux/i386/i486/sem_wait.S
@@ -45,13 +45,13 @@ __new_sem_wait:
 
 	movl	(%ebx), %eax
 2:	testl	%eax, %eax
-	je	1f
+	jle	1f
 
 	leal	-1(%eax), %edx
 	LOCK
 	cmpxchgl %edx, (%ebx)
 	jne	2b
-7:	xorl	%eax, %eax
+	xorl	%eax, %eax
 
 9:	movl	4(%esp), %esi
 	movl	8(%esp), %ebx
@@ -63,8 +63,16 @@ __new_sem_wait:
 1:	LOCK
 	incl	NWAITERS(%ebx)
 
+	/* If value is 0, set it to -1.  */
+6:	movl	%eax, %edx
+	movl	$-1, %ecx
+	xorl	%eax, %eax
+	LOCK
+	cmpxchgl %ecx, (%ebx)
+	movl	%edx, %eax
+
 .LcleanupSTART:
-6:	call	__pthread_enable_asynccancel
+	call	__pthread_enable_asynccancel
 	movl	%eax, (%esp)
 
 #if FUTEX_WAIT == 0
@@ -74,7 +82,7 @@ __new_sem_wait:
 	orl	PRIVATE(%ebx), %ecx
 #endif
 	xorl	%esi, %esi
-	xorl	%edx, %edx
+	movl	$-1, %edx
 	movl	$SYS_futex, %eax
 	ENTER_KERNEL
 	movl	%eax, %esi
@@ -91,19 +99,30 @@ __new_sem_wait:
 3:
 	movl	(%ebx), %eax
 5:	testl	%eax, %eax
-	je	6b
+	jle	6b
 
 	leal	-1(%eax), %edx
 	LOCK
 	cmpxchgl %edx, (%ebx)
 	jne	5b
 
-	LOCK
+	xorl	%eax, %eax
+	/* Decrement nwaiters and if zero, reset the value to 0 if it is
+	   -1.  */
+7:	LOCK
 	decl	NWAITERS(%ebx)
-	jmp	7b
+	jnz	9b
+	movl	%eax, %edx
+	movl	$-1, %eax
+	xorl	%ecx, %ecx
+	LOCK
+	cmpxchgl %ecx, (%ebx)
+	movl	%edx, %eax
+	jmp	9b
 
-4:	LOCK
-	decl	NWAITERS(%ebx)
+	/* Back up the semaphore pointer before we set up the GOT pointer to
+	   store errno.  */
+4:	movl	%ebx, %ecx
 
 	negl	%esi
 #ifdef PIC
@@ -122,17 +141,24 @@ __new_sem_wait:
 	movl	%esi, %gs:(%edx)
 #endif
 	orl	$-1, %eax
+	movl	%ecx, %ebx
 
-	jmp	9b
+	jmp	7b
 	.size	__new_sem_wait,.-__new_sem_wait
 	versioned_symbol(libpthread, __new_sem_wait, sem_wait, GLIBC_2_1)
 
 
 	.type	sem_wait_cleanup,@function
 sem_wait_cleanup:
+	movl	%eax, %edx
 	LOCK
 	decl	NWAITERS(%ebx)
-	movl	%eax, (%esp)
+	jnz	1f
+	movl	$-1, %eax
+	xorl	%ecx, %ecx
+	LOCK
+	cmpxchgl %ecx, (%ebx)
+1:	movl	%edx, (%esp)
 .LcallUR:
 	call	_Unwind_Resume@PLT
 	hlt
diff --git a/sysdeps/unix/sysv/linux/x86_64/sem_post.S b/sysdeps/unix/sysv/linux/x86_64/sem_post.S
index 1c11600..854fa56 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sem_post.S
+++ b/sysdeps/unix/sysv/linux/x86_64/sem_post.S
@@ -29,6 +29,8 @@
 	.type	sem_post,@function
 	.align	16
 sem_post:
+	/* Get the private flag in advance.  */
+	movl	PRIVATE(%rdi), %esi
 #if VALUE == 0
 	movl	(%rdi), %eax
 #else
@@ -36,21 +38,27 @@ sem_post:
 #endif
 0:	cmpl	$SEM_VALUE_MAX, %eax
 	je	3f
-	leal	1(%rax), %esi
-	LOCK
+
+	/* Add 2 to the value if it is negative, or else add 1.  */
+	leal	1(%rax), %edx
+	testl	%eax, %eax
+	jns	5f
+	addl	$1, %edx
+5:	LOCK
 #if VALUE == 0
-	cmpxchgl %esi, (%rdi)
+	cmpxchgl %edx, (%rdi)
 #else
-	cmpxchgl %esi, VALUE(%rdi)
+	cmpxchgl %edx, VALUE(%rdi)
 #endif
 	jnz	0b
 
-	LP_OP(cmp) $0, NWAITERS(%rdi)
-	je	2f
+	/* Test the old semaphore value again.  Don't do the syscall if the
+	   sign bit is not set, i.e. the value is not negative.  */
+	testl	%eax, %eax
+	jns	2f
 
 	movl	$SYS_futex, %eax
-	movl	$FUTEX_WAKE, %esi
-	orl	PRIVATE(%rdi), %esi
+	orl	$FUTEX_WAKE, %esi
 	movl	$1, %edx
 	syscall
 
diff --git a/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S b/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S
index 880610e..104505b 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S
+++ b/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S
@@ -45,7 +45,7 @@ sem_timedwait:
 	movl	VALUE(%rdi), %eax
 #endif
 2:	testl	%eax, %eax
-	je	1f
+	jle	1f
 
 	leaq	-1(%rax), %rdx
 	LOCK
@@ -85,10 +85,24 @@ sem_timedwait:
 	LOCK
 	LP_OP(add) $1, NWAITERS(%rdi)
 
+
+13:	movl	%eax, %r8d
+
+	/* If the semaphore value is 0, set it to -1 before going to wait.  */
+	movl	$-1, %esi
+	movl	$0, %eax
+
+	LOCK
+#if VALUE == 0
+	cmpxchgl	%esi, (%rdi)
+#else
+	cmpxchgl	%esi, VALUE(%rdi)
+#endif
+	movl	%r8d, %eax
+
 .LcleanupSTART:
-13:	call	__pthread_enable_asynccancel
+	call	__pthread_enable_asynccancel
 	movl	%eax, %r8d
-
 #if VALUE != 0
 	leaq	VALUE(%rdi), %rdi
 #endif
@@ -96,7 +110,7 @@ sem_timedwait:
 	movl	$FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, %esi
 	orl	PRIVATE(%rdi), %esi
 	movl	$SYS_futex, %eax
-	xorl	%edx, %edx
+	movl	$-1, %edx
 	syscall
 	movq	%rax, %r9
 #if VALUE != 0
@@ -120,7 +134,7 @@ sem_timedwait:
 	movl	VALUE(%rdi), %eax
 #endif
 14:	testl	%eax, %eax
-	je	13b
+	jle	13b
 
 	leaq	-1(%rax), %rcx
 	LOCK
@@ -135,8 +149,24 @@ sem_timedwait:
 
 15:	LOCK
 	LP_OP(sub) $1, NWAITERS(%rdi)
+	jne 17f
+
+	/* If we were the last waiter, reset the value to 0 if it was set to
+	   -1.  This may race with another thread setting itself up to wait,
+	   but it is OK since it will just spin around and set up its wait
+	   again.  */
+	movq	%rax, %r12
+	movl	$-1, %eax
+	movl	$0, %edx
+	LOCK
+#if VALUE == 0
+	cmpxchgl	%edx, (%rdi)
+#else
+	cmpxchgl	%edx, VALUE(%rdi)
+#endif
+	movq	%r12, %rax
 
-	leaq	8(%rsp), %rsp
+17:	leaq	8(%rsp), %rsp
 	cfi_adjust_cfa_offset(-8)
 	retq
 
@@ -215,6 +245,19 @@ sem_timedwait:
 	movq	%rdi, (%rsp)	/* Store relative timeout.  */
 	movq	%rsi, 8(%rsp)
 
+	/* If the semaphore value is 0, set it to -1 before going to wait.  */
+	movl	%eax, %r10d
+	movl	$-1, %esi
+	movl	$0, %eax
+
+	LOCK
+#if VALUE == 0
+	cmpxchgl	%esi, (%r12)
+#else
+	cmpxchgl	%esi, VALUE(%r12)
+#endif
+	movl	%r10d, %eax
+
 .LcleanupSTART2:
 	call	__pthread_enable_asynccancel
 	movl	%eax, 16(%rsp)
@@ -232,7 +275,7 @@ sem_timedwait:
 	orl	PRIVATE(%rdi), %esi
 # endif
 	movl	$SYS_futex, %eax
-	xorl	%edx, %edx
+	movl	$-1, %edx
 	syscall
 	movq	%rax, %r14
 
@@ -252,7 +295,7 @@ sem_timedwait:
 	movl	VALUE(%r12), %eax
 # endif
 8:	testl	%eax, %eax
-	je	7b
+	jle	7b
 
 	leaq	-1(%rax), %rcx
 	LOCK
@@ -267,8 +310,24 @@ sem_timedwait:
 
 45:	LOCK
 	LP_OP(sub) $1, NWAITERS(%r12)
+	jne 46f
+
+	/* If we were the last waiter, reset the value to 0 if it was set to
+	   -1.  This may race with another thread setting itself up to wait,
+	   but it is OK since it will just spin around and set up its wait
+	   again.  */
+	movq	%rax, %r13
+	movl	$-1, %eax
+	movl	$0, %edx
+	LOCK
+#if VALUE == 0
+	cmpxchgl	%edx, (%r12)
+#else
+	cmpxchgl	%edx, VALUE(%r12)
+#endif
+	movq	%r13, %rax
 
-	addq	$STACKFRAME, %rsp
+46:	addq	$STACKFRAME, %rsp
 	cfi_adjust_cfa_offset(-STACKFRAME)
 	popq	%r14
 	cfi_adjust_cfa_offset(-8)
@@ -305,7 +364,24 @@ sem_timedwait_cleanup:
 	movq	(%rsp), %rdi
 	LOCK
 	LP_OP(sub) $1, NWAITERS(%rdi)
-	movq	%rax, %rdi
+	movq	%rax, %r12
+	jne 1f
+
+	/* If we were the last waiter, reset the value to 0 if it was set to
+	   -1.  This may race with another thread setting itself up to wait,
+	   but it is OK since it will just spin around and set up its wait
+	   again.  */
+	movl	$-1, %eax
+	movl	$0, %edx
+
+	LOCK
+#if VALUE == 0
+	cmpxchgl	%edx, (%rdi)
+#else
+	cmpxchgl	%edx, VALUE(%rdi)
+#endif
+
+1:	movq	%r12, %rdi
 .LcallUR:
 	call	_Unwind_Resume@PLT
 	hlt
@@ -326,7 +402,23 @@ sem_timedwait_cleanup2:
 	LOCK
 	LP_OP(sub) $1, NWAITERS(%r12)
 	movq	%rax, %rdi
-	movq	STACKFRAME(%rsp), %r14
+	jne 1f
+
+	/* If we were the last waiter, reset the value to 0 if it was set to
+	   -1.  This may race with another thread setting itself up to wait,
+	   but it is OK since it will just spin around and set up its wait
+	   again.  */
+	movl	$-1, %eax
+	movl	$0, %edx
+
+	LOCK
+#if VALUE == 0
+	cmpxchgl	%edx, (%r12)
+#else
+	cmpxchgl	%edx, VALUE(%r12)
+#endif
+
+1:	movq	STACKFRAME(%rsp), %r14
 	movq	STACKFRAME+8(%rsp), %r13
 	movq	STACKFRAME+16(%rsp), %r12
 .LcallUR2:
diff --git a/sysdeps/unix/sysv/linux/x86_64/sem_wait.S b/sysdeps/unix/sysv/linux/x86_64/sem_wait.S
index 8f4d068..bdbeb8a 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sem_wait.S
+++ b/sysdeps/unix/sysv/linux/x86_64/sem_wait.S
@@ -46,7 +46,7 @@ sem_wait:
 	movl	VALUE(%rdi), %eax
 #endif
 2:	testl	%eax, %eax
-	je	1f
+	jle	1f
 
 	leal	-1(%rax), %edx
 	LOCK
@@ -68,8 +68,21 @@ sem_wait:
 	LOCK
 	LP_OP(add) $1, NWAITERS(%rdi)
 
+	/* If the semaphore value is 0, set it to -1 before going to wait.  */
+6:	movl	%eax, %r8d
+	movl	$-1, %esi
+	movl	$0, %eax
+
+	LOCK
+#if VALUE == 0
+	cmpxchgl	%esi, (%rdi)
+#else
+	cmpxchgl	%esi, VALUE(%rdi)
+#endif
+	movl	%r8d, %eax
+
 .LcleanupSTART:
-6:	call	__pthread_enable_asynccancel
+	call	__pthread_enable_asynccancel
 	movl	%eax, %r8d
 
 	xorq	%r10, %r10
@@ -80,7 +93,7 @@ sem_wait:
 	movl	$FUTEX_WAIT, %esi
 	orl	PRIVATE(%rdi), %esi
 #endif
-	xorl	%edx, %edx
+	movl	$-1, %edx
 	syscall
 	movq	%rax, %rcx
 
@@ -101,7 +114,7 @@ sem_wait:
 	movl	VALUE(%rdi), %eax
 #endif
 5:	testl	%eax, %eax
-	je	6b
+	jle	6b
 
 	leal	-1(%rax), %edx
 	LOCK
@@ -137,7 +150,23 @@ sem_wait_cleanup:
 	movq	(%rsp), %rdi
 	LOCK
 	LP_OP(sub) $1, NWAITERS(%rdi)
-	movq	%rax, %rdi
+	movq	%rax, %r12
+	jne 1f
+
+	/* If we were the last waiter, reset the value to 0 if it was set to
+	   -1.  This may race with another thread setting itself up to wait,
+	   but it is OK since it will just spin around and set up its wait
+	   again.  */
+	movl	$-1, %eax
+	movl	$0, %edx
+
+	LOCK
+#if VALUE == 0
+	cmpxchgl	%edx, (%rdi)
+#else
+	cmpxchgl	%edx, VALUE(%rdi)
+#endif
+1:	movq	%r12, %rdi
 .LcallUR:
 	call	_Unwind_Resume@PLT
 	hlt

Attachment: pgp06Xtl7hkwx.pgp
Description: PGP signature