[PATCH 0/3] Mips support for PT_GNU_STACK

[PATCH 0/3] Mips support for PT_GNU_STACK

[Dragan Mladjenovic]

Hello everyone,

Patches in this series are slight variation of work done previously by Faraz Shahbazker [1]
in 2016. A brief summary of the issue this is trying to address:

Up until the Linux kernel version 4.8 [2] MIPS FPU emulator used a small trampoline created on
user stack to handle delay slots when emulating FPU branches. Because of this non-executable stack
could not be enabled by default on MIPS. The compatibility issue is that these old kernels respect
PT_GNU_STACK, making the stack non-executable if requested, and could crash the user process if there
would be need to emulate a instruction in the delay slot of a FPU branch.

In order to allow for the tool-chain to safely use PT_GNU_STACK by default and to provide the
compatibility with pre-4.8 kernels, original patch would revert stack protection back to executable
stack if it could not detect that kernel supports non-executable stack.

The form of detection the patch proposes is not yet provided by the kernel. Instead, this version of
the patch does kernel version check at runtime and provides compatible behavior if it cannot detect
the 4.8 kernel or newer.
 
The last patch increments the ABI Version number in order to disallow new binaries to run with older glibc.
The number is not set in stone. I'm assuming it will probably land after GNU_HASH [3] support which consumes
ABI version 5 for MIPS. I will send proposal for Binutils and GCC after this part gets finalized.

Even if this part doesn't get in the next release due to issue [4] with ABI version handling.
It would be still nice if the back-compat support gets in. I would like to hear your thoughts on this.

Best regards,

Dragan

[1] https://sourceware.org/ml/libc-alpha/2016-02/msg00076.html
[2] https://github.com/torvalds/linux/commit/432c6bacbd0c16ec210c43da411ccc3855c4c010
[3] https://sourceware.org/ml/libc-alpha/2019-06/msg00456.html
[4] https://sourceware.org/ml/libc-alpha/2019-06/msg00730.html


Dragan Mladjenovic (3):
  [ELF] Allow the machine to override stack permissions via
    USE_DL_EXEC_STACK_OVERRIDE.
  [MIPS] Define USE_DL_EXEC_STACK_OVERRIDE on Mips
  [RFC][MIPS] Define GNU_STACK ABI

 elf/dl-load.c                                      | 10 +++++
 elf/dl-support.c                                   |  8 +++-
 sysdeps/generic/ldsodefs.h                         |  4 ++
 sysdeps/unix/sysv/linux/mips/Makefile              | 28 +++++++++++--
 sysdeps/unix/sysv/linux/mips/configure.ac          |  2 +
 sysdeps/unix/sysv/linux/mips/dl-execstack-ovrd.c   | 48 ++++++++++++++++++++++
 sysdeps/unix/sysv/linux/mips/dl-sysdep.h           | 28 +++++++++++++
 sysdeps/unix/sysv/linux/mips/ldsodefs.h            |  2 +-
 sysdeps/unix/sysv/linux/mips/libc-abis             |  2 +
 .../sysv/linux/mips/tst-execstack-ovrd-static.c    |  1 +
 sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd.c  |  2 +
 .../sysv/linux/mips/tst-execstack-ovrd1-static.c   |  1 +
 sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1.c | 11 +++++
 13 files changed, 141 insertions(+), 6 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/mips/dl-execstack-ovrd.c
 create mode 100644 sysdeps/unix/sysv/linux/mips/dl-sysdep.h
 create mode 100644 sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd-static.c
 create mode 100644 sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd.c
 create mode 100644 sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1-static.c
 create mode 100644 sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1.c

-- 
1.9.1

[PATCH 1/3] [ELF] Allow the machine to override stack permissions via USE_DL_EXEC_STACK_OVERRIDE.

[Dragan Mladjenovic]

This patch allows the machine-dependent code to override non-executable stack
permissions by defining USE_DL_EXEC_STACK_OVERRIDE and implementing _dl_exec_stack_override.
It is called early during the static startup after the os version check and during the load
of every shared library or main executable if ld.so is invoked explicitly.

	* elf/dl-load.c (_dl_map_object_from_fd): Call '_dl_exec_stack_override'.
	* elf/dl-support.c (_dl_non_dynamic_init): Likewise.
	* sysdeps/generic/ldsodefs.h (_dl_exec_stack_override): New prototype.
