[03/18] nptl: Move legacy unwinding implementation into libc

Message ID 4485b54412a6fb871b58c694bf8b0efd098f1111.1615569355.git.fweimer@redhat.com
State Superseded
Headers show
Series
  • Repost of pending libpthread removal patches
Related show

Commit Message

Ben Woodard via Libc-alpha March 12, 2021, 5:49 p.m.
It is still used internally.  Since unwinding is now available
unconditionally, avoid indirect calls through function pointers loaded
from the stack by inlining the non-cancellation cleanup code.  This
avoids a regression in security hardening.

The out-of-line  __libc_cleanup_routine implementation is no longer
needed because the inline definition is now static __always_inline.
---
 nptl/Versions                    |  2 +
 nptl/cleanup_defer_compat.c      | 56 ++--------------------------
 nptl/libc-cleanup.c              | 64 ++++++++++++++++++++++++++++++--
 nptl/nptl-init.c                 |  2 -
 sysdeps/nptl/libc-lock.h         | 59 ++++++++++++++---------------
 sysdeps/nptl/libc-lockP.h        | 26 +------------
 sysdeps/nptl/pthread-functions.h |  4 --
 7 files changed, 97 insertions(+), 116 deletions(-)

-- 
2.29.2

Comments

Ben Woodard via Libc-alpha March 15, 2021, 8:02 p.m. | #1
On 12/03/2021 14:49, Florian Weimer via Libc-alpha wrote:
> It is still used internally.  Since unwinding is now available

> unconditionally, avoid indirect calls through function pointers loaded

> from the stack by inlining the non-cancellation cleanup code.  This

> avoids a regression in security hardening.

> 

> The out-of-line  __libc_cleanup_routine implementation is no longer

> needed because the inline definition is now static __always_inline.


LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>


> ---

>  nptl/Versions                    |  2 +

>  nptl/cleanup_defer_compat.c      | 56 ++--------------------------

>  nptl/libc-cleanup.c              | 64 ++++++++++++++++++++++++++++++--

>  nptl/nptl-init.c                 |  2 -

>  sysdeps/nptl/libc-lock.h         | 59 ++++++++++++++---------------

>  sysdeps/nptl/libc-lockP.h        | 26 +------------

>  sysdeps/nptl/pthread-functions.h |  4 --

>  7 files changed, 97 insertions(+), 116 deletions(-)

> 

> diff --git a/nptl/Versions b/nptl/Versions

> index f2db649f9d..e3eb686a04 100644

> --- a/nptl/Versions

> +++ b/nptl/Versions

> @@ -86,6 +86,8 @@ libc {

>      __futex_abstimed_wait_cancelable64;

>      __libc_alloca_cutoff;

>      __libc_allocate_rtsig_private;

> +    __libc_cleanup_pop_restore;

> +    __libc_cleanup_push_defer;

>      __libc_current_sigrtmax_private;

>      __libc_current_sigrtmin_private;

>      __libc_dl_error_tsd;


Ok.

> diff --git a/nptl/cleanup_defer_compat.c b/nptl/cleanup_defer_compat.c

> index 49ef53ea60..1957318208 100644

> --- a/nptl/cleanup_defer_compat.c

> +++ b/nptl/cleanup_defer_compat.c

> @@ -17,41 +17,15 @@

>     <https://www.gnu.org/licenses/>.  */

>  

>  #include "pthreadP.h"

> -

> +#include <libc-lock.h>

>  

>  void

>  _pthread_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer,

>  			     void (*routine) (void *), void *arg)

>  {

> -  struct pthread *self = THREAD_SELF;

> -

>    buffer->__routine = routine;

>    buffer->__arg = arg;

> -  buffer->__prev = THREAD_GETMEM (self, cleanup);

> -

> -  int cancelhandling = THREAD_GETMEM (self, cancelhandling);

> -

> -  /* Disable asynchronous cancellation for now.  */

> -  if (__glibc_unlikely (cancelhandling & CANCELTYPE_BITMASK))

> -    while (1)

> -      {

> -	int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling,

> -						cancelhandling

> -						& ~CANCELTYPE_BITMASK,

> -						cancelhandling);

> -	if (__glibc_likely (curval == cancelhandling))

> -	  /* Successfully replaced the value.  */

> -	  break;

> -

> -	/* Prepare for the next round.  */

> -	cancelhandling = curval;

> -      }

