Re: [PATCH v7 12/16] elf: Fix initial-exec TLS access on audit modules (BZ #28096)

[Florian Weimer <fweimer@redhat.com>]

* Adhemerval Zanella:

> For audit modules or dependencies with initial-exec TLS, we can not
> set the initial TLS image on default loader initialization because it
> would already be set by the audit setup.  However, subsequent thread
> creation would need to follow the default behaviour.
>
> This patch fixes by making_dl_new_object sets a new link_map flag
> 'l_dont_set_tls_static' for namespaces different than the default
> one.  The flag is later reset on _dl_allocate_tls_init.

Typo: making[ ]dl_new-object

> diff --git a/elf/dl-object.c b/elf/dl-object.c
> index 1875599eb2..f3d637ac27 100644
> --- a/elf/dl-object.c
> +++ b/elf/dl-object.c
> @@ -175,6 +175,9 @@ _dl_new_object (char *realname, const char *libname, int type,
>  
>    new->l_local_scope[0] = &new->l_searchlist;
>  
> +  if (nsid != LM_ID_BASE)
> +    new->l_dont_set_tls_static = 1;
> +
>    /* Determine the origin.  If allocating the link map for the main
>       executable, the realname is not known and "".  In this case, the
>       origin needs to be determined by other means.  However, in case
> diff --git a/elf/dl-tls.c b/elf/dl-tls.c
> index 273f60f233..89200ccc48 100644
> --- a/elf/dl-tls.c
> +++ b/elf/dl-tls.c
> @@ -593,10 +593,18 @@ _dl_allocate_tls_init (void *result)
>  	     some platforms use in static programs requires it.  */
>  	  dtv[map->l_tls_modid].pointer.val = dest;
>  
> -	  /* Copy the initialization image and clear the BSS part.  */
> -	  memset (__mempcpy (dest, map->l_tls_initimage,
> -			     map->l_tls_initimage_size), '\0',
> -		  map->l_tls_blocksize - map->l_tls_initimage_size);
> +	  /* Copy the initialization image and clear the BSS part.  For
> +	     audit modules or depedencies with initial-exec TLS, we can not
> +	     set the initial TLS image on default loader initialization
> +	     because it would already be set by the audit setup.  However,
> +	     subsequent thread creation would need to follow the default
> +	     behaviour.   */
> +	  if (__glibc_unlikely (!map->l_dont_set_tls_static))
> +	    memset (__mempcpy (dest, map->l_tls_initimage,
> +			       map->l_tls_initimage_size), '\0',
> +		    map->l_tls_blocksize - map->l_tls_initimage_size);
> +	  else
> +	    map->l_dont_set_tls_static = 0;
>  	}

Wouldn't it be simpler to skip the memcpy/memset for non-base namespaces
if a flag is pased to _dl_allocate_tls_init?  I still don't think it's
the status as an audit module what matters here, it applies to
everything that is loaded through the dlopen implementation (as opposed
to _dl_map_object_deps for the main executable).

Thanks,
Florian