---
 elf/dl-load.c              | 10 ++++++++++
 elf/dl-support.c           |  8 +++++++-
 sysdeps/generic/ldsodefs.h |  4 ++++
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/elf/dl-load.c b/elf/dl-load.c
index 5abeb86..9155b74 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1242,6 +1242,16 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
     /* Adjust the PT_PHDR value by the runtime load address.  */
     l->l_phdr = (ElfW(Phdr) *) ((ElfW(Addr)) l->l_phdr + l->l_addr);
 
+#ifdef USE_DL_EXEC_STACK_OVERRIDE
+  /* Program requests a non-executable stack, but architecture does
+     not support it.  */
+  if (__glibc_unlikely (_dl_exec_stack_override (&stack_flags) != 0))
+    {
+      errstring = N_("cannot override stack memory protections");
+      goto call_lose_errno;
+    }
+#endif
+
   if (__glibc_unlikely ((stack_flags &~ GL(dl_stack_flags)) & PF_X))
     {
       /* The stack is presently not executable, but this module
diff --git a/elf/dl-support.c b/elf/dl-support.c
index 0a8b636..dd99d58 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -179,7 +179,6 @@ ElfW(Word) _dl_stack_flags = DEFAULT_STACK_PERMS;
    It returns an errno code or zero on success.  */
 int (*_dl_make_stack_executable_hook) (void **) = _dl_make_stack_executable;
 
-
 /* Function in libpthread to wait for termination of lookups.  */
 void (*_dl_wait_lookup_done) (void);
 
@@ -375,6 +374,13 @@ _dl_non_dynamic_init (void)
 	  _dl_stack_flags = _dl_phdr[i].p_flags;
 	  break;
 	}
+
+#ifdef USE_DL_EXEC_STACK_OVERRIDE
+  if (__glibc_unlikely (_dl_exec_stack_override (&_dl_stack_flags) != 0))
+    {
+      _dl_fatal_printf ("cannot override stack memory protections\n");
+    }
+#endif
 }
 
 #ifdef DL_SYSINFO_IMPLEMENTATION
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index b1fc5c3..4e1f0f1 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -642,6 +642,10 @@ extern size_t _dl_phnum;
 extern int _dl_make_stack_executable (void **stack_endp);
 rtld_hidden_proto (_dl_make_stack_executable)
 