> -

> -  buffer->__canceltype = (cancelhandling & CANCELTYPE_BITMASK

> -			  ? PTHREAD_CANCEL_ASYNCHRONOUS

> -			  : PTHREAD_CANCEL_DEFERRED);

> -

> -  THREAD_SETMEM (self, cleanup, buffer);

> +  __libc_cleanup_push_defer (buffer);

>  }

>  strong_alias (_pthread_cleanup_push_defer, __pthread_cleanup_push_defer)

>  


Ok.

> @@ -60,31 +34,7 @@ void

>  _pthread_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer,

>  			      int execute)

>  {

> -  struct pthread *self = THREAD_SELF;

> -

> -  THREAD_SETMEM (self, cleanup, buffer->__prev);

> -

> -  int cancelhandling;

> -  if (__builtin_expect (buffer->__canceltype != PTHREAD_CANCEL_DEFERRED, 0)

> -      && ((cancelhandling = THREAD_GETMEM (self, cancelhandling))

> -	  & CANCELTYPE_BITMASK) == 0)

> -    {

> -      while (1)

> -	{

> -	  int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling,

> -						  cancelhandling

> -						  | CANCELTYPE_BITMASK,

> -						  cancelhandling);

> -	  if (__glibc_likely (curval == cancelhandling))

> -	    /* Successfully replaced the value.  */

> -	    break;

> -

> -	  /* Prepare for the next round.  */

> -	  cancelhandling = curval;

> -	}

> -

> -      CANCELLATION_P (self);

> -    }

> +  __libc_cleanup_pop_restore (buffer);

>  

>    /* If necessary call the cleanup routine after we removed the

>       current cleanup block from the list.  */


Ok.

> diff --git a/nptl/libc-cleanup.c b/nptl/libc-cleanup.c

> index 61f97eceda..14ccfe9285 100644

> --- a/nptl/libc-cleanup.c

> +++ b/nptl/libc-cleanup.c

> @@ -17,11 +17,69 @@

>     <https://www.gnu.org/licenses/>.  */

>  

>  #include "pthreadP.h"

> +#include <tls.h>

> +#include <libc-lock.h>

>  

> +void

> +__libc_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer)

> +{

> +  struct pthread *self = THREAD_SELF;

> +

> +  buffer->__prev = THREAD_GETMEM (self, cleanup);

> +

> +  int cancelhandling = THREAD_GETMEM (self, cancelhandling);

> +

> +  /* Disable asynchronous cancellation for now.  */

> +  if (__glibc_unlikely (cancelhandling & CANCELTYPE_BITMASK))

> +    while (1)

> +      {

> +	int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling,

> +						cancelhandling

> +						& ~CANCELTYPE_BITMASK,

> +						cancelhandling);

> +	if (__glibc_likely (curval == cancelhandling))

> +	  /* Successfully replaced the value.  */

> +	  break;

> +

> +	/* Prepare for the next round.  */

> +	cancelhandling = curval;

> +      }

> +

> +  buffer->__canceltype = (cancelhandling & CANCELTYPE_BITMASK

> +			  ? PTHREAD_CANCEL_ASYNCHRONOUS

> +			  : PTHREAD_CANCEL_DEFERRED);

> +

> +  THREAD_SETMEM (self, cleanup, buffer);

> +}

> +libc_hidden_def (__libc_cleanup_push_defer)

>  

>  void

> -__libc_cleanup_routine (struct __pthread_cleanup_frame *f)

> +__libc_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer)

