From 6b23316e66ec3d2f1417896e1fa25c24a8ae32d3 Mon Sep 17 00:00:00 2001 From: Ulrich Drepper Date: Fri, 15 May 2009 20:42:36 -0700 Subject: [PATCH] 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. (cherry picked from commit 9437b427cec6266abd303983848549a5c4ba0d0a) --- nptl/ChangeLog | 6 ++++++ nptl/cancellation.c | 8 ++------ nptl/libc-cancellation.c | 8 ++------ nptl/pthread_cancel.c | 8 ++++++-- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/nptl/ChangeLog b/nptl/ChangeLog index 74a2a73666b..f3bd32f5778 100644 --- a/nptl/ChangeLog +++ b/nptl/ChangeLog @@ -1,5 +1,11 @@ 2009-05-15 Ulrich Drepper + * 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 4d528cfc2f6..2a6f83d28a3 100644 --- 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 35ac82b3d1b..cada464d2bd 100644 --- 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 a13af56b37d..55bb0da922b 100644 --- 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 , 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. */ -- 2.47.2