+#ifdef USE_DL_EXEC_STACK_OVERRIDE
+extern int _dl_exec_stack_override (void *);
+rtld_hidden_proto (_dl_exec_stack_override)
+#endif
 /* Variable pointing to the end of the stack (or close to it).  This value
    must be constant over the runtime of the application.  Some programs
    might use the variable which results in copy relocations on some
-- 
1.9.1

[PATCH 2/3] [MIPS] Define USE_DL_EXEC_STACK_OVERRIDE on Mips

[Dragan Mladjenovic]

This patch conditionally defines USE_DL_EXEC_STACK_OVERRIDE for hard-float builds
targeting minimum Linux kernel version lower than 4.8. In that case _dl_exec_stack_override
performs run-time check of kernel version and enforces executable stack on pre-4.8 kernels.

We now detect when glibc is built with toolchain that uses GNU.stack notes and xfail the
check-execstack only if one isn't used.

	* sysdeps/unix/sysv/linux/mips/Makefile[$(subdir) == elf] (sysdep-dl-routines):
	Add dl-execstack-ovrd.
	(tests): Add tst-execstack-ovrd and tst-execstack-ovrd1.
	(tests-static): Add tst-execstack-ovrd-static and tst-execstack-ovrd1-static.
	(LDFLAGS-tst-execstack-ovrd*, tst-execstack-ovrd*-ENV ...): New.
	(test-xfail-check-execstack): Enable when mips-has-gnustack is false.
	(test-xfail-tst-execstack-ovrd1, test-xfail-tst-execstack-ovrd1-static): New.
	Likewise enabled.
	* sysdeps/unix/sysv/linux/mips/configure.ac (mips-has-gnustack): New var.
	Set to value of libc_cv_as_noexecstack.
	* sysdeps/unix/sysv/linux/mips/configure: Regenerated.
	* sysdeps/unix/sysv/linux/mips/dl-execstack-ovrd.c: New file.
	* sysdeps/unix/sysv/linux/mips/dl-sysdep.h: New file.
	* sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd-static.c: New file.
	* sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd.c: New file.
	* sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1-static.c: New file.
	* sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1.c: New file.
---
 sysdeps/unix/sysv/linux/mips/Makefile              | 28 +++++++++++--
 sysdeps/unix/sysv/linux/mips/configure.ac          |  2 +
 sysdeps/unix/sysv/linux/mips/dl-execstack-ovrd.c   | 48 ++++++++++++++++++++++
 sysdeps/unix/sysv/linux/mips/dl-sysdep.h           | 28 +++++++++++++
 .../sysv/linux/mips/tst-execstack-ovrd-static.c    |  1 +
 sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd.c  |  2 +
 .../sysv/linux/mips/tst-execstack-ovrd1-static.c   |  1 +
 sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1.c | 11 +++++
 8 files changed, 117 insertions(+), 4 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/mips/dl-execstack-ovrd.c
 create mode 100644 sysdeps/unix/sysv/linux/mips/dl-sysdep.h
 create mode 100644 sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd-static.c
 create mode 100644 sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd.c
 create mode 100644 sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1-static.c
 create mode 100644 sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1.c

diff --git a/sysdeps/unix/sysv/linux/mips/Makefile b/sysdeps/unix/sysv/linux/mips/Makefile
index 8217f42..4f2f422 100644
--- a/sysdeps/unix/sysv/linux/mips/Makefile
+++ b/sysdeps/unix/sysv/linux/mips/Makefile
@@ -57,6 +57,8 @@ abi-n64_hard_2008-condition := defined(__mips_nan2008) \
 			       && (_MIPS_SIM == _MIPS_SIM_ABI64)
 
 ifeq ($(subdir),elf)
+sysdep-dl-routines += dl-execstack-ovrd
+
 ifeq ($(build-shared),yes)
 # This is needed for DSO loading from static binaries.
 sysdep-dl-routines += dl-static
@@ -64,11 +66,29 @@ sysdep-dl-routines += dl-static
 sysdep_routines += dl-vdso
 endif
 
-# Supporting non-executable stacks on MIPS requires changes to both
-# the Linux kernel and glibc.  See
-# <https://sourceware.org/ml/libc-alpha/2016-01/msg00567.html> and
-# <https://sourceware.org/ml/libc-alpha/2016-01/msg00719.html>.
+tests-static += tst-execstack-ovrd-static
+tests += tst-execstack-ovrd-static
+tests += tst-execstack-ovrd
+tests-static += tst-execstack-ovrd1-static
+tests += tst-execstack-ovrd1-static
+tests += tst-execstack-ovrd1
+LDFLAGS-tst-execstack-ovrd = -Wl,-z,noexecstack
+LDFLAGS-tst-execstack-ovrd-static = -Wl,-z,noexecstack
+LDFLAGS-tst-execstack-ovrd1 = -Wl,-z,noexecstack
+LDFLAGS-tst-execstack-ovrd1-static = -Wl,-z,noexecstack
+tst-execstack-ovrd-ENV = LD_ASSUME_KERNEL=4.5.0
+tst-execstack-ovrd-static-ENV = LD_ASSUME_KERNEL=4.5.0
+tst-execstack-ovrd1-ENV = LD_ASSUME_KERNEL=4.8.0
+tst-execstack-ovrd1-static-ENV = LD_ASSUME_KERNEL=4.8.0
+
+# If the compiler doesn't use GNU.stack note,
+# thease tests are expected to fail.
+ifneq ($(mips-has-gnustack),yes)
 test-xfail-check-execstack = yes
+test-xfail-tst-execstack-ovrd1 = yes
+test-xfail-tst-execstack-ovrd1-static = yes
+endif
+
 endif
 
 ifeq ($(subdir),stdlib)
diff --git a/sysdeps/unix/sysv/linux/mips/configure.ac b/sysdeps/unix/sysv/linux/mips/configure.ac
index 9147aa4..bf2c6a9 100644
--- a/sysdeps/unix/sysv/linux/mips/configure.ac
+++ b/sysdeps/unix/sysv/linux/mips/configure.ac
@@ -118,6 +118,8 @@ fi
 LIBC_CONFIG_VAR([default-abi],
   [${libc_mips_abi}_${libc_mips_float}${libc_mips_nan}])
 
+LIBC_CONFIG_VAR([mips-has-gnustack],[${libc_cv_as_noexecstack}])
+
 case $machine in
 mips/mips64/n64/*)
   LIBC_SLIBDIR_RTLDDIR([lib64], [lib64])
diff --git a/sysdeps/unix/sysv/linux/mips/dl-execstack-ovrd.c b/sysdeps/unix/sysv/linux/mips/dl-execstack-ovrd.c
new file mode 100644
index 0000000..408186d
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/dl-execstack-ovrd.c
@@ -0,0 +1,48 @@
+/* Non-executable stack override for GNU dynamic linker.  MIPS/Linux version.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <elf.h>
+#include <ldsodefs.h>
+#include <dl-sysdep.h>
+
+
+#ifdef USE_DL_EXEC_STACK_OVERRIDE
+
+extern int __stack_prot attribute_relro attribute_hidden;
+
+int
+_dl_exec_stack_override (void* flags)
+{
+  if ((*(ElfW(Word) *)flags & PF_X) == 0
+       && (GLRO(dl_osversion) > 0)
+       && (GLRO(dl_osversion) < __NOEXECSTACK_MIN_KERNEL_VERSION))
+    {
+#ifndef SHARED
+      /* For static executable, we need to set stack permission here. */
+      uintptr_t page = ((uintptr_t) __libc_stack_end
+                        & -(intptr_t) GLRO(dl_pagesize));
+      if (__mprotect ((void *) page, GLRO(dl_pagesize),
+          PROT_READ | PROT_WRITE | PROT_EXEC | __stack_prot) < 0)
+        return errno;
+#endif
+      *(ElfW(Word) *)flags |= PF_X;
+    }
+  return 0;
+}
+rtld_hidden_def (_dl_exec_stack_override)
+#endif
diff --git a/sysdeps/unix/sysv/linux/mips/dl-sysdep.h b/sysdeps/unix/sysv/linux/mips/dl-sysdep.h
new file mode 100644
index 0000000..6ac36b6
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/dl-sysdep.h
@@ -0,0 +1,28 @@
+/* System-specific settings for dynamic linker code.  Linux version.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include_next <dl-sysdep.h>
+
+#define __NOEXECSTACK_MIN_KERNEL_VERSION (0x040800)
+
+#ifdef __mips_hard_float
+# if (__LINUX_KERNEL_VERSION < __NOEXECSTACK_MIN_KERNEL_VERSION)
+#  define USE_DL_EXEC_STACK_OVERRIDE 1
+# endif
+#endif
+
diff --git a/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd-static.c b/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd-static.c
new file mode 100644
index 0000000..0e5e61b
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd-static.c
@@ -0,0 +1 @@
+#include "tst-execstack-ovrd.c"
diff --git a/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd.c b/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd.c
new file mode 100644
index 0000000..9d2f4ef
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd.c
@@ -0,0 +1,2 @@
+#include "tst-execstack-prog.c"
+
diff --git a/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1-static.c b/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1-static.c
new file mode 100644
index 0000000..e45ac94
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1-static.c
@@ -0,0 +1 @@
+#include "tst-execstack-ovrd1.c"
diff --git a/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1.c b/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1.c
new file mode 100644
index 0000000..7e6252d
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1.c
@@ -0,0 +1,11 @@
+#include <signal.h>
+
+/* This test may fail (not produce a SIGSEGV) either because
+   DL_SYSDEP_OSCHECK detects that we are running on older kernel
+   than what we specify with LD_ASSUME_KERNEL and thus uses that
+   or the execution environment doesn't have NX semantics
+   (no RIXI support).  */
+#define EXPECTED_SIGNAL SIGSEGV
+
+#include "tst-execstack-prog.c"
+
-- 
1.9.1