>  {

> -  if (f->__do_it)

> -    f->__cancel_routine (f->__cancel_arg);

> +  struct pthread *self = THREAD_SELF;

> +

> +  THREAD_SETMEM (self, cleanup, buffer->__prev);

> +

> +  int cancelhandling;

> +  if (__builtin_expect (buffer->__canceltype != PTHREAD_CANCEL_DEFERRED, 0)

> +      && ((cancelhandling = THREAD_GETMEM (self, cancelhandling))

> +	  & CANCELTYPE_BITMASK) == 0)

> +    {

> +      while (1)

> +	{

> +	  int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling,

> +						  cancelhandling

> +						  | CANCELTYPE_BITMASK,

> +						  cancelhandling);

> +	  if (__glibc_likely (curval == cancelhandling))

> +	    /* Successfully replaced the value.  */

> +	    break;

> +

> +	  /* Prepare for the next round.  */

> +	  cancelhandling = curval;

> +	}

> +

> +      CANCELLATION_P (self);

> +    }

>  }

> +libc_hidden_def (__libc_cleanup_pop_restore)


Ok.

> diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c

> index 865ee8db29..c2b563cc68 100644

> --- a/nptl/nptl-init.c

> +++ b/nptl/nptl-init.c

> @@ -96,8 +96,6 @@ static const struct pthread_functions pthread_functions =

>      .ptr___pthread_key_create = __pthread_key_create,

>      .ptr___pthread_getspecific = __pthread_getspecific,

>      .ptr___pthread_setspecific = __pthread_setspecific,

> -    .ptr__pthread_cleanup_push_defer = __pthread_cleanup_push_defer,

> -    .ptr__pthread_cleanup_pop_restore = __pthread_cleanup_pop_restore,

>      .ptr_nthreads = &__nptl_nthreads,

>      .ptr___pthread_unwind = &__pthread_unwind,

>      .ptr__nptl_deallocate_tsd = __nptl_deallocate_tsd,


Ok.

> diff --git a/sysdeps/nptl/libc-lock.h b/sysdeps/nptl/libc-lock.h

> index dea96121b3..56fe83f69f 100644

> --- a/sysdeps/nptl/libc-lock.h

> +++ b/sysdeps/nptl/libc-lock.h

> @@ -143,39 +143,40 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t;

>    __libc_maybe_call (__pthread_mutex_unlock, (&(NAME).mutex), 0)

>  #endif

>  

> -/* Note that for I/O cleanup handling we are using the old-style

> -   cancel handling.  It does not have to be integrated with C++ since

> -   no C++ code is called in the middle.  The old-style handling is

> -   faster and the support is not going away.  */

> -extern void _pthread_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer,

> -					 void (*routine) (void *), void *arg);

> -extern void _pthread_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer,

> -					  int execute);

> +/* Put the unwind buffer BUFFER on the per-thread callback stack.  The

> +   caller must fill BUFFER->__routine and BUFFER->__arg before calling

> +   this function.  */

> +void __libc_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer);

> +libc_hidden_proto (__libc_cleanup_push_defer)

> +/* Remove BUFFER from the unwind callback stack.  The caller must invoke

> +   the callback if desired.  */

> +void __libc_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer);

> +libc_hidden_proto (__libc_cleanup_pop_restore)

>  


Ok.

>  /* Start critical region with cleanup.  */

> -#define __libc_cleanup_region_start(DOIT, FCT, ARG) \

