Re: ppc64: Call to gettimeofday fails with segfault in __glink_PLTresolve because .plt0 is all zeros.

["Carlos O'Donell" <carlos at redhat dot com>]

On 11/07/2013 06:54 AM, Adhemerval Zanella wrote:
> After a intense debug session, Carlos narrowed down the culprit: vDSO are not attached
> in the process with an ODP section and the compiler/link does not create one using
> the trick at sysdeps/unix/sysv/linux/powerpc/bits/libc-vdso.h (it also does not make
> sense to create one, since it should be library itself to provide suck entries, not
> the modules which uses it). We checked on kernel code (I did also after dump the
> vDSO object from the process).
> 
> By not having an ODP associated the IFUNC resolver for gettimeofday returns an entry
> to BSS segment. The value for function descriptor would be correct pointing vDSO symbol,
> however the TOC value can be anything. In fact with older linker, according with the
> organization of symbols, it is indeed 0 leading to described issue. With newer linkers,
> that changed slight they hash function used to organize the symbol placement, the BSS
> place where the gettimeofday IFUNC resolver return is near the __cache_line_size and
> then the TOC is not null by accident. The current behavior is working by chance and
> the linker can break it by placing the symbol different in the BSS (such is the case
> of the issue related).
> 
> A way to fix it, as pointed by Carlos, would be make the vDSO get ODP entries for the
> symbols, then fixup _dl_vdso_vsym to return an OPD (which may or may not involve
> changes to dl_lookup_symbol_x and DL_SYMBOL_ADDRESS). Although it is possible, there are
> 2 issues with:
> 
> 1. Older kernel will still need some handling from GLIBC for the missing ODP entries.
> 2. It will require additional logic from userland in case for a new ABI which does not
>    require ODPs. I will check with kernel guys, but I see that ODP are not exported
>    to not tie userland with a specific ABI.
> 
> So, as also suggested by Carlos, I propose a more simple fix which is add static memory
> location to act as artificial ODP entry for the vDSO symbol returned in IFUNC resolver.
> The ODP entry is fill as the vDSO value (if it exists) its TOC is initialized to value
> different from 0. It is need so PLT resolution does not trigger lazy resolution and it
> works because none vDSO symbol uses the TOC. It also does not require any relocations.
> 
> I want to thanks Carlos O'Donnel for the much need help on this issue.

Thank you for helping!

I'm OK with this patch if you accept my comment suggestion.

> ---
> 
> 2013-11-07  Adhemerval Zanella  <azanella@linux.vnet.ibm.com>
> 
> 	* sysdeps/unix/sysv/linux/powerpc/bits/libc-vdso.h (VDSO_IFUNC_RET):
> 	Add artificial ODP entry for vDSO symbol for PPC64.
> 	* sysdeps/unix/sysv/linux/powerpc/gettimeofday.c: Adjust includes.
> 	* sysdeps/unix/sysv/linux/powerpc/time.c: Likewise.

OK.

> --
> 
> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/libc-vdso.h b/sysdeps/unix/sysv/linux/powerpc/bits/libc-vdso.h
> index ba54de4..70a8ca8 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/bits/libc-vdso.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/libc-vdso.h
> @@ -41,12 +41,23 @@ extern void *__vdso_sigtramp32;
>  extern void *__vdso_sigtramp_rt32;
>  #endif
>  
> -/* This macro is needed for PPC64 to return a skeleton OPD entry of a vDSO
> -   symbol.  This works because _dl_vdso_vsym always return the function
> -   address, and no vDSO symbols use the TOC or chain pointers from the OPD
> -   so we can allow them to be garbage.  */
>  #if defined(__PPC64__) || defined(__powerpc64__)
> -#define VDSO_IFUNC_RET(value)  ((void *) &(value))
> +/* vDSO symbols do not contain an associated OPD entry (the vDSO elf image
> +   does not even contain a .odp section). However, PLT relocations for IFUNC
> +   expects function resolvers to return an OPD address.
> +   By just blindly returning the __vdso_xxx value it ends up returning a
> +   pointer to BSS segment and it may eventually lead to an undefined behavior.
> +   This code creates an arficial OPD to act as the vDSO one. The TOC value
> +   is set to a non zero value to avoid triggering a lazy resolution through
> +   .glink0/.plt0 when the TOC is compared to 0 for thread-safe PLT routines
> +   (the vDSO symbol itself does not use the TCO or chain pointer).  */

Suggest:

/* The correct solution is for _dl_vdso_vsym to return the address of the OPD
   for the kernel VDSO function.  That address would then be stored in the
   __vdso_* variables and returned as the result of the IFUNC resolver function.
   Yet, the kernel does not contain any OPD entries for the VDSO functions
   (incomplete implementation).  However, PLT relocations for IFUNCs still expect
   the address of an OPD to be returned from the IFUNC resolver function (since
   PLT entries on PPC64 are just copies of OPDs).  The solution for now is to
   create an artificial static OPD for each VDSO function returned by a resolver
   function.  The TOC value is set to a non-zero value to avoid triggering lazy
   symbol resolution via .glink0/.plt0 for a zero TOC (requires thread-safe PLT
   sequences) when the dyanmic linker isn't prepared for it e.g. RTLD_NOW.  None
   of the kernel VDSO routines use the TOC or AUX values so any non-zero value
   will work.  Note that function pointer comparisons will not use this artificial
   static OPD since those are resolved via ADDR64 relocations and will point at
   the non-IFUNC default OPD for the symbol.  Lastly, because the IFUNC relocations
   are processed immediately at startup the resolver functions and this code need
   not be thread-safe, but if the caller writes to a PLT slot it must do so in a
   thread-safe manner with all the required barriers.  */

> +/* vDSO symbols do not contain an associated OPD entry (the vDSO elf image
> +   does not even contain a .odp section). However, PLT relocations for IFUNC
> +   expects function resolvers to return an OPD address.
> +   By just blindly returning the __vdso_xxx value it ends up returning a
> +   pointer to BSS segment and it may eventually lead to an undefined behavior.
> +   This code creates an arficial OPD to act as the vDSO one. The TOC value
> +   is set to a non zero value to avoid triggering a lazy resolution through
> +   .glink0/.plt0 when the TOC is compared to 0 for thread-safe PLT routines
> +   (the vDSO symbol itself does not use the TCO or chain pointer).  */

> +#define VDSO_IFUNC_RET(value)                            \
> +  ({                                                     \
> +    static Elf64_FuncDesc vdso_opd = { .fd_toc = ~0x0 }; \
> +    vdso_opd.fd_func = (Elf64_Addr)value;                \
> +    &vdso_opd;                                           \
> +  })
> +

OK.

>  #else
>  #define VDSO_IFUNC_RET(value)  ((void *) (value))
>  #endif
> diff --git a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
> index 6506d75..48c3f84 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
> @@ -22,6 +22,7 @@
>  
>  # include <dl-vdso.h>
>  # include <bits/libc-vdso.h>
> +# include <dl-machine.h>

OK.

>  void *gettimeofday_ifunc (void) __asm__ ("__gettimeofday");
>  
> diff --git a/sysdeps/unix/sysv/linux/powerpc/time.c b/sysdeps/unix/sysv/linux/powerpc/time.c
> index 66b4eb3..2d77ece 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/time.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/time.c
> @@ -20,7 +20,9 @@
>  
>  # include <time.h>
>  # include <sysdep.h>
> +# include <dl-vdso.h>
>  # include <bits/libc-vdso.h>
> +# include <dl-machine.h>
>  
>  void *time_ifunc (void) asm ("time");

OK.

Cheers,
Carlos.