[PATCH 3/3] [RFC][MIPS] Define GNU_STACK ABI

[Dragan Mladjenovic]

	* sysdeps/unix/sysv/linux/mips/ldsodefs.h (VALID_ELF_ABIVERSION):
	Bump max ABI version for ELFOSABI_SYSV to 6.
	* sysdeps/unix/sysv/linux/mips/libc-abis (GNU_STACK): New ABI.
---
 sysdeps/unix/sysv/linux/mips/ldsodefs.h | 2 +-
 sysdeps/unix/sysv/linux/mips/libc-abis  | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/sysdeps/unix/sysv/linux/mips/ldsodefs.h b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
index 8dde855..ce7b2f9 100644
--- a/sysdeps/unix/sysv/linux/mips/ldsodefs.h
+++ b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
@@ -34,7 +34,7 @@ extern void _dl_static_init (struct link_map *map);
 #undef VALID_ELF_ABIVERSION
 #define VALID_ELF_ABIVERSION(osabi,ver)			\
   (ver == 0						\
-   || (osabi == ELFOSABI_SYSV && ver < 4)		\
+   || (osabi == ELFOSABI_SYSV && ver < 6)		\
    || (osabi == ELFOSABI_GNU && ver < LIBC_ABI_MAX))
 
 #endif /* ldsodefs.h */