> -  { struct _pthread_cleanup_buffer _buffer;				      \

> -    int _avail;								      \

> -    if (DOIT) {								      \

> -      _avail = PTFAVAIL (_pthread_cleanup_push_defer);			      \

> -      if (_avail) {							      \

> -	__libc_ptf_call_always (_pthread_cleanup_push_defer, (&_buffer, FCT,  \

> -							      ARG));	      \

> -      } else {								      \

> -	_buffer.__routine = (FCT);					      \

> -	_buffer.__arg = (ARG);						      \

> -      }									      \

> -    } else {								      \

> -      _avail = 0;							      \

> -    }

> +#define __libc_cleanup_region_start(DOIT, FCT, ARG)			\

> +  {   bool _cleanup_start_doit;						\

> +  struct _pthread_cleanup_buffer _buffer;				\

> +  /* Non-addresable copy of FCT, so that we avoid indirect calls on	\


s/addresable/addressable

> +     the non-unwinding path.  */					\

> +  void (*_cleanup_routine) (void *) = (FCT);				\

> +  _buffer.__arg = (ARG);						\

> +  if (DOIT)								\

> +    {									\

> +      _cleanup_start_doit = true;					\

> +      _buffer.__routine = _cleanup_routine;				\

> +      __libc_cleanup_push_defer (&_buffer);				\

> +    }									\

> +  else									\

> +      _cleanup_start_doit = false;

>  

>  /* End critical region with cleanup.  */

> -#define __libc_cleanup_region_end(DOIT) \

> -    if (_avail) {							      \

> -      __libc_ptf_call_always (_pthread_cleanup_pop_restore, (&_buffer, DOIT));\

> -    } else if (DOIT)							      \

> -      _buffer.__routine (_buffer.__arg);				      \

> -  }

> +#define __libc_cleanup_region_end(DOIT)		\

> +  if (_cleanup_start_doit)			\

> +    __libc_cleanup_pop_restore (&_buffer);	\

> +  if (DOIT)					\

> +    _cleanup_routine (_buffer.__arg);		\

> +  } /* matches __libc_cleanup_region_start */

>  


Ok.

>  

>  /* Hide the definitions which are only supposed to be used inside libc in

> diff --git a/sysdeps/nptl/libc-lockP.h b/sysdeps/nptl/libc-lockP.h

> index 4a0b96e6d9..1a861b0d3f 100644

> --- a/sysdeps/nptl/libc-lockP.h

> +++ b/sysdeps/nptl/libc-lockP.h

> @@ -251,32 +251,12 @@ _Static_assert (LLL_LOCK_INITIALIZER == 0, "LLL_LOCK_INITIALIZER != 0");

>  /* Get once control variable.  */

>  #define __libc_once_get(ONCE_CONTROL)	((ONCE_CONTROL) != PTHREAD_ONCE_INIT)

>  

> -/* Note that for I/O cleanup handling we are using the old-style

> -   cancel handling.  It does not have to be integrated with C++ snce

> -   no C++ code is called in the middle.  The old-style handling is

> -   faster and the support is not going away.  */

> -extern void _pthread_cleanup_push (struct _pthread_cleanup_buffer *buffer,

> -				   void (*routine) (void *), void *arg);

> -extern void _pthread_cleanup_pop (struct _pthread_cleanup_buffer *buffer,

> -				  int execute);

> -extern void _pthread_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer,

> -					 void (*routine) (void *), void *arg);

> -extern void _pthread_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer,

> -					  int execute);

> -

> -/* Sometimes we have to exit the block in the middle.  */

> -#define __libc_cleanup_end(DOIT) \

> -    if (_avail) {							      \

> -      __libc_ptf_call_always (_pthread_cleanup_pop_restore, (&_buffer, DOIT));\

> -    } else if (DOIT)							      \

> -      _buffer.__routine (_buffer.__arg)

> -

>  /* __libc_cleanup_push and __libc_cleanup_pop depend on exception

>     handling and stack unwinding.  */

>  #ifdef __EXCEPTIONS

>  


Ok.

>  /* Normal cleanup handling, based on C cleanup attribute.  */

> -__extern_inline void

> +static __always_inline void

>  __libc_cleanup_routine (struct __pthread_cleanup_frame *f)

