[v2] elf: Use the minimal malloc on tunables_strdup

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.
---
 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

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 <siddhesh@sourceware.org>


> ---
>   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
> +   <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;
> -}
>   
>   /* 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 <fcntl.h>
>   #include <ldsodefs.h>
>   #include <array_length.h>
> +#include <dl-minimal-malloc.h>
>   
>   #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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#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
>

This seems to have caused build failures for many architectures (plus a 
localplt test failure for i686-gnu, "Missing required PLT reference: 
ld.so: __sbrk").

https://sourceware.org/pipermail/libc-testresults/2021q4/008843.html

alpha-linux-gnu seems representative of the build failures:

dl-minimal.c: In function '__chk_fail':
dl-minimal.c:168:3: error: implicit declaration of function '_exit' [-Werror=implicit-function-declaration]
  168 |   _exit (127);
      |   ^~~~~
dl-minimal.c:168:3: error: incompatible implicit declaration of built-in function '_exit' [-Werror=builtin-declaration-mismatch]
In file included from <command-line>:
dl-minimal.c: At top level:
./../include/libc-symbols.h:549:33: error: '__EI___libc_fatal' aliased to undefined symbol '__GI___libc_fatal'
  549 |   extern thread __typeof (name) __EI_##name \
      |                                 ^~~~~
./../include/libc-symbols.h:545:3: note: in expansion of macro '__hidden_ver2'
  545 |   __hidden_ver2 (, local, internal, name)
      |   ^~~~~~~~~~~~~
./../include/libc-symbols.h:554:41: note: in expansion of macro '__hidden_ver1'
  554 | #  define hidden_def(name)              __hidden_ver1(__GI_##name, name, name);
      |                                         ^~~~~~~~~~~~~
./../include/libc-symbols.h:646:32: note: in expansion of macro 'hidden_def'
  646 | # define rtld_hidden_def(name) hidden_def (name)
      |                                ^~~~~~~~~~
dl-minimal.c:162:1: note: in expansion of macro 'rtld_hidden_def'
  162 | rtld_hidden_def (__libc_fatal)
      | ^~~~~~~~~~~~~~~

> On 9 Nov 2021, at 20:09, Joseph Myers <joseph@codesourcery.com> wrote:
> 
> This seems to have caused build failures for many architectures (plus a 
> localplt test failure for i686-gnu, "Missing required PLT reference: 
> ld.so: __sbrk").
> 
> https://sourceware.org/pipermail/libc-testresults/2021q4/008843.html
> 
> alpha-linux-gnu seems representative of the build failures:
> 
> dl-minimal.c: In function '__chk_fail':
> dl-minimal.c:168:3: error: implicit declaration of function '_exit' [-Werror=implicit-function-declaration]
>  168 |   _exit (127);
>      |   ^~~~~
> dl-minimal.c:168:3: error: incompatible implicit declaration of built-in function '_exit' [-Werror=builtin-declaration-mismatch]
> In file included from <command-line>:
> dl-minimal.c: At top level:
> ./../include/libc-symbols.h:549:33: error: '__EI___libc_fatal' aliased to undefined symbol '__GI___libc_fatal'
>  549 |   extern thread __typeof (name) __EI_##name \
>      |                                 ^~~~~
> ./../include/libc-symbols.h:545:3: note: in expansion of macro '__hidden_ver2'
>  545 |   __hidden_ver2 (, local, internal, name)
>      |   ^~~~~~~~~~~~~
> ./../include/libc-symbols.h:554:41: note: in expansion of macro '__hidden_ver1'
>  554 | #  define hidden_def(name)              __hidden_ver1(__GI_##name, name, name);
>      |                                         ^~~~~~~~~~~~~
> ./../include/libc-symbols.h:646:32: note: in expansion of macro 'hidden_def'
>  646 | # define rtld_hidden_def(name) hidden_def (name)
>      |                                ^~~~~~~~~~
> dl-minimal.c:162:1: note: in expansion of macro 'rtld_hidden_def'
>  162 | rtld_hidden_def (__libc_fatal)
>      | ^~~~~~~~~~~~~~~
> 

I will take care of it.