Re: [PATCH] elf: Remove __libc_enable_secure_decided

[Florian Weimer <fweimer@redhat.com>]

* Fangrui Song:

> No functional change. __libc_enable_secure_decided is always 0 since
> 73fc4e28b9464f0e13edc719a5372839970e7ddb.
> ---
>  elf/enbl-secure.c | 7 ++-----
>  include/unistd.h  | 1 -
>  2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/elf/enbl-secure.c b/elf/enbl-secure.c
> index aa2a0bd877..6a0a6d0f0f 100644
> --- a/elf/enbl-secure.c
> +++ b/elf/enbl-secure.c
> @@ -26,15 +26,12 @@
>  #include <startup.h>
>  #include <libc-internal.h>
>  
> -/* If nonzero __libc_enable_secure is already set.  */
> -int __libc_enable_secure_decided;
>  /* Safest assumption, if somehow the initializer isn't run.  */
>  int __libc_enable_secure = 1;
>  
>  void
>  __libc_init_secure (void)
>  {
> -  if (__libc_enable_secure_decided == 0)
> -    __libc_enable_secure = (startup_geteuid () != startup_getuid ()
> -			    || startup_getegid () != startup_getgid ());
> +  __libc_enable_secure = (startup_geteuid () != startup_getuid ()
> +			  || startup_getegid () != startup_getgid ());
>  }

Thanks for catching this in time before a release, this would have
turned into a minor security vulnerability.

__libc_init_secure should not overwrite __libc_enable_secure on Linux
because _dl_aux_init in dl-support.c already initializes it, and the
real vs effective check does not always yield the right results (it
exits AT_SECURE mode for a pure capabilities-based transition, for
example).

Hurd already overrides __libc_init_secure with an empty function.  I
think we should remove it completely.  Then we can also simply
<startup.h> a bit, I think.  Would you be able to write a patch along
those lines, or should I work on this?

Thanks,
Florian