From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from crocodile.elm.relay.mailchannels.net (crocodile.elm.relay.mailchannels.net [23.83.212.45]) by sourceware.org (Postfix) with ESMTPS id 988073858401 for ; Tue, 9 Nov 2021 15:31:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 988073858401 X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 0CDE222665; Tue, 9 Nov 2021 15:31:48 +0000 (UTC) Received: from pdx1-sub0-mail-a306.dreamhost.com (100-96-18-144.trex.outbound.svc.cluster.local [100.96.18.144]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 830B9218C9; Tue, 9 Nov 2021 15:31:47 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a306.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.96.18.144 (trex/6.4.3); Tue, 09 Nov 2021 15:31:48 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Cure-Thoughtful: 7f316dfb745bd6ad_1636471907866_1779107067 X-MC-Loop-Signature: 1636471907866:4288089397 X-MC-Ingress-Time: 1636471907866 Received: from [192.168.1.174] (unknown [1.186.121.127]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a306.dreamhost.com (Postfix) with ESMTPSA id 4HpX5Y2Tb0z3T; Tue, 9 Nov 2021 07:31:44 -0800 (PST) Message-ID: <87003cce-709a-ef50-9ab5-546ddae06b08@sourceware.org> Date: Tue, 9 Nov 2021 21:01:40 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 Subject: Re: [PATCH v2] elf: Use the minimal malloc on tunables_strdup Content-Language: en-US To: Adhemerval Zanella , libc-alpha@sourceware.org, DJ Delorie References: <20211104131130.801849-1-adhemerval.zanella@linaro.org> From: Siddhesh Poyarekar In-Reply-To: <20211104131130.801849-1-adhemerval.zanella@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3496.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_SHORT, NICE_REPLY_A, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, RCVD_IN_SBL, SPF_HELO_NONE, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-List-Received-Date: Tue, 09 Nov 2021 15:31:53 -0000 On 11/4/21 18:41, Adhemerval Zanella via Libc-alpha wrote: > The rtld_malloc functions are moved to its own file so it can be > used on csu code. Also, the functiosn are renamed to __minimal_* > (since there are now used not only on loader code). > > Using the __minimal_malloc on tunables_strdup() avoids potential > issues with sbrk() calls while processing the tunables (I see > sporadic elf/tst-dso-ordering9 on powerpc64le with different > tests failing due ASLR). > > Also, using __minimal_malloc over plain mmap optimizes the memory > allocation on both static and dynamic case (since it will any unused > space in either the last page of data segments, avoiding mmap() call, > or from the previous mmap() call). > > Checked on x86_64-linux-gnu, i686-linux-gnu, and powerpc64le-linux-gnu. > --- > Changes from v1: > * Fixed memory allocation failure message. > * Fixed dl-minimal-malloc.h. LGTM Reviewed-by: Siddhesh Poyarekar > --- > elf/Makefile | 7 +- > elf/dl-minimal-malloc.c | 112 +++++++++++++++++++++++++ > elf/dl-minimal.c | 122 ++-------------------------- > elf/dl-tunables.c | 7 +- > sysdeps/generic/dl-minimal-malloc.h | 28 +++++++ > 5 files changed, 157 insertions(+), 119 deletions(-) > create mode 100644 elf/dl-minimal-malloc.c > create mode 100644 sysdeps/generic/dl-minimal-malloc.h > > diff --git a/elf/Makefile b/elf/Makefile > index 7e4f0c3121..7245309516 100644 > --- a/elf/Makefile > +++ b/elf/Makefile > @@ -36,7 +36,7 @@ dl-routines = $(addprefix dl-,load lookup object reloc deps \ > exception sort-maps lookup-direct \ > call-libc-early-init write \ > thread_gscope_wait tls_init_tp \ > - debug-symbols) > + debug-symbols minimal-malloc) > ifeq (yes,$(use-ldconfig)) > dl-routines += dl-cache > endif > @@ -75,6 +75,11 @@ CFLAGS-dl-runtime.c += -fexceptions -fasynchronous-unwind-tables > CFLAGS-dl-lookup.c += -fexceptions -fasynchronous-unwind-tables > CFLAGS-dl-iteratephdr.c += $(uses-callbacks) > > +# Called during static library initialization, so turn stack-protection > +# off for non-shared builds. > +CFLAGS-dl-minimal-malloc.o = $(no-stack-protector) > +CFLAGS-dl-minimal-malloc.op = $(no-stack-protector) > + > # On targets without __builtin_memset, rtld.c uses a hand-coded loop > # in _dl_start. Make sure this isn't turned into a call to regular memset. > ifeq (yes,$(have-loop-to-function)) > diff --git a/elf/dl-minimal-malloc.c b/elf/dl-minimal-malloc.c > new file mode 100644 > index 0000000000..939b5271ca > --- /dev/null > +++ b/elf/dl-minimal-malloc.c > @@ -0,0 +1,112 @@ > +/* Minimal malloc implementation for dynamic linker and static > + initialization. > + Copyright (C) 1995-2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#include > +#include > +#include > +#include > + > +static void *alloc_ptr, *alloc_end, *alloc_last_block; > + > +/* Allocate an aligned memory block. */ > +void * > +__minimal_malloc (size_t n) > +{ > + if (alloc_end == 0) > + { > + /* Consume any unused space in the last page of our data segment. */ > + extern int _end attribute_hidden; > + alloc_ptr = &_end; > + alloc_end = (void *) 0 + (((alloc_ptr - (void *) 0) > + + GLRO(dl_pagesize) - 1) > + & ~(GLRO(dl_pagesize) - 1)); > + } > + > + /* Make sure the allocation pointer is ideally aligned. */ > + alloc_ptr = (void *) 0 + (((alloc_ptr - (void *) 0) + MALLOC_ALIGNMENT - 1) > + & ~(MALLOC_ALIGNMENT - 1)); > + > + if (alloc_ptr + n >= alloc_end || n >= -(uintptr_t) alloc_ptr) > + { > + /* Insufficient space left; allocate another page plus one extra > + page to reduce number of mmap calls. */ > + caddr_t page; > + size_t nup = (n + GLRO(dl_pagesize) - 1) & ~(GLRO(dl_pagesize) - 1); > + if (__glibc_unlikely (nup == 0 && n != 0)) > + return NULL; > + nup += GLRO(dl_pagesize); > + page = __mmap (0, nup, PROT_READ|PROT_WRITE, > + MAP_ANON|MAP_PRIVATE, -1, 0); > + if (page == MAP_FAILED) > + return NULL; > + if (page != alloc_end) > + alloc_ptr = page; > + alloc_end = page + nup; > + } > + > + alloc_last_block = (void *) alloc_ptr; > + alloc_ptr += n; > + return alloc_last_block; > +} > + > +/* We use this function occasionally since the real implementation may > + be optimized when it can assume the memory it returns already is > + set to NUL. */ > +void * > +__minimal_calloc (size_t nmemb, size_t size) > +{ > + /* New memory from the trivial malloc above is always already cleared. > + (We make sure that's true in the rare occasion it might not be, > + by clearing memory in free, below.) */ > + size_t bytes = nmemb * size; > + > +#define HALF_SIZE_T (((size_t) 1) << (8 * sizeof (size_t) / 2)) > + if (__builtin_expect ((nmemb | size) >= HALF_SIZE_T, 0) > + && size != 0 && bytes / size != nmemb) > + return NULL; > + > + return malloc (bytes); > +} > + > +/* This will rarely be called. */ > +void > +__minimal_free (void *ptr) > +{ > + /* We can free only the last block allocated. */ > + if (ptr == alloc_last_block) > + { > + /* Since this is rare, we clear the freed block here > + so that calloc can presume malloc returns cleared memory. */ > + memset (alloc_last_block, '\0', alloc_ptr - alloc_last_block); > + alloc_ptr = alloc_last_block; > + } > +} > + > +/* This is only called with the most recent block returned by malloc. */ > +void * > +__minimal_realloc (void *ptr, size_t n) > +{ > + if (ptr == NULL) > + return malloc (n); > + assert (ptr == alloc_last_block); > + size_t old_size = alloc_ptr - alloc_last_block; > + alloc_ptr = alloc_last_block; > + void *new = malloc (n); > + return new != ptr ? memcpy (new, ptr, old_size) : new; > +} > diff --git a/elf/dl-minimal.c b/elf/dl-minimal.c > index 4e7f11aeab..152192d451 100644 > --- a/elf/dl-minimal.c > +++ b/elf/dl-minimal.c > @@ -16,23 +16,14 @@ > License along with the GNU C Library; if not, see > . */ > > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > +#include > #include > #include > #include > #include > #include <_itoa.h> > -#include > +#include > > -#include > > /* The rtld startup code calls __rtld_malloc_init_stubs after the > first self-relocation to adjust the pointers to the minimal > @@ -44,19 +35,13 @@ __typeof (free) *__rtld_free attribute_relro; > __typeof (malloc) *__rtld_malloc attribute_relro; > __typeof (realloc) *__rtld_realloc attribute_relro; > > -/* Defined below. */ > -static __typeof (calloc) rtld_calloc; > -static __typeof (free) rtld_free; > -static __typeof (malloc) rtld_malloc; > -static __typeof (realloc) rtld_realloc; > - > void > __rtld_malloc_init_stubs (void) > { > - __rtld_calloc = &rtld_calloc; > - __rtld_free = &rtld_free; > - __rtld_malloc = &rtld_malloc; > - __rtld_realloc = &rtld_realloc; > + __rtld_calloc = &__minimal_calloc; > + __rtld_free = &__minimal_free; > + __rtld_malloc = &__minimal_malloc; > + __rtld_realloc = &__minimal_realloc; > } > > bool > @@ -64,7 +49,7 @@ __rtld_malloc_is_complete (void) > { > /* The caller assumes that there is an active malloc. */ > assert (__rtld_malloc != NULL); > - return __rtld_malloc != &rtld_malloc; > + return __rtld_malloc != &__minimal_malloc; > } > > /* Lookup NAME at VERSION in the scope of MATCH. */ > @@ -115,99 +100,6 @@ __rtld_malloc_init_real (struct link_map *main_map) > __rtld_realloc = new_realloc; > } > > -/* Minimal malloc allocator for used during initial link. After the > - initial link, a full malloc implementation is interposed, either > - the one in libc, or a different one supplied by the user through > - interposition. */ > - > -static void *alloc_ptr, *alloc_end, *alloc_last_block; > - > -/* Allocate an aligned memory block. */ > -static void * > -rtld_malloc (size_t n) > -{ > - if (alloc_end == 0) > - { > - /* Consume any unused space in the last page of our data segment. */ > - extern int _end attribute_hidden; > - alloc_ptr = &_end; > - alloc_end = (void *) 0 + (((alloc_ptr - (void *) 0) > - + GLRO(dl_pagesize) - 1) > - & ~(GLRO(dl_pagesize) - 1)); > - } > - > - /* Make sure the allocation pointer is ideally aligned. */ > - alloc_ptr = (void *) 0 + (((alloc_ptr - (void *) 0) + MALLOC_ALIGNMENT - 1) > - & ~(MALLOC_ALIGNMENT - 1)); > - > - if (alloc_ptr + n >= alloc_end || n >= -(uintptr_t) alloc_ptr) > - { > - /* Insufficient space left; allocate another page plus one extra > - page to reduce number of mmap calls. */ > - caddr_t page; > - size_t nup = (n + GLRO(dl_pagesize) - 1) & ~(GLRO(dl_pagesize) - 1); > - if (__glibc_unlikely (nup == 0 && n != 0)) > - return NULL; > - nup += GLRO(dl_pagesize); > - page = __mmap (0, nup, PROT_READ|PROT_WRITE, > - MAP_ANON|MAP_PRIVATE, -1, 0); > - if (page == MAP_FAILED) > - return NULL; > - if (page != alloc_end) > - alloc_ptr = page; > - alloc_end = page + nup; > - } > - > - alloc_last_block = (void *) alloc_ptr; > - alloc_ptr += n; > - return alloc_last_block; > -} > - > -/* We use this function occasionally since the real implementation may > - be optimized when it can assume the memory it returns already is > - set to NUL. */ > -static void * > -rtld_calloc (size_t nmemb, size_t size) > -{ > - /* New memory from the trivial malloc above is always already cleared. > - (We make sure that's true in the rare occasion it might not be, > - by clearing memory in free, below.) */ > - size_t bytes = nmemb * size; > - > -#define HALF_SIZE_T (((size_t) 1) << (8 * sizeof (size_t) / 2)) > - if (__builtin_expect ((nmemb | size) >= HALF_SIZE_T, 0) > - && size != 0 && bytes / size != nmemb) > - return NULL; > - > - return malloc (bytes); > -} > - > -/* This will rarely be called. */ > -void > -rtld_free (void *ptr) > -{ > - /* We can free only the last block allocated. */ > - if (ptr == alloc_last_block) > - { > - /* Since this is rare, we clear the freed block here > - so that calloc can presume malloc returns cleared memory. */ > - memset (alloc_last_block, '\0', alloc_ptr - alloc_last_block); > - alloc_ptr = alloc_last_block; > - } > -} > - > -/* This is only called with the most recent block returned by malloc. */ > -void * > -rtld_realloc (void *ptr, size_t n) > -{ > - if (ptr == NULL) > - return malloc (n); > - assert (ptr == alloc_last_block); > - size_t old_size = alloc_ptr - alloc_last_block; > - alloc_ptr = alloc_last_block; > - void *new = malloc (n); > - return new != ptr ? memcpy (new, ptr, old_size) : new; > -} > > /* Avoid signal frobnication in setjmp/longjmp. Keeps things smaller. */ > > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > index 1666736bc1..497e948f1c 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -31,6 +31,7 @@ > #include > #include > #include > +#include > > #define TUNABLES_INTERNAL 1 > #include "dl-tunables.h" > @@ -48,13 +49,13 @@ tunables_strdup (const char *in) > size_t i = 0; > > while (in[i++] != '\0'); > - char *out = __sbrk (i); > + char *out = __minimal_malloc (i + 1); > > /* For most of the tunables code, we ignore user errors. However, > this is a system error - and running out of memory at program > startup should be reported, so we do. */ > - if (out == (void *)-1) > - _dl_fatal_printf ("sbrk() failure while processing tunables\n"); > + if (out == NULL) > + _dl_fatal_printf ("failed to allocate memory to process tunables\n"); > > while (i-- > 0) > out[i] = in[i]; > diff --git a/sysdeps/generic/dl-minimal-malloc.h b/sysdeps/generic/dl-minimal-malloc.h > new file mode 100644 > index 0000000000..7f50e52df5 > --- /dev/null > +++ b/sysdeps/generic/dl-minimal-malloc.h > @@ -0,0 +1,28 @@ > +/* Minimal malloc implementation for dynamic linker and static > + initialization. > + Copyright (C) 2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#ifndef _DL_MINIMAL_MALLOC_H > +#define _DL_MINIMAL_MALLOC_H > + > +extern void *__minimal_malloc (size_t n) attribute_hidden; > +extern void *__minimal_calloc (size_t nmemb, size_t size) attribute_hidden; > +extern void __minimal_free (void *ptr) attribute_hidden; > +extern void *__minimal_realloc (void *ptr, size_t n) attribute_hidden; > + > +#endif >