From patchwork Thu May 6 18:11:05 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 43280 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 4A9E63AAB4B1; Thu, 6 May 2021 18:11:03 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4A9E63AAB4B1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1620324663; bh=Hnll44Wqudl8Ba8soljy1SbhNAhae0++ybEyjk9A448=; 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=toNTzAkHqrLfTo60sMmHmHELT0c8a1o3WEedba24hI91qkdc1CPp0N7SSO74vIPBo 9cX9Fj/TsLdvUMA9FiaW2r/ZSfQYn5fAn2/LoTv9imuPIVgv/WYw7ZUhJlw6ZiQjsy lINB6/rCwJFgRrSkJ0wypi2hBG0niOwI5AnKqiqU= 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 [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id E561B3AAB477 for ; Thu, 6 May 2021 18:10:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E561B3AAB477 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-548-U7OCPWtjPP-iPeC2149Qaw-1; Thu, 06 May 2021 14:10:54 -0400 X-MC-Unique: U7OCPWtjPP-iPeC2149Qaw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 455AAC73A5 for ; Thu, 6 May 2021 18:10:49 +0000 (UTC) Received: from oldenburg.str.redhat.com (ovpn-112-137.ams2.redhat.com [10.36.112.137]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 382E860871 for ; Thu, 6 May 2021 18:10:48 +0000 (UTC) To: libc-alpha@sourceware.org Subject: [PATCH 10/13] nptl: Move changing of stack permissions into ld.so In-Reply-To: References: Message-Id: Date: Thu, 06 May 2021 20:11:05 +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.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.8 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" All the stack lists are now in _rtld_global, so it is possible to change stack permissions directly from there, instead of calling into libpthread to do the change. Tested-by: Carlos O'Donell Reviewed-by: Carlos O'Donell --- elf/dl-load.c | 4 ++ elf/dl-support.c | 10 ++-- elf/rtld.c | 2 + nptl/allocatestack.c | 63 +-------------------- nptl/nptl-init.c | 4 -- nptl/pthreadP.h | 7 ++- sysdeps/generic/ldsodefs.h | 11 +++- sysdeps/unix/sysv/linux/Versions | 6 ++ sysdeps/unix/sysv/linux/dl-execstack.c | 76 +++++++++++++++++++++++--- 9 files changed, 100 insertions(+), 83 deletions(-) diff --git a/elf/dl-load.c b/elf/dl-load.c index 2832ab3540..918ec7546c 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -1368,7 +1368,11 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd, check_consistency (); #endif +#if PTHREAD_IN_LIBC + errval = _dl_make_stacks_executable (stack_endp); +#else errval = (*GL(dl_make_stack_executable_hook)) (stack_endp); +#endif if (errval) { errstring = N_("\ diff --git a/elf/dl-support.c b/elf/dl-support.c index 580b0202ad..dfc9ab760e 100644 --- a/elf/dl-support.c +++ b/elf/dl-support.c @@ -183,12 +183,6 @@ uint64_t _dl_hwcap_mask __attribute__ ((nocommon)); * executable but this isn't true for all platforms. */ ElfW(Word) _dl_stack_flags = DEFAULT_STACK_PERMS; -/* If loading a shared object requires that we make the stack executable - when it was not, we do it by calling this function. - It returns an errno code or zero on success. */ -int (*_dl_make_stack_executable_hook) (void **) = _dl_make_stack_executable; - - #if THREAD_GSCOPE_IN_TCB list_t _dl_stack_used; list_t _dl_stack_user; @@ -197,6 +191,10 @@ size_t _dl_stack_cache_actsize; uintptr_t _dl_in_flight_stack; int _dl_stack_cache_lock; #else +/* If loading a shared object requires that we make the stack executable + when it was not, we do it by calling this function. + It returns an errno code or zero on success. */ +int (*_dl_make_stack_executable_hook) (void **) = _dl_make_stack_executable; int _dl_thread_gscope_count; void (*_dl_init_static_tls) (struct link_map *) = &_dl_nothread_init_static_tls; #endif diff --git a/elf/rtld.c b/elf/rtld.c index 1255d5cc7d..fbbd60b446 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -1125,9 +1125,11 @@ dl_main (const ElfW(Phdr) *phdr, __tls_pre_init_tp (); +#if !PTHREAD_IN_LIBC /* The explicit initialization here is cheaper than processing the reloc in the _rtld_local definition's initializer. */ GL(dl_make_stack_executable_hook) = &_dl_make_stack_executable; +#endif /* Process the environment variable which control the behaviour. */ process_envvars (&state); diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 46089163f4..12cd1058d4 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -291,31 +291,6 @@ queue_stack (struct pthread *stack) free_stacks (stack_cache_maxsize); } - -static int -change_stack_perm (struct pthread *pd) -{ -#ifdef NEED_SEPARATE_REGISTER_STACK - size_t pagemask = __getpagesize () - 1; - void *stack = (pd->stackblock - + (((((pd->stackblock_size - pd->guardsize) / 2) - & pagemask) + pd->guardsize) & pagemask)); - size_t len = pd->stackblock + pd->stackblock_size - stack; -#elif _STACK_GROWS_DOWN - void *stack = pd->stackblock + pd->guardsize; - size_t len = pd->stackblock_size - pd->guardsize; -#elif _STACK_GROWS_UP - void *stack = pd->stackblock; - size_t len = (uintptr_t) pd - pd->guardsize - (uintptr_t) pd->stackblock; -#else -# error "Define either _STACK_GROWS_DOWN or _STACK_GROWS_UP" -#endif - if (__mprotect (stack, len, PROT_READ | PROT_WRITE | PROT_EXEC) != 0) - return errno; - - return 0; -} - /* Return the guard page position on allocated stack. */ static inline char * __attribute ((always_inline)) @@ -625,7 +600,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, if (__builtin_expect ((GL(dl_stack_flags) & PF_X) != 0 && (prot & PROT_EXEC) == 0, 0)) { - int err = change_stack_perm (pd); + int err = __nptl_change_stack_perm (pd); if (err != 0) { /* Free the stack memory we just allocated. */ @@ -780,42 +755,6 @@ __deallocate_stack (struct pthread *pd) lll_unlock (GL (dl_stack_cache_lock), LLL_PRIVATE); } - -int -__make_stacks_executable (void **stack_endp) -{ - /* First the main thread's stack. */ - int err = _dl_make_stack_executable (stack_endp); - if (err != 0) - return err; - - lll_lock (GL (dl_stack_cache_lock), LLL_PRIVATE); - - list_t *runp; - list_for_each (runp, &GL (dl_stack_used)) - { - err = change_stack_perm (list_entry (runp, struct pthread, list)); - if (err != 0) - break; - } - - /* Also change the permission for the currently unused stacks. This - might be wasted time but better spend it here than adding a check - in the fast path. */ - if (err == 0) - list_for_each (runp, &GL (dl_stack_cache)) - { - err = change_stack_perm (list_entry (runp, struct pthread, list)); - if (err != 0) - break; - } - - lll_unlock (GL (dl_stack_cache_lock), LLL_PRIVATE); - - return err; -} - - /* In case of a fork() call the memory allocation in the child will be the same but only one thread is running. All stacks except that of the one running thread are not used anymore. We have to recycle diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c index 2fb1117f3e..4c89e7a792 100644 --- a/nptl/nptl-init.c +++ b/nptl/nptl-init.c @@ -173,10 +173,6 @@ __pthread_initialize_minimal_internal (void) __default_pthread_attr.internal.guardsize = GLRO (dl_pagesize); lll_unlock (__default_pthread_attr_lock, LLL_PRIVATE); -#ifdef SHARED - GL(dl_make_stack_executable_hook) = &__make_stacks_executable; -#endif - /* Register the fork generation counter with the libc. */ __libc_pthread_init (__reclaim_stacks); } diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 8ab247f977..3a6b436400 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -335,8 +335,11 @@ extern void __deallocate_stack (struct pthread *pd) attribute_hidden; function also re-initializes the lock for the stack cache. */ extern void __reclaim_stacks (void) attribute_hidden; -/* Make all threads's stacks executable. */ -extern int __make_stacks_executable (void **stack_endp) attribute_hidden; +/* Change the permissions of a thread stack. Called from + _dl_make_stacks_executable and pthread_create. */ +int +__nptl_change_stack_perm (struct pthread *pd); +rtld_hidden_proto (__nptl_change_stack_perm) /* longjmp handling. */ extern void __pthread_cleanup_upto (__jmp_buf target, char *targetframe); diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h index 81cce2e4d5..8426b5cbd8 100644 --- a/sysdeps/generic/ldsodefs.h +++ b/sysdeps/generic/ldsodefs.h @@ -416,10 +416,12 @@ struct rtld_global #endif #include +#if !PTHREAD_IN_LIBC /* If loading a shared object requires that we make the stack executable when it was not, we do it by calling this function. It returns an errno code or zero on success. */ EXTERN int (*_dl_make_stack_executable_hook) (void **); +#endif /* Prevailing state of the stack, PF_X indicating it's executable. */ EXTERN ElfW(Word) _dl_stack_flags; @@ -717,10 +719,17 @@ extern const ElfW(Phdr) *_dl_phdr; extern size_t _dl_phnum; #endif +#if PTHREAD_IN_LIBC +/* This function changes the permissions of all stacks (not just those + of the main stack). */ +int _dl_make_stacks_executable (void **stack_endp) attribute_hidden; +#else /* This is the initial value of GL(dl_make_stack_executable_hook). - A threads library can change it. */ + A threads library can change it. The ld.so implementation changes + the permissions of the main stack only. */ extern int _dl_make_stack_executable (void **stack_endp); rtld_hidden_proto (_dl_make_stack_executable) +#endif /* Variable pointing to the end of the stack (or close to it). This value must be constant over the runtime of the application. Some programs diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions index c35f783e2a..220bb2dffe 100644 --- a/sysdeps/unix/sysv/linux/Versions +++ b/sysdeps/unix/sysv/linux/Versions @@ -181,3 +181,9 @@ libc { __netlink_assert_response; } } + +ld { + GLIBC_PRIVATE { + __nptl_change_stack_perm; + } +} diff --git a/sysdeps/unix/sysv/linux/dl-execstack.c b/sysdeps/unix/sysv/linux/dl-execstack.c index 3339138c42..e2449d1890 100644 --- a/sysdeps/unix/sysv/linux/dl-execstack.c +++ b/sysdeps/unix/sysv/linux/dl-execstack.c @@ -16,20 +16,21 @@ License along with the GNU C Library; if not, see . */ -#include -#include #include +#include #include -#include +#include +#include #include +#include +#include #include - +#include extern int __stack_prot attribute_relro attribute_hidden; - -int -_dl_make_stack_executable (void **stack_endp) +static int +make_main_stack_executable (void **stack_endp) { /* This gives us the highest/lowest page that needs to be changed. */ uintptr_t page = ((uintptr_t) *stack_endp @@ -56,4 +57,63 @@ _dl_make_stack_executable (void **stack_endp) return result; } -rtld_hidden_def (_dl_make_stack_executable) + +int +_dl_make_stacks_executable (void **stack_endp) +{ + /* First the main thread's stack. */ + int err = make_main_stack_executable (stack_endp); + if (err != 0) + return err; + + lll_lock (GL (dl_stack_cache_lock), LLL_PRIVATE); + + list_t *runp; + list_for_each (runp, &GL (dl_stack_used)) + { + err = __nptl_change_stack_perm (list_entry (runp, struct pthread, list)); + if (err != 0) + break; + } + + /* Also change the permission for the currently unused stacks. This + might be wasted time but better spend it here than adding a check + in the fast path. */ + if (err == 0) + list_for_each (runp, &GL (dl_stack_cache)) + { + err = __nptl_change_stack_perm (list_entry (runp, struct pthread, + list)); + if (err != 0) + break; + } + + lll_unlock (GL (dl_stack_cache_lock), LLL_PRIVATE); + + return err; +} + +int +__nptl_change_stack_perm (struct pthread *pd) +{ +#ifdef NEED_SEPARATE_REGISTER_STACK + size_t pagemask = __getpagesize () - 1; + void *stack = (pd->stackblock + + (((((pd->stackblock_size - pd->guardsize) / 2) + & pagemask) + pd->guardsize) & pagemask)); + size_t len = pd->stackblock + pd->stackblock_size - stack; +#elif _STACK_GROWS_DOWN + void *stack = pd->stackblock + pd->guardsize; + size_t len = pd->stackblock_size - pd->guardsize; +#elif _STACK_GROWS_UP + void *stack = pd->stackblock; + size_t len = (uintptr_t) pd - pd->guardsize - (uintptr_t) pd->stackblock; +#else +# error "Define either _STACK_GROWS_DOWN or _STACK_GROWS_UP" +#endif + if (__mprotect (stack, len, PROT_READ | PROT_WRITE | PROT_EXEC) != 0) + return errno; + + return 0; +} +rtld_hidden_def (__nptl_change_stack_perm)