Ignore and remove LD_HWCAP_MASK for AT_SECURE programs (bug #21209)

  The LD_HWCAP_MASK environment variable may alter the selection of
function variants for some architectures.  For AT_SECURE process it
means that if an outdated routine has a bug that would otherwise not
affect newer platforms by default, LD_HWCAP_MASK will allow that bug
to be exploited.

To be on the safe side, ignore and disable LD_HWCAP_MASK for setuid
binaries.

	[BZ #21209]
	* elf/rtld.c (process_envvars): Ignore LD_HWCAP_MASK for
	AT_SECURE processes.
	* sysdeps/generic/unsecvars.h: Add LD_HWCAP_MASK.
	* elf/tst-env-setuid.c (test_parent): Test LD_HWCAP_MASK.
	(test_child): Likewise.
	* elf/Makefile (tst-env-setuid-ENV): Add LD_HWCAP_MASK.
---
 elf/Makefile                |  3 ++-
 elf/rtld.c                  |  3 ++-
 elf/tst-env-setuid.c        | 12 ++++++++++++
 sysdeps/generic/unsecvars.h |  1 +
 4 files changed, 17 insertions(+), 2 deletions(-)
  

On 03/02/2017 04:59 AM, Siddhesh Poyarekar wrote:
> The LD_HWCAP_MASK environment variable may alter the selection of
> function variants for some architectures.  For AT_SECURE process it
> means that if an outdated routine has a bug that would otherwise not
> affect newer platforms by default, LD_HWCAP_MASK will allow that bug
> to be exploited.
> 
> To be on the safe side, ignore and disable LD_HWCAP_MASK for setuid
> binaries.

Agreed.

> 	[BZ #21209]
> 	* elf/rtld.c (process_envvars): Ignore LD_HWCAP_MASK for
> 	AT_SECURE processes.
> 	* sysdeps/generic/unsecvars.h: Add LD_HWCAP_MASK.
> 	* elf/tst-env-setuid.c (test_parent): Test LD_HWCAP_MASK.
> 	(test_child): Likewise.
> 	* elf/Makefile (tst-env-setuid-ENV): Add LD_HWCAP_MASK.

What about LD_HWCAP_MASK usage in ldconfig when creating the cache?
It appears that this could alter the hwcap-keyed platform directory
selection and also alter exactly which paths are searched.

> ---
>  elf/Makefile                |  3 ++-
>  elf/rtld.c                  |  3 ++-
>  elf/tst-env-setuid.c        | 12 ++++++++++++
>  sysdeps/generic/unsecvars.h |  1 +
>  4 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 61abeb5..cc4aeb2 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -1398,6 +1398,7 @@ $(objpfx)tst-nodelete-dlclose: $(objpfx)tst-nodelete-dlclose-dso.so
>  $(objpfx)tst-nodelete-dlclose.out: $(objpfx)tst-nodelete-dlclose-dso.so \
>  				   $(objpfx)tst-nodelete-dlclose-plugin.so
>  
> -tst-env-setuid-ENV = MALLOC_CHECK_=2 MALLOC_MMAP_THRESHOLD_=4096
> +tst-env-setuid-ENV = MALLOC_CHECK_=2 MALLOC_MMAP_THRESHOLD_=4096 \
> +		     LD_HWCAP_MASK=0xffffffff

OK.

>  tst-env-setuid-tunables-ENV = \
>  	GLIBC_TUNABLES=glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096
> diff --git a/elf/rtld.c b/elf/rtld.c
> index a036ece..5986eaf 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -2404,7 +2404,8 @@ process_envvars (enum mode *modep)
>  
>  	case 10:
>  	  /* Mask for the important hardware capabilities.  */
> -	  if (memcmp (envline, "HWCAP_MASK", 10) == 0)
> +	  if (!__libc_enable_secure
> +	      && memcmp (envline, "HWCAP_MASK", 10) == 0)

OK.

>  	    GLRO(dl_hwcap_mask) = __strtoul_internal (&envline[11], NULL,
>  						      0, 0);
>  	  break;
> diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c
> index 6ec3fa5..eec408e 100644
> --- a/elf/tst-env-setuid.c
> +++ b/elf/tst-env-setuid.c
> @@ -213,6 +213,12 @@ test_child (void)
>        return 1;
>      }
>  
> +  if (getenv ("LD_HWCAP_MASK") != NULL)
> +    {
> +      printf ("LD_HWCAP_MASK still set\n");
> +      return 1;
> +    }

OK.

> +
>    return 0;
>  }
>  #endif
> @@ -233,6 +239,12 @@ test_parent (void)
>        return 1;
>      }
>  
> +  if (getenv ("LD_HWCAP_MASK") == NULL)
> +    {
> +      printf ("LD_HWCAP_MASK lost\n");
> +      return 1;
> +    }

Shouldn't this verify the value also?

> +
>    return 0;
>  }
>  #endif
> diff --git a/sysdeps/generic/unsecvars.h b/sysdeps/generic/unsecvars.h
> index a740837..5ea8a4a 100644
> --- a/sysdeps/generic/unsecvars.h
> +++ b/sysdeps/generic/unsecvars.h
> @@ -16,6 +16,7 @@
>    "LD_DEBUG\0"								      \
>    "LD_DEBUG_OUTPUT\0"							      \
>    "LD_DYNAMIC_WEAK\0"							      \
> +  "LD_HWCAP_MASK\0"							      \

OK.

>    "LD_LIBRARY_PATH\0"							      \
>    "LD_ORIGIN_PATH\0"							      \
>    "LD_PRELOAD\0"							      \
>
  

On Monday 06 March 2017 08:25 PM, Carlos O'Donell wrote:
> What about LD_HWCAP_MASK usage in ldconfig when creating the cache?
> It appears that this could alter the hwcap-keyed platform directory
> selection and also alter exactly which paths are searched.

Yes, but that should be OK.  It sets up cache for hwcaps and without and
the actual selection happens in dl-cache based on dl_hwcap_mask, which
will be correctly set.

Siddhesh