>  {

>    if (f->__do_it)

> @@ -396,8 +376,6 @@ weak_extern (__pthread_once)

>  weak_extern (__pthread_initialize)

>  weak_extern (__pthread_atfork)

>  weak_extern (__pthread_setcancelstate)

> -weak_extern (_pthread_cleanup_push_defer)

> -weak_extern (_pthread_cleanup_pop_restore)

>  # else

>  #  pragma weak __pthread_mutex_init

>  #  pragma weak __pthread_mutex_destroy

> @@ -420,8 +398,6 @@ weak_extern (_pthread_cleanup_pop_restore)

>  #  pragma weak __pthread_initialize

>  #  pragma weak __pthread_atfork

>  #  pragma weak __pthread_setcancelstate

> -#  pragma weak _pthread_cleanup_push_defer

> -#  pragma weak _pthread_cleanup_pop_restore

>  # endif

>  #endif

>  

> diff --git a/sysdeps/nptl/pthread-functions.h b/sysdeps/nptl/pthread-functions.h

> index 97a5c48939..4268084b66 100644

> --- a/sysdeps/nptl/pthread-functions.h

> +++ b/sysdeps/nptl/pthread-functions.h

> @@ -57,10 +57,6 @@ struct pthread_functions

>    int (*ptr___pthread_key_create) (pthread_key_t *, void (*) (void *));

>    void *(*ptr___pthread_getspecific) (pthread_key_t);

>    int (*ptr___pthread_setspecific) (pthread_key_t, const void *);

> -  void (*ptr__pthread_cleanup_push_defer) (struct _pthread_cleanup_buffer *,

> -					   void (*) (void *), void *);

> -  void (*ptr__pthread_cleanup_pop_restore) (struct _pthread_cleanup_buffer *,

> -					    int);

>  #define HAVE_PTR_NTHREADS

>    unsigned int *ptr_nthreads;

>    void (*ptr___pthread_unwind) (__pthread_unwind_buf_t *)

> 


Ok.
Ben Woodard via Libc-alpha March 16, 2021, 7:03 a.m. | #2
* Adhemerval Zanella:

>> +#define __libc_cleanup_region_start(DOIT, FCT, ARG)			\

>> +  {   bool _cleanup_start_doit;						\

>> +  struct _pthread_cleanup_buffer _buffer;				\

>> +  /* Non-addresable copy of FCT, so that we avoid indirect calls on	\

>

> s/addresable/addressable


Thanks, fixed.

Florian

Patch

diff --git a/nptl/Versions b/nptl/Versions
index f2db649f9d..e3eb686a04 100644
--- a/nptl/Versions
+++ b/nptl/Versions
@@ -86,6 +86,8 @@  libc {
     __futex_abstimed_wait_cancelable64;
     __libc_alloca_cutoff;
     __libc_allocate_rtsig_private;
+    __libc_cleanup_pop_restore;
+    __libc_cleanup_push_defer;
     __libc_current_sigrtmax_private;
     __libc_current_sigrtmin_private;
     __libc_dl_error_tsd;
diff --git a/nptl/cleanup_defer_compat.c b/nptl/cleanup_defer_compat.c
index 49ef53ea60..1957318208 100644
--- a/nptl/cleanup_defer_compat.c
+++ b/nptl/cleanup_defer_compat.c
@@ -17,41 +17,15 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include "pthreadP.h"
-
+#include <libc-lock.h>
 
 void
 _pthread_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer,
 			     void (*routine) (void *), void *arg)
 {
-  struct pthread *self = THREAD_SELF;
-
   buffer->__routine = routine;
   buffer->__arg = arg;
-  buffer->__prev = THREAD_GETMEM (self, cleanup);
-
-  int cancelhandling = THREAD_GETMEM (self, cancelhandling);
-
-  /* Disable asynchronous cancellation for now.  */
-  if (__glibc_unlikely (cancelhandling & CANCELTYPE_BITMASK))
-    while (1)
-      {
-	int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling,
-						cancelhandling
-						& ~CANCELTYPE_BITMASK,
-						cancelhandling);
-	if (__glibc_likely (curval == cancelhandling))
-	  /* Successfully replaced the value.  */
-	  break;
-
-	/* Prepare for the next round.  */
-	cancelhandling = curval;
-      }
-
-  buffer->__canceltype = (cancelhandling & CANCELTYPE_BITMASK
-			  ? PTHREAD_CANCEL_ASYNCHRONOUS
-			  : PTHREAD_CANCEL_DEFERRED);
-
-  THREAD_SETMEM (self, cleanup, buffer);
+  __libc_cleanup_push_defer (buffer);
 }
 strong_alias (_pthread_cleanup_push_defer, __pthread_cleanup_push_defer)
 
