Fix change to prevent cancel signal in unsafe places.

The bits tested to decide when to delay the return when switching
off async cancel mode were wrong.  Fix that.  Also close a race
condition in pthread_cancel where the bit indicating the cancellation
is unconditionally set even if the cancel type might have changed.

diff --git a/nptl/ChangeLog b/nptl/ChangeLog
index 74a2a73666b7e3e6628aeb635b6d74b670470894..f3bd32f5778eff561cf1b891c10e0468de37a3ef 100644 (file)
--- a/nptl/ChangeLog
+++ b/nptl/ChangeLog
@@ -1,5 +1,11 @@
 2009-05-15  Ulrich Drepper  <drepper@redhat.com>
 
+       * cancellation.c (__pthread_disable_asynccancel): Correct the bits
+       to test when deciding on the delay.
+       * libc-cancellation.c (__libc_disable_asynccancel): Likewise.
+       * pthread_cancel.c: Close race between deciding on sending a signal
+       and setting the CANCELING_BIT bit.
+
        * cancellation.c (__pthread_disable_asynccancel): Don't return if
        thread is canceled.
        * libc-cancellation.c (__libc_disable_asynccancel): Likewise.

diff --git a/nptl/cancellation.c b/nptl/cancellation.c
index 4d528cfc2f6a4d1cd737c7508e0b36af9ffdef08..2a6f83d28a31ceca0899bf87a6fba55251f7749d 100644 (file)
--- a/nptl/cancellation.c
+++ b/nptl/cancellation.c
@@ -72,10 +72,6 @@ __pthread_disable_asynccancel (int oldtype)
   struct pthread *self = THREAD_SELF;
   int newval;
 
-#ifdef THREAD_ATOMIC_AND
-  THREAD_ATOMIC_AND (self, cancelhandling, ~CANCELTYPE_BITMASK);
-  newval = THREAD_GETMEM (self, cancelhandling);
-#else
   int oldval = THREAD_GETMEM (self, cancelhandling);
 
   while (1)
@@ -93,13 +89,13 @@ __pthread_disable_asynccancel (int oldtype)
       /* Prepare the next round.  */
       oldval = curval;
     }
-#endif
 
   /* We cannot return when we are being canceled.  Upon return the
      thread might be things which would have to be undone.  The
      following loop should loop until the cancellation signal is
      delivered.  */
-  while (__builtin_expect (newval & CANCELED_BITMASK, 0))
+  while (__builtin_expect ((newval & (CANCELING_BITMASK | CANCELED_BITMASK))
+                          == CANCELING_BITMASK, 0))
     {
       lll_futex_wait (&self->cancelhandling, newval, LLL_PRIVATE);
       newval = THREAD_GETMEM (self, cancelhandling);

diff --git a/nptl/libc-cancellation.c b/nptl/libc-cancellation.c
index 35ac82b3d1bd1e07eeae54d24167005588352d66..cada464d2bd2909dbce058cdbb3709fbdba8683b 100644 (file)
--- a/nptl/libc-cancellation.c
+++ b/nptl/libc-cancellation.c
@@ -88,10 +88,6 @@ __libc_disable_asynccancel (int oldtype)
   struct pthread *self = THREAD_SELF;
   int newval;
 
-#ifdef THREAD_ATOMIC_AND
-  THREAD_ATOMIC_AND (self, cancelhandling, ~CANCELTYPE_BITMASK);
-  newval = THREAD_GETMEM (self, cancelhandling);
-#else
   int oldval = THREAD_GETMEM (self, cancelhandling);
 
   while (1)
@@ -109,13 +105,13 @@ __libc_disable_asynccancel (int oldtype)
       /* Prepare the next round.  */
       oldval = curval;
     }
-#endif
 
   /* We cannot return when we are being canceled.  Upon return the
      thread might be things which would have to be undone.  The
      following loop should loop until the cancellation signal is
      delivered.  */
-  while (__builtin_expect (newval & CANCELED_BITMASK, 0))
+  while (__builtin_expect ((newval & (CANCELING_BITMASK | CANCELED_BITMASK))
+                          == CANCELING_BITMASK, 0))
     {
       lll_futex_wait (&self->cancelhandling, newval, LLL_PRIVATE);
       newval = THREAD_GETMEM (self, cancelhandling);

diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c
index a13af56b37d027d4258f0035a912a6158b22d730..55bb0da922ba1ed1c4bd33478075e1b41f2baaff 100644 (file)
--- a/nptl/pthread_cancel.c
+++ b/nptl/pthread_cancel.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
+/* Copyright (C) 2002, 2003, 2004, 2009 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
 
@@ -44,6 +44,7 @@ pthread_cancel (th)
   int newval;
   do
     {
+    again:
       oldval = pd->cancelhandling;
       newval = oldval | CANCELING_BITMASK | CANCELED_BITMASK;
 
@@ -59,7 +60,10 @@ pthread_cancel (th)
       if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval))
        {
          /* Mark the cancellation as "in progress".  */
-         atomic_bit_set (&pd->cancelhandling, CANCELING_BIT);
+         if (atomic_compare_and_exchange_bool_acq (&pd->cancelhandling,
+                                                   oldval | CANCELING_BITMASK,
+                                                   oldval))
+           goto again;
 
          /* The cancellation handler will take care of marking the
             thread as canceled.  */