Re: [PATCH] elf: Use the minimal malloc on tunables_strdup

[Siddhesh Poyarekar <siddhesh@sourceware.org>]

On 11/3/21 21:05, Adhemerval Zanella 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.
> ---
>   elf/Makefile                        |   7 +-
>   elf/dl-minimal-malloc.c             | 112 +++++++++++++++++++++++++
>   elf/dl-minimal.c                    | 122 ++--------------------------
>   elf/dl-tunables.c                   |   5 +-
>   sysdeps/generic/dl-minimal-malloc.h |  28 +++++++
>   5 files changed, 156 insertions(+), 118 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <assert.h>
> +#include <string.h>
> +#include <ldsodefs.h>
> +#include <malloc/malloc-internal.h>
> +
> +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
>      <https://www.gnu.org/licenses/>.  */
>   
> -#include <errno.h>
> -#include <limits.h>
> -#include <stdio.h>
> -#include <string.h>
> -#include <tls.h>
> -#include <unistd.h>
> -#include <sys/mman.h>
> -#include <sys/param.h>
> -#include <sys/types.h>
> +#include <assert.h>
>   #include <ldsodefs.h>
>   #include <dl-irel.h>
>   #include <dl-hash.h>
>   #include <dl-sym-post.h>
>   #include <_itoa.h>
> -#include <malloc/malloc-internal.h>
> +#include <dl-minimal-malloc.h>
>   
> -#include <assert.h>
>   
>   /* 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;
> -}
>   \f
>   /* Avoid signal frobnication in setjmp/longjmp.  Keeps things smaller.  */
>   
> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> index 1666736bc1..ee5b315e6a 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -31,6 +31,7 @@
>   #include <fcntl.h>
>   #include <ldsodefs.h>
>   #include <array_length.h>
> +#include <dl-minimal-malloc.h>
>   
>   #define TUNABLES_INTERNAL 1
>   #include "dl-tunables.h"
> @@ -48,12 +49,12 @@ 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)
> +  if (out == NULL)
>       _dl_fatal_printf ("sbrk() failure while processing tunables\n");

It's not a sbrk() failure anymore, this should now be "failed to 
allocate memory to process tunables"

>   
>     while (i-- > 0)
> diff --git a/sysdeps/generic/dl-minimal-malloc.h b/sysdeps/generic/dl-minimal-malloc.h
> new file mode 100644
> index 0000000000..cba5ed3225
> --- /dev/null
> +++ b/sysdeps/generic/dl-minimal-malloc.h
> @@ -0,0 +1,28 @@
> +/* Minimal malloc implementation for dynamic linker and static
> +   initialization.
> +   Copyright (C) 1995-2021 Free Software Foundation, Inc.

This is a new file, so just 2021.

> +   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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _DL_MINIMAL_MALLOC_H
> +#define _DL_MINIMAL_MALLOC_H
> +
> +void *__minimal_malloc (size_t n) attribute_hidden;
> +void *__minimal_calloc (size_t nmemb, size_t size) attribute_hidden;
> +void __minimal_free (void *ptr) attribute_hidden;
> +void *__minimal_realloc (void *ptr, size_t n) attribute_hidden;

extern for all of them?

> +
> +#endif
>