Re: [PATCH 3/8] Enable vDSO on interposed malloc linked statically

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

Ping.

On 05/09/2019 17:56, Adhemerval Zanella wrote:
> For statically linked binaries, _dl_non_dynamic_init (csu/init-first.c:74)
> calls malloc before the vDSO is properly initialized (it will fully usable
> only after VDSO_SETUP call).  The bug is triggered for the cases where an
> interposed malloc calls clock_gettime or any other implementation that may
> call a vDSO symbol.
> 
> It is because PTR_DEMANDLE at {INLINE,INTERNAL}_VSYSCALL is called with
> uninitialized __vdso_* pointer value, which should be done by PTR_MANGLE
> (zero-initialized value in this case is invalid).  The patch fixes by adding
> an extra hook before _dl_non_dynamic_init which setup the initial __vdso_*
> pointer symbol to an mangled NULL value.
> 
> For NULL mangled values {INLINE,INTERNAL}_VSYSCALL issues the syscall
> directly in fallback code path.  Once the __vdso_* is setup the vsyscall
> branches to vDSO.
> 
> Checked on x86_64-linux-gnu.
> 
> 	[BZ #24967]
> 	* sysdeps/unix/sysv/linux/init-first.c (libc_vdso_mangled_symbol):
> 	New symbol.
> 	(__libc_vdso_platform_setup): Add an option to call it to initialize
> 	the hooks to null values.
> 	(VDSO_PRE_SETUP): Define.
> 	* csu/init-first.c (_init): Add a pre-hook to initialize the vDSO
> 	internal pointer.
> 	* malloc/tst-interpose-aux.c (allocation_header): Add ts member.
> 	(malloc_internal): Call clock_gettime.
> ---
>  csu/init-first.c                     |  8 ++++++
>  malloc/tst-interpose-aux.c           |  5 ++++
>  sysdeps/unix/sysv/linux/init-first.c | 37 +++++++++++++++++++---------
>  3 files changed, 39 insertions(+), 11 deletions(-)
> 
> diff --git a/csu/init-first.c b/csu/init-first.c
> index 10762b61f5..d984419554 100644
> --- a/csu/init-first.c
> +++ b/csu/init-first.c
> @@ -69,6 +69,14 @@ _init (int argc, char **argv, char **envp)
>    __environ = envp;
>  
>  #ifndef SHARED
> +  /* Initialize the vDSO internal pointers to mangled zero value.  It makes
> +     the {INTERNAL,INLINE}_VSYSCALL macro to fallback to direct syscall
> +     and allows call the symbol that might the vDSO on _dl_non_dynamic_init
> +     (for instance clock_gettime on an interposed malloc).  */
> +# ifdef VDSO_PRE_SETUP
> +  VDSO_PRE_SETUP ();
> +# endif
> +
>    /* First the initialization which normally would be done by the
>       dynamic linker.  */
>    _dl_non_dynamic_init ();
> diff --git a/malloc/tst-interpose-aux.c b/malloc/tst-interpose-aux.c
> index bf86224401..de62d436bc 100644
> --- a/malloc/tst-interpose-aux.c
> +++ b/malloc/tst-interpose-aux.c
> @@ -28,6 +28,7 @@
>  #include <sys/mman.h>
>  #include <sys/uio.h>
>  #include <unistd.h>
> +#include <time.h>
>  
>  #if INTERPOSE_THREADS
>  #include <pthread.h>
> @@ -96,6 +97,7 @@ struct __attribute__ ((aligned (__alignof__ (max_align_t)))) allocation_header
>  {
>    size_t allocation_index;
>    size_t allocation_size;
> +  struct timespec ts;
>  };
>  
>  /* Array of known allocations, to track invalid frees.  */
> @@ -166,6 +168,9 @@ malloc_internal (size_t size)
>        .allocation_index = index,
>        .allocation_size = allocation_size
>      };
> +  /* Check if calling a symbol which may use the vDSO does not fail.
> +     The CLOCK_REALTIME should be supported on all systems.  */
> +  clock_gettime (CLOCK_REALTIME, &allocations[index]->ts);
>    return allocations[index] + 1;
>  }
>  
> diff --git a/sysdeps/unix/sysv/linux/init-first.c b/sysdeps/unix/sysv/linux/init-first.c
> index d90ca820be..a23e5d9f6e 100644
> --- a/sysdeps/unix/sysv/linux/init-first.c
> +++ b/sysdeps/unix/sysv/linux/init-first.c
> @@ -44,37 +44,52 @@ long int (*VDSO_SYMBOL(getcpu)) (unsigned *, unsigned *, void *)
>  time_t (*VDSO_SYMBOL(time)) (time_t *) attribute_hidden;
>  #endif
>  
> +static inline void *
> +__libc_vdso_mangled_symbol (const char *symbol)
> +{
> +  void *vdsop = NULL;
> +  if (symbol != NULL)
> +    vdsop = get_vdso_mangle_symbol (symbol);
> +  else
> +    PTR_MANGLE (vdsop);
> +  return vdsop;
> +}
> +
>  static inline void
> -__libc_vdso_platform_setup (void)
> +__libc_vdso_platform_setup (bool initial)
>  {
>  #ifdef HAVE_CLOCK_GETTIME_VSYSCALL
> -  VDSO_SYMBOL(clock_gettime)
> -    = get_vdso_mangle_symbol (HAVE_CLOCK_GETTIME_VSYSCALL);
> +  VDSO_SYMBOL(clock_gettime) =
> +    __libc_vdso_mangled_symbol (initial ? NULL : HAVE_CLOCK_GETTIME_VSYSCALL);
>  #endif
>  
>  #ifdef HAVE_CLOCK_GETRES_VSYSCALL
> -  VDSO_SYMBOL(clock_getres)
> -    = get_vdso_mangle_symbol (HAVE_CLOCK_GETRES_VSYSCALL);
> +  VDSO_SYMBOL(clock_getres) =
> +    __libc_vdso_mangled_symbol (initial ? NULL : HAVE_CLOCK_GETRES_VSYSCALL);
>  #endif
>  
>  #ifdef HAVE_GETTIMEOFDAY_VSYSCALL
> -  VDSO_SYMBOL(gettimeofday)
> -    = get_vdso_mangle_symbol (HAVE_GETTIMEOFDAY_VSYSCALL);
> +  VDSO_SYMBOL(gettimeofday) =
> +    __libc_vdso_mangled_symbol (initial ? NULL : HAVE_GETTIMEOFDAY_VSYSCALL);
>  #endif
>  
>  #ifdef HAVE_GETCPU_VSYSCALL
> -  VDSO_SYMBOL(getcpu) = get_vdso_mangle_symbol (HAVE_GETCPU_VSYSCALL);
> +  VDSO_SYMBOL(getcpu) =
> +    __libc_vdso_mangled_symbol (initial ? NULL : HAVE_GETCPU_VSYSCALL);
>  #endif
>  
>  #ifdef HAVE_TIME_VSYSCALL
> -  VDSO_SYMBOL(time) = get_vdso_mangle_symbol (HAVE_TIME_VSYSCALL);
> +  VDSO_SYMBOL(time) =
> +    __libc_vdso_mangled_symbol (initial ? NULL : HAVE_TIME_VSYSCALL);
>  #endif
>  
>  #ifdef VDSO_SETUP_ARCH
> -  VDSO_SETUP_ARCH ();
> +  if (!initial)
> +    VDSO_SETUP_ARCH ();
>  #endif
>  }
>  
> -#define VDSO_SETUP __libc_vdso_platform_setup
> +#define VDSO_PRE_SETUP() ({ __libc_vdso_platform_setup (true); })
> +#define VDSO_SETUP()     ({ __libc_vdso_platform_setup (false); })
>  
>  #include <csu/init-first.c>
>