From patchwork Fri Apr 16 09:20:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 42986 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E39AC3892021; Fri, 16 Apr 2021 09:20:21 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E39AC3892021 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1618564822; bh=7OqfzF6aUypQu7uEmRzlHewQH7/5zR5t7PyHJLmegmE=; h=To:Subject:In-Reply-To:References:Date:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=sijNB3mlnsgQdBiYPdaYNPeA3rAtGy6dgTsV5FXwcAZY/AFGE1D6I8taEHOP3DK73 MPc5Mmqj7XDhPoyMeeTfwWnZnHmvBhTFL18YS37asYCDcOhh9WghtCd9fdbxNFQTqY RGMaILuGzPIBVlEWJWb3frAfGQkKtiFX5Knm4xJw= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 2156E389200E for ; Fri, 16 Apr 2021 09:20:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2156E389200E Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-295-tVPsEsypPviSzGmkEMcNTw-1; Fri, 16 Apr 2021 05:20:11 -0400 X-MC-Unique: tVPsEsypPviSzGmkEMcNTw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A00DE83DD21 for ; Fri, 16 Apr 2021 09:20:10 +0000 (UTC) Received: from oldenburg.str.redhat.com (ovpn-113-139.ams2.redhat.com [10.36.113.139]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9435E50FA2 for ; Fri, 16 Apr 2021 09:20:09 +0000 (UTC) To: libc-alpha@sourceware.org Subject: [PATCH v4 03/37] nptl: Move legacy unwinding implementation into libc In-Reply-To: References: Message-Id: <96089e4a5014fe1c0ddcbf929118772f03618675.1618564630.git.fweimer@redhat.com> Date: Fri, 16 Apr 2021 11:20:27 +0200 User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Florian Weimer via Libc-alpha From: Florian Weimer Reply-To: Florian Weimer Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" 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. Reviewed-by: Adhemerval Zanella --- 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 60202b4969..2e5a964b11 100644 --- a/nptl/Versions +++ b/nptl/Versions @@ -87,6 +87,8 @@ libc { __futex_abstimed_wait64; __futex_abstimed_wait_cancelable64; __libc_alloca_cutoff; + __libc_cleanup_pop_restore; + __libc_cleanup_push_defer; __libc_dl_error_tsd; __libc_pthread_init; __lll_clocklock_elision; 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 @@ . */ #include "pthreadP.h" - +#include 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 @@ . */ #include "pthreadP.h" +#include +#include +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 2c7e2222d4..43e2564e59 100644 --- a/nptl/nptl-init.c +++ b/nptl/nptl-init.c @@ -95,8 +95,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..e8a5e68a12 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-addressable 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 63b605dee2..2c928b7a76 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) @@ -388,8 +368,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 @@ -412,8 +390,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 *)