Re: [PATCH] rtld: Check __libc_enable_secure before honoring LD_PREFER_MAP_32BIT_EXEC

[Florian Weimer <fweimer at redhat dot com>]

* Marcin Kościelnicki:

> From: Marcin Kościelnicki <mwk@0x04.net>
>
> Fixes #25204.
> ---
>  sysdeps/unix/sysv/linux/x86_64/64/dl-librecon.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/unix/sysv/linux/x86_64/64/dl-librecon.h b/sysdeps/unix/sysv/linux/x86_64/64/dl-librecon.h
> index 0e95221908..e3af239faa 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/64/dl-librecon.h
> +++ b/sysdeps/unix/sysv/linux/x86_64/64/dl-librecon.h
> @@ -31,7 +31,8 @@
>     environment variable, LD_PREFER_MAP_32BIT_EXEC.  */
>  #define EXTRA_LD_ENVVARS \
>    case 21:								  \
> -    if (memcmp (envline, "PREFER_MAP_32BIT_EXEC", 21) == 0)		  \
> +    if (!__libc_enable_secure						  \
> +	&& memcmp (envline, "PREFER_MAP_32BIT_EXEC", 21) == 0)		  \
>        GLRO(dl_x86_cpu_features).feature[index_arch_Prefer_MAP_32BIT_EXEC] \
>  	|= bit_arch_Prefer_MAP_32BIT_EXEC;				  \
>      break;

This change looks correct in isolation, however I have not been able to
verify that LD_PREFER_MAP_32BIT_EXEC has any effect with current
glibc/binutils versions.

I believe this is the result of the initial (full) mapping of an object
inherting the premissions of the first load segment, and this is now
read-only:

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  PHDR           0x0000000000000040 0x0000000000000040 0x0000000000000040
                 0x00000000000002a0 0x00000000000002a0  R      0x8
  INTERP         0x00000000000002e0 0x00000000000002e0 0x00000000000002e0
                 0x000000000000001c 0x000000000000001c  R      0x1
      [Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]
  LOAD           0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x000000000002ce90 0x000000000002ce90  R      0x1000
  LOAD           0x000000000002d000 0x000000000002d000 0x000000000002d000
                 0x00000000000b04a5 0x00000000000b04a5  R E    0x1000
  LOAD           0x00000000000de000 0x00000000000de000 0x00000000000de000
                 0x0000000000036108 0x0000000000036108  R      0x1000
  LOAD           0x0000000000114cd0 0x0000000000115cd0 0x0000000000115cd0
                 0x000000000000b934 0x00000000000155e8  RW     0x1000
  DYNAMIC        0x00000000001175f0 0x00000000001185f0 0x00000000001185f0
                 0x0000000000000210 0x0000000000000210  RW     0x8
  NOTE           0x0000000000000300 0x0000000000000300 0x0000000000000300
                 0x0000000000000020 0x0000000000000020  R      0x8
  NOTE           0x0000000000000320 0x0000000000000320 0x0000000000000320
                 0x0000000000000044 0x0000000000000044  R      0x4
  GNU_EH_FRAME   0x00000000000f7f58 0x00000000000f7f58 0x00000000000f7f58
                 0x00000000000044ec 0x00000000000044ec  R      0x4
  GNU_STACK      0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000  RW     0x10
  GNU_RELRO      0x0000000000114cd0 0x0000000000115cd0 0x0000000000115cd0
                 0x0000000000003330 0x0000000000003330  R      0x1

It used to be read-execute.

With a read-only mapping, the Prefer_MAP_32BIT_EXEC override in
sysdeps/unix/sysv/linux/x86_64/64/mmap_internal.h does not kick in.

My preference would be to remove this feature and redo it in the right
way.  It does not work for modern distributions with PIE anyway (in the
sense the you won't get 2 GiB offsets between executable and libraries),
and fixing that needs kernel support.

Thanks,
Florian