diff --git a/sysdeps/unix/sysv/linux/mips/libc-abis b/sysdeps/unix/sysv/linux/mips/libc-abis
index eaea558..cdf413b 100644
--- a/sysdeps/unix/sysv/linux/mips/libc-abis
+++ b/sysdeps/unix/sysv/linux/mips/libc-abis
@@ -16,3 +16,5 @@ UNIQUE
 MIPS_O32_FP64   mips*-*-linux*
 # Absolute (SHN_ABS) symbols working correctly.
 ABSOLUTE
+# Non-executable stack support working correctly
+MIPS_GNU_STACK mips*-*-linux*
-- 
1.9.1

Re: [PATCH 0/3] Mips support for PT_GNU_STACK

[Florian Weimer]

* Dragan Mladjenovic:

> The form of detection the patch proposes is not yet provided by the
> kernel. Instead, this version of the patch does kernel version check
> at runtime and provides compatible behavior if it cannot detect the
> 4.8 kernel or newer.

People patch their kernels to lie about the version, so I don't think
this is correct.

Kernel developers also think it's acceptable to change compatibility
mechanisms that have already been deployed in binutils or glibc, so I
really think this needs to wait until some signal has been added to the
the auxiliary vector in a mainline kernel.  Sorry.

Thanks,
Florian

Re: [PATCH 0/3] Mips support for PT_GNU_STACK

[Dragan Mladjenovic]

Thanks for the comment.

>> The form of detection the patch proposes is not yet provided by the
>> kernel. Instead, this version of the patch does kernel version check
>> at runtime and provides compatible behavior if it cannot detect the
>> 4.8 kernel or newer.
>
> People patch their kernels to lie about the version, so I don't think
> this is correct.

I'm not particularly fond of doing version checks, but this something
that is already done to enforce minimum kernel version supported 
by the glibc. Not sure this would be more broken that that. 

> Kernel developers also think it's acceptable to change compatibility
> mechanisms that have already been deployed in binutils or glibc, so I
> really think this needs to wait until some signal has been added to the
> the auxiliary vector in a mainline kernel.

I don't think that any new change on kernel side will make this change
obsolete or broken. At best if some kind of the signal gets provided by 
the kernel in the future that would allow us the have a real non-executable stack
on pre-4.8 kernel + 4.8 patch + future patch that provides the signal.

Best regards,

Dragan

Re: [PATCH 0/3] Mips support for PT_GNU_STACK

[Faraz Shahbazker]

On 6/28/19 1:34 AM, Florian Weimer wrote:
>> The form of detection the patch proposes is not yet provided by the
>> kernel. Instead, this version of the patch does kernel version check
>> at runtime and provides compatible behavior if it cannot detect the
>> 4.8 kernel or newer.
> 
> People patch their kernels to lie about the version, so I don't think
> this is correct.

Could a possible compromise be to forego the run-time check and instead make
the non-exec stack override trigger statically for MIPs when building glibc
with 4.8 or later kernel headers? In that case, the potential gap between glibc's
expectation and an old kernel masquerading as a newer version is exactly what it
would be for the usual minimum kernel version check.

We'd lose the ability to build against older kernel headers and work seamlessly
with newer kernels. This is not ideal, but it is more important to get a 
working non-executable stack solution out in user space.
> Kernel developers also think it's acceptable to change compatibility
> mechanisms that have already been deployed in binutils or glibc, so I
> really think this needs to wait until some signal has been added to the
> the auxiliary vector in a mainline kernel.

Note that as it stands, this is not an interface between the kernel and glibc.
Non-executable stack support is looked upon as a security fix in the kernel and
hence is not liable to flip back and forth, to the extent that there isn't a
KConfig setting which allows one to build the kernel without it. The auxiliary
vector OTOH would be a compatibility mechanism between the kernel and glibc and
hence would be vulnerable to the malicious manipulations of those devious kernel
developers :D

Regards,
Faraz

Re: [PATCH 0/3] Mips support for PT_GNU_STACK

[Dragan Mladjenovic]

Faraz Shahbazker *

