[PATCH] rtld: Check __libc_enable_secure before honoring LD_PREFER_MAP_32BIT_EXEC

[PATCH] rtld: Check __libc_enable_secure before honoring LD_PREFER_MAP_32BIT_EXEC

[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;
-- 
2.23.0

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

[Florian Weimer]

* 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

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

[Florian Weimer]

Given the patch is a clear improvement over what we have today, would
you please repost it with the following changes:

Please try to add CVE-2019-19126 to the commit subject, along with the
bug number.  The latter has to be of the form “(bug 25204)” or
“[BZ #25204]”, so that our tooling recongizes it.

Please mention the commit which introduced the flow in the commit
message, e.g. “commit b9eb92ab05204df772eb4929eccd018637c9f3e9
("Add Prefer_MAP_32BIT_EXEC to map executable pages with MAP_32BIT")”.

It would be nice if you could write a short entry for NEWS, in the
Security related changes section for the 2.31 release.

If you do not want to make these changes, I can make them for you before
pushing the patch.

Thanks,
Florian