@@ -60,31 +34,7 @@  void
 _pthread_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer,
 			      int execute)
 {
-  struct pthread *self = THREAD_SELF;
-
-  THREAD_SETMEM (self, cleanup, buffer->__prev);
-
-  int cancelhandling;
-  if (__builtin_expect (buffer->__canceltype != PTHREAD_CANCEL_DEFERRED, 0)
-      && ((cancelhandling = THREAD_GETMEM (self, cancelhandling))
-	  & CANCELTYPE_BITMASK) == 0)
-    {
-      while (1)
-	{
-	  int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling,
-						  cancelhandling
-						  | CANCELTYPE_BITMASK,
-						  cancelhandling);
-	  if (__glibc_likely (curval == cancelhandling))
-	    /* Successfully replaced the value.  */
-	    break;
-
-	  /* Prepare for the next round.  */
-	  cancelhandling = curval;
-	}
-
-      CANCELLATION_P (self);
-    }
+  __libc_cleanup_pop_restore (buffer);
 
   /* If necessary call the cleanup routine after we removed the
      current cleanup block from the list.  */
diff --git a/nptl/libc-cleanup.c b/nptl/libc-cleanup.c
index 61f97eceda..14ccfe9285 100644
--- a/nptl/libc-cleanup.c
+++ b/nptl/libc-cleanup.c
@@ -17,11 +17,69 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include "pthreadP.h"
+#include <tls.h>
+#include <libc-lock.h>
 
+void
+__libc_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer)
+{
+  struct pthread *self = THREAD_SELF;
+
+  buffer->__prev = THREAD_GETMEM (self, cleanup);
+
+  int cancelhandling = THREAD_GETMEM (self, cancelhandling);
+
+  /* Disable asynchronous cancellation for now.  */
+  if (__glibc_unlikely (cancelhandling & CANCELTYPE_BITMASK))
+    while (1)
+      {
+	int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling,
+						cancelhandling
+						& ~CANCELTYPE_BITMASK,
+						cancelhandling);
+	if (__glibc_likely (curval == cancelhandling))
+	  /* Successfully replaced the value.  */
+	  break;
+
+	/* Prepare for the next round.  */
+	cancelhandling = curval;
+      }
+
+  buffer->__canceltype = (cancelhandling & CANCELTYPE_BITMASK
+			  ? PTHREAD_CANCEL_ASYNCHRONOUS
+			  : PTHREAD_CANCEL_DEFERRED);
+
+  THREAD_SETMEM (self, cleanup, buffer);
+}
+libc_hidden_def (__libc_cleanup_push_defer)
 
 void
-__libc_cleanup_routine (struct __pthread_cleanup_frame *f)
+__libc_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer)
 {
-  if (f->__do_it)
-    f->__cancel_routine (f->__cancel_arg);
+  struct pthread *self = THREAD_SELF;
+
+  THREAD_SETMEM (self, cleanup, buffer->__prev);
+
+  int cancelhandling;
+  if (__builtin_expect (buffer->__canceltype != PTHREAD_CANCEL_DEFERRED, 0)
+      && ((cancelhandling = THREAD_GETMEM (self, cancelhandling))
+	  & CANCELTYPE_BITMASK) == 0)
+    {
+      while (1)
+	{
+	  int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling,
+						  cancelhandling
+						  | CANCELTYPE_BITMASK,
+						  cancelhandling);
+	  if (__glibc_likely (curval == cancelhandling))
+	    /* Successfully replaced the value.  */
+	    break;
+
+	  /* Prepare for the next round.  */
+	  cancelhandling = curval;
+	}
+
+      CANCELLATION_P (self);
+    }
 }
+libc_hidden_def (__libc_cleanup_pop_restore)
diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
index 865ee8db29..c2b563cc68 100644
--- a/nptl/nptl-init.c
+++ b/nptl/nptl-init.c
@@ -96,8 +96,6 @@  static const struct pthread_functions pthread_functions =
     .ptr___pthread_key_create = __pthread_key_create,
     .ptr___pthread_getspecific = __pthread_getspecific,
     .ptr___pthread_setspecific = __pthread_setspecific,
-    .ptr__pthread_cleanup_push_defer = __pthread_cleanup_push_defer,
-    .ptr__pthread_cleanup_pop_restore = __pthread_cleanup_pop_restore,
     .ptr_nthreads = &__nptl_nthreads,
     .ptr___pthread_unwind = &__pthread_unwind,
     .ptr__nptl_deallocate_tsd = __nptl_deallocate_tsd,
diff --git a/sysdeps/nptl/libc-lock.h b/sysdeps/nptl/libc-lock.h
index dea96121b3..56fe83f69f 100644
--- a/sysdeps/nptl/libc-lock.h
+++ b/sysdeps/nptl/libc-lock.h
@@ -143,39 +143,40 @@  typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t;
   __libc_maybe_call (__pthread_mutex_unlock, (&(NAME).mutex), 0)
 #endif
 
-/* Note that for I/O cleanup handling we are using the old-style
-   cancel handling.  It does not have to be integrated with C++ since
-   no C++ code is called in the middle.  The old-style handling is
-   faster and the support is not going away.  */
-extern void _pthread_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer,
-					 void (*routine) (void *), void *arg);
-extern void _pthread_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer,
-					  int execute);
+/* Put the unwind buffer BUFFER on the per-thread callback stack.  The
+   caller must fill BUFFER->__routine and BUFFER->__arg before calling
+   this function.  */
+void __libc_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer);
+libc_hidden_proto (__libc_cleanup_push_defer)
+/* Remove BUFFER from the unwind callback stack.  The caller must invoke
+   the callback if desired.  */
+void __libc_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer);
+libc_hidden_proto (__libc_cleanup_pop_restore)
 
 /* Start critical region with cleanup.  */
-#define __libc_cleanup_region_start(DOIT, FCT, ARG) \
-  { struct _pthread_cleanup_buffer _buffer;				      \
-    int _avail;								      \
-    if (DOIT) {								      \
-      _avail = PTFAVAIL (_pthread_cleanup_push_defer);			      \
-      if (_avail) {							      \
-	__libc_ptf_call_always (_pthread_cleanup_push_defer, (&_buffer, FCT,  \
-							      ARG));	      \
-      } else {								      \
-	_buffer.__routine = (FCT);					      \
-	_buffer.__arg = (ARG);						      \
-      }									      \
-    } else {								      \
-      _avail = 0;							      \
-    }
+#define __libc_cleanup_region_start(DOIT, FCT, ARG)			\
+  {   bool _cleanup_start_doit;						\
+  struct _pthread_cleanup_buffer _buffer;				\
+  /* Non-addresable copy of FCT, so that we avoid indirect calls on	\
+     the non-unwinding path.  */					\
+  void (*_cleanup_routine) (void *) = (FCT);				\
+  _buffer.__arg = (ARG);						\
+  if (DOIT)								\
+    {									\
+      _cleanup_start_doit = true;					\
+      _buffer.__routine = _cleanup_routine;				\
+      __libc_cleanup_push_defer (&_buffer);				\
+    }									\
+  else									\
+      _cleanup_start_doit = false;
 
 /* End critical region with cleanup.  */
-#define __libc_cleanup_region_end(DOIT) \
-    if (_avail) {							      \
-      __libc_ptf_call_always (_pthread_cleanup_pop_restore, (&_buffer, DOIT));\
-    } else if (DOIT)							      \
-      _buffer.__routine (_buffer.__arg);				      \
-  }
+#define __libc_cleanup_region_end(DOIT)		\
+  if (_cleanup_start_doit)			\
+    __libc_cleanup_pop_restore (&_buffer);	\
+  if (DOIT)					\
+    _cleanup_routine (_buffer.__arg);		\
+  } /* matches __libc_cleanup_region_start */
 
 
 /* Hide the definitions which are only supposed to be used inside libc in
diff --git a/sysdeps/nptl/libc-lockP.h b/sysdeps/nptl/libc-lockP.h
index 4a0b96e6d9..1a861b0d3f 100644
--- a/sysdeps/nptl/libc-lockP.h
+++ b/sysdeps/nptl/libc-lockP.h
@@ -251,32 +251,12 @@  _Static_assert (LLL_LOCK_INITIALIZER == 0, "LLL_LOCK_INITIALIZER != 0");
 /* Get once control variable.  */
 #define __libc_once_get(ONCE_CONTROL)	((ONCE_CONTROL) != PTHREAD_ONCE_INIT)
 
-/* Note that for I/O cleanup handling we are using the old-style
-   cancel handling.  It does not have to be integrated with C++ snce
-   no C++ code is called in the middle.  The old-style handling is
-   faster and the support is not going away.  */
-extern void _pthread_cleanup_push (struct _pthread_cleanup_buffer *buffer,
-				   void (*routine) (void *), void *arg);
-extern void _pthread_cleanup_pop (struct _pthread_cleanup_buffer *buffer,
-				  int execute);
-extern void _pthread_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer,
-					 void (*routine) (void *), void *arg);
-extern void _pthread_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer,
-					  int execute);
-
-/* Sometimes we have to exit the block in the middle.  */
-#define __libc_cleanup_end(DOIT) \
-    if (_avail) {							      \
-      __libc_ptf_call_always (_pthread_cleanup_pop_restore, (&_buffer, DOIT));\
-    } else if (DOIT)							      \
-      _buffer.__routine (_buffer.__arg)
-
 /* __libc_cleanup_push and __libc_cleanup_pop depend on exception
    handling and stack unwinding.  */
 #ifdef __EXCEPTIONS
 
 /* Normal cleanup handling, based on C cleanup attribute.  */
-__extern_inline void
+static __always_inline void
 __libc_cleanup_routine (struct __pthread_cleanup_frame *f)
 {
   if (f->__do_it)
@@ -396,8 +376,6 @@  weak_extern (__pthread_once)
 weak_extern (__pthread_initialize)
 weak_extern (__pthread_atfork)
 weak_extern (__pthread_setcancelstate)
-weak_extern (_pthread_cleanup_push_defer)
-weak_extern (_pthread_cleanup_pop_restore)
 # else
 #  pragma weak __pthread_mutex_init
 #  pragma weak __pthread_mutex_destroy
@@ -420,8 +398,6 @@  weak_extern (_pthread_cleanup_pop_restore)
 #  pragma weak __pthread_initialize
 #  pragma weak __pthread_atfork
 #  pragma weak __pthread_setcancelstate
-#  pragma weak _pthread_cleanup_push_defer
-#  pragma weak _pthread_cleanup_pop_restore
 # endif
 #endif
 
diff --git a/sysdeps/nptl/pthread-functions.h b/sysdeps/nptl/pthread-functions.h
index 97a5c48939..4268084b66 100644
--- a/sysdeps/nptl/pthread-functions.h
+++ b/sysdeps/nptl/pthread-functions.h
@@ -57,10 +57,6 @@  struct pthread_functions
   int (*ptr___pthread_key_create) (pthread_key_t *, void (*) (void *));
   void *(*ptr___pthread_getspecific) (pthread_key_t);
   int (*ptr___pthread_setspecific) (pthread_key_t, const void *);
-  void (*ptr__pthread_cleanup_push_defer) (struct _pthread_cleanup_buffer *,
-					   void (*) (void *), void *);
-  void (*ptr__pthread_cleanup_pop_restore) (struct _pthread_cleanup_buffer *,
-					    int);
 #define HAVE_PTR_NTHREADS
   unsigned int *ptr_nthreads;
   void (*ptr___pthread_unwind) (__pthread_unwind_buf_t *)