> On 6/28/19 1:34 AM, Florian Weimer wrote:
>>> The form of detection the patch proposes is not yet provided by the
>>> kernel. Instead, this version of the patch does kernel version check
>>> at runtime and provides compatible behavior if it cannot detect the
>>> 4.8 kernel or newer.
>>
>> People patch their kernels to lie about the version, so I don't think
>> this is correct.
>
> Could a possible compromise be to forego the run-time check and instead make
> the non-exec stack override trigger statically for MIPs when building glibc
> with 4.8 or later kernel headers? In that case, the potential gap between glibc's
> expectation and an old kernel masquerading as a newer version is exactly what it
> would be for the usual minimum kernel version check.
>
> We'd lose the ability to build against older kernel headers and work seamlessly
> with newer kernels. This is not ideal, but it is more important to get a
> working non-executable stack solution out in user space.

I'm interested if proposed compromise is acceptable for the community to be done in this release cycle?
If not what else can we do to move this issue forward?

Best regards,

Dragan

Re: [PATCH 0/3] Mips support for PT_GNU_STACK

[Maciej W. Rozycki]

On Fri, 5 Jul 2019, Dragan Mladjenovic wrote:

> >>> The form of detection the patch proposes is not yet provided by the
> >>> kernel. Instead, this version of the patch does kernel version check
> >>> at runtime and provides compatible behavior if it cannot detect the
> >>> 4.8 kernel or newer.
> >>
> >> People patch their kernels to lie about the version, so I don't think
> >> this is correct.

 It is their problem then, I don't think it's a valid excuse.  We have 
previous art in this area; cf. commit d5f2798a0ac9 ("MIPS: Set the 
required Linux kernel version to 4.5.0 for 2008 NaN") and I reckon there 
have been more cases like this.

> > Could a possible compromise be to forego the run-time check and instead make
> > the non-exec stack override trigger statically for MIPs when building glibc
> > with 4.8 or later kernel headers? In that case, the potential gap between glibc's
> > expectation and an old kernel masquerading as a newer version is exactly what it
> > would be for the usual minimum kernel version check.
> >
> > We'd lose the ability to build against older kernel headers and work seamlessly
> > with newer kernels. This is not ideal, but it is more important to get a
> > working non-executable stack solution out in user space.
> 
> I'm interested if proposed compromise is acceptable for the community to be done in this release cycle?
> If not what else can we do to move this issue forward?

 Submitting stuff late in the cycle never helps, so I think the best 
compromise might be to target 2.31 instead.  And I think a kernel version 
check is the right approach.

  Maciej

Re: [PATCH 1/3] [ELF] Allow the machine to override stack permissions via USE_DL_EXEC_STACK_OVERRIDE.

[Joseph Myers]

On Thu, 27 Jun 2019, Dragan Mladjenovic wrote:

> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 5abeb86..9155b74 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1242,6 +1242,16 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>      /* Adjust the PT_PHDR value by the runtime load address.  */
>      l->l_phdr = (ElfW(Phdr) *) ((ElfW(Addr)) l->l_phdr + l->l_addr);
>  
> +#ifdef USE_DL_EXEC_STACK_OVERRIDE
> +  /* Program requests a non-executable stack, but architecture does
> +     not support it.  */
> +  if (__glibc_unlikely (_dl_exec_stack_override (&stack_flags) != 0))
> +    {
> +      errstring = N_("cannot override stack memory protections");
> +      goto call_lose_errno;
> +    }
> +#endif

This sort of #ifdef is not proper glibc style.  You should have a default 
trivial (inline?) definition of _dl_exec_stack_override and then have MIPS 
override the file with that function definition (without duplicating any 
architecture-independent code in the process).  If you have a default 
inline function definition, that means all this code gets checked for 
syntax when building for any architecture, not just for MIPS.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 1/3] [ELF] Allow the machine to override stack permissions via USE_DL_EXEC_STACK_OVERRIDE.

[Florian Weimer]

* Dragan Mladjenovic:

> +#ifdef USE_DL_EXEC_STACK_OVERRIDE
> +  /* Program requests a non-executable stack, but architecture does
> +     not support it.  */
> +  if (__glibc_unlikely (_dl_exec_stack_override (&stack_flags) != 0))
> +    {
> +      errstring = N_("cannot override stack memory protections");
> +      goto call_lose_errno;
> +    }
> +#endif

Is the comment really correct?  I think the proposed MIPS implementation
is not architecture-specific, but specific to the kernel version.

Thanks,
Florian

Re: [PATCH 0/3] Mips support for PT_GNU_STACK

[Florian Weimer]

* Faraz Shahbazker:

> On 6/28/19 1:34 AM, Florian Weimer wrote:
>>> The form of detection the patch proposes is not yet provided by the
>>> kernel. Instead, this version of the patch does kernel version check
>>> at runtime and provides compatible behavior if it cannot detect the
>>> 4.8 kernel or newer.
>> 
>> People patch their kernels to lie about the version, so I don't think
>> this is correct.
>
> Could a possible compromise be to forego the run-time check and
> instead make the non-exec stack override trigger statically for MIPs
> when building glibc with 4.8 or later kernel headers? In that case,
> the potential gap between glibc's expectation and an old kernel
> masquerading as a newer version is exactly what it would be for the
> usual minimum kernel version check.

The minimum kernel version check is the reason why kernels are patched
to lie about their version. 8-/

>> Kernel developers also think it's acceptable to change compatibility
>> mechanisms that have already been deployed in binutils or glibc, so I
>> really think this needs to wait until some signal has been added to the
>> the auxiliary vector in a mainline kernel.
>
> Note that as it stands, this is not an interface between the kernel
> and glibc.  Non-executable stack support is looked upon as a security
> fix in the kernel and hence is not liable to flip back and forth, to
> the extent that there isn't a KConfig setting which allows one to
> build the kernel without it. The auxiliary vector OTOH would be a
> compatibility mechanism between the kernel and glibc and hence would
> be vulnerable to the malicious manipulations of those devious kernel
> developers :D

Not sure I understand.  We have the same problem with vsyscall.  Its
absence is also advertised as a security feature, and yet there is no
easy way to detect that the kernel is missing what was once a key piece
of the x86-64 userspace ABI.

Based on the vsyscall experience, lack of a reliable detection mechanism
means that it can be impossible to get old userspace ready for new
kernels (because you can't just conventionalize code and limit the
impact on legacy products which share the same binaries).

Thanks,
Florian

Re: [PATCH 1/3] [ELF] Allow the machine to override stack permissions via USE_DL_EXEC_STACK_OVERRIDE.

[Dragan Mladjenovic]

Florian Weimer *

> Is the comment really correct?  I think the proposed MIPS implementation
> is not architecture-specific, but specific to the kernel version.

My mistake. I guess the correct term would be machine in glibc nomenclature.

Joseph Mayers*

>> > +#ifdef USE_DL_EXEC_STACK_OVERRIDE
>> > +  /* Program requests a non-executable stack, but architecture does
>> > +     not support it.  */
>> > +  if (__glibc_unlikely (_dl_exec_stack_override (&stack_flags) != 0))
>> > +    {
>> > +      errstring = N_("cannot override stack memory protections");
>> > +      goto call_lose_errno;
>> > +    }
>> > +#endif
> This sort of #ifdef is not proper glibc style.  You should have a default
> trivial (inline?) definition of _dl_exec_stack_override and then have MIPS
> override the file with that function definition (without duplicating any
> architecture-independent code in the process).  If you have a default
> inline function definition, that means all this code gets checked for
> syntax when building for any architecture, not just for MIPS.
>

Is patch below more suitable? We would have a common _dl_exec_stack_override
that would (preferably) not be overridden by the machine support, but instead one can define
DL_EXEC_STACK_OVERRIDE or maybe  DL_EXEC_STACK_OVERRIDE_P to control 
the conditions under which the stack "override" happens. 

I realized that I don't actually need to duplicate the __stack_prot unprotect/protect code from
elf/dl-load.c, so I moved the dynamic linking case into dl-main. This way the version check is done
at most once.

Static linking case is still done as part of _dl_non_dynamic_init. If we ever gain support for IFUNC
on MIPS we would probably need to move this somewhere before running IFUNC revolvers.

What are your thoughts on this?

diff --git a/elf/dl-exec-stack-override.h b/elf/dl-exec-stack-override.h
new file mode 100644
index 0000000..10401a8
--- /dev/null
+++ b/elf/dl-exec-stack-override.h
@@ -0,0 +1,36 @@
+/* Make stack executable if the machine requires it.  Generic version.
...
+
+#include <ldsodefs.h>
+
+extern int __stack_prot attribute_relro attribute_hidden;
+
+static __always_inline void
+_dl_exec_stack_override (void)
+{
+  if (__glibc_unlikely ((GL(dl_stack_flags) & PF_X) == 0
+                         && DL_EXEC_STACK_OVERRIDE))
+  {
+    __stack_prot |= PROT_READ|PROT_WRITE|PROT_EXEC;
+
+    void *stack_end = __libc_stack_end;
+    int err = _dl_make_stack_executable (&stack_end);
+    if (__glibc_unlikely (err))
+      _dl_fatal_printf ("cannot enable executable stack as machine requires\n");
+  }
+}
diff --git a/elf/dl-support.c b/elf/dl-support.c
index 0a8b636..923aa4c 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -29,6 +29,7 @@
 #include <dl-machine.h>
 #include <libc-lock.h>
 #include <dl-cache.h>
+#include <dl-exec-stack-override.h>
 #include <dl-librecon.h>
 #include <dl-procinfo.h>
 #include <unsecvars.h>
@@ -375,6 +376,8 @@ _dl_non_dynamic_init (void)
 	  _dl_stack_flags = _dl_phdr[i].p_flags;
 	  break;
 	}
+
+  _dl_exec_stack_override ();
 }
 
 #ifdef DL_SYSINFO_IMPLEMENTATION
diff --git a/elf/rtld.c b/elf/rtld.c
index c9490ff..f3e00f9 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -36,6 +36,7 @@
 #include <dl-librecon.h>
 #include <unsecvars.h>
 #include <dl-cache.h>
+#include <dl-exec-stack-override.h>
 #include <dl-osinfo.h>
 #include <dl-procinfo.h>
 #include <dl-prop.h>
@@ -1542,6 +1543,8 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
   DL_SYSDEP_OSCHECK (_dl_fatal_printf);
 #endif
 
+  _dl_exec_stack_override ();
+
   /* Initialize the data structures for the search paths for shared
      objects.  */
   _dl_init_paths (library_path);
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index b1fc5c3..70e96c0 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -119,6 +119,10 @@ dl_symbol_visibility_binds_local_p (const ElfW(Sym) *sym)
 # define DL_STATIC_INIT(map)
 #endif
 
+#ifndef DL_EXEC_STACK_OVERRIDE
+# define DL_EXEC_STACK_OVERRIDE false
+#endif
+
 /* Reloc type classes as returned by elf_machine_type_class().
    ELF_RTYPE_CLASS_PLT means this reloc should not be satisfied by
    some PLT symbol, ELF_RTYPE_CLASS_COPY means this reloc should not be

Re: [PATCH 0/3] Mips support for PT_GNU_STACK

[Dragan Mladjenovic]

On 08.07.2019. 14:01, Florian Weimer  wrote:

>> Could a possible compromise be to forego the run-time check and
>> instead make the non-exec stack override trigger statically for MIPs
>> when building glibc with 4.8 or later kernel headers? In that case,
>> the potential gap between glibc's expectation and an old kernel
>> masquerading as a newer version is exactly what it would be for the
>> usual minimum kernel version check.
>
> The minimum kernel version check is the reason why kernels are patched
> to lie about their version. 8-/

The user can lie via LD_ASSUME_KERNEL env or by hacking the kernel,
but that implies that they know what they are doing.  

There is an use-case of someone wanting to back-port the 4.8 kernel patch to their older
kernel and have glibc chose to honor the RW GNU_STACK.
Version check doesn't help here, and while it sounds a bit far-fetched I guess one could
be tempted to hack the kernel version along the way. 

Having something else than version check would be better in above case, but until that hits
the mainline we could as well move to 4.8 as minimum kernel version.

>>> Kernel developers also think it's acceptable to change compatibility
>>> mechanisms that have already been deployed in binutils or glibc, so I
>>> really think this needs to wait until some signal has been added to the
>>> the auxiliary vector in a mainline kernel.
>>
>> Note that as it stands, this is not an interface between the kernel
>> and glibc.  Non-executable stack support is looked upon as a security
>> fix in the kernel and hence is not liable to flip back and forth, to
>> the extent that there isn't a KConfig setting which allows one to
>> build the kernel without it. The auxiliary vector OTOH would be a
>> compatibility mechanism between the kernel and glibc and hence would
>> be vulnerable to the malicious manipulations of those devious kernel
>> developers :D
>
> Not sure I understand.  We have the same problem with vsyscall.  Its
> absence is also advertised as a security feature, and yet there is no
> easy way to detect that the kernel is missing what was once a key piece
> of the x86-64 userspace ABI.
>
> Based on the vsyscall experience, lack of a reliable detection mechanism
> means that it can be impossible to get old userspace ready for new
> kernels (because you can't just conventionalize code and limit the
> impact on legacy products which share the same binaries).

If someone in the future would decide do go back to the 
old kernel behavior of "randomly"* crashing user-space application that use
RW GNU_STACK we would be in the problem ether way.

* Not really random, but might as well be.

Best regards,

Dragan