unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Mips support for PT_GNU_STACK
@ 2019-07-15 18:23 Dragan Mladjenovic
  2019-07-16 11:15 ` [PATCH v2 1/3] [ELF] Allow the machine support to enforce executable stack Dragan Mladjenovic
  2019-07-17 19:43 ` [PATCH v2 0/3] Mips support for PT_GNU_STACK Adhemerval Zanella
  0 siblings, 2 replies; 14+ messages in thread
From: Dragan Mladjenovic @ 2019-07-15 18:23 UTC (permalink / raw)
  To: libc-alpha@sourceware.org
  Cc: Dragan Mladjenovic, Joseph Myers, Carlos O'Donell,
	Maciej W . Rozycki, Faraz Shahbazker

Hello everyone,

Patches in this series are a 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 an instruction in the delay slot of an 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, the 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 a proposal for Binutils and GCC after this
part gets finalized.

Even if this part doesn't get in the next release due to issues [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.

Changes from v1 [5]: Moved stack override logic behind inline _dl_exec_stack_override.

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
[5] https://sourceware.org/ml/libc-alpha/2019-06/msg00889.html

Dragan Mladjenovic (3):
  [ELF] Allow the machine support to enforce executable stack
  [MIPS] Define DL_EXEC_STACK_OVERRIDE
  [RFC][MIPS] Define GNU_STACK ABI

 elf/dl-exec-stack-override.h                       | 36 ++++++++++++++++++++++
 elf/dl-support.c                                   |  3 ++
 elf/rtld.c                                         |  3 ++
 sysdeps/generic/ldsodefs.h                         |  4 +++
 sysdeps/unix/sysv/linux/mips/Makefile              | 26 +++++++++++++---
 sysdeps/unix/sysv/linux/mips/configure.ac          |  3 ++
 sysdeps/unix/sysv/linux/mips/ldsodefs.h            | 13 +++++++-
 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 | 10 ++++++
 12 files changed, 99 insertions(+), 5 deletions(-)
 create mode 100644 elf/dl-exec-stack-override.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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/3] [ELF] Allow the machine support to enforce executable stack
  2019-07-15 18:23 [PATCH v2 0/3] Mips support for PT_GNU_STACK Dragan Mladjenovic
@ 2019-07-16 11:15 ` Dragan Mladjenovic
  2019-07-16 11:16   ` [PATCH v2 2/3] [MIPS] Define DL_EXEC_STACK_OVERRIDE Dragan Mladjenovic
  2019-07-16 11:16   ` [PATCH v2 3/3] [RFC][MIPS] Define GNU_STACK ABI Dragan Mladjenovic
  2019-07-17 19:43 ` [PATCH v2 0/3] Mips support for PT_GNU_STACK Adhemerval Zanella
  1 sibling, 2 replies; 14+ messages in thread
From: Dragan Mladjenovic @ 2019-07-16 11:15 UTC (permalink / raw)
  To: libc-alpha@sourceware.org
  Cc: Joseph Myers, Carlos O'Donell, Maciej W. Rozycki,
	Faraz Shahbazker, Dragan Mladjenovic

This patch allows the machine-dependent code to override non-executable
stack permissions by defining DL_EXEC_STACK_OVERRIDE to the condition under
which the stack should be made executable.

	* elf/dl-exec-stack-override.h: New file.
	* elf/dl-support.c (_dl_non_dynamic_init): Call _dl_exec_stack_override.
	* elf/rtld.c (dl_main): Likewise.
	* sysdeps/generic/ldsodefs.h: Default DL_EXEC_STACK_OVERRIDE to false.
---
 elf/dl-exec-stack-override.h | 36 ++++++++++++++++++++++++++++++++++++
 elf/dl-support.c             |  3 +++
 elf/rtld.c                   |  3 +++
 sysdeps/generic/ldsodefs.h   |  4 ++++
 4 files changed, 46 insertions(+)
 create mode 100644 elf/dl-exec-stack-override.h

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.
+   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 <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
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/3] [MIPS] Define DL_EXEC_STACK_OVERRIDE
  2019-07-16 11:15 ` [PATCH v2 1/3] [ELF] Allow the machine support to enforce executable stack Dragan Mladjenovic
@ 2019-07-16 11:16   ` Dragan Mladjenovic
  2019-07-16 11:16   ` [PATCH v2 3/3] [RFC][MIPS] Define GNU_STACK ABI Dragan Mladjenovic
  1 sibling, 0 replies; 14+ messages in thread
From: Dragan Mladjenovic @ 2019-07-16 11:16 UTC (permalink / raw)
  To: libc-alpha@sourceware.org
  Cc: Joseph Myers, Carlos O'Donell, Maciej W. Rozycki,
	Faraz Shahbazker, Dragan Mladjenovic

For hard-float builds targeting minimum Linux kernel version lower than
4.8 DL_EXEC_STACK_OVERRIDE will request executable stack if it detects
that runtime kernel version is also lower than 4.8.

Build now detects if mips toolchain uses GNU.stack note and updates
the expected result of check-execstack accordingly.

	* sysdeps/unix/sysv/linux/mips/Makefile
	[$(subdir) == elf][$(mips-float) == hard]
	(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 rules.
	* sysdeps/unix/sysv/linux/mips/Makefile
	[$(subdir) == elf][$(mips-has-gnustack) != yes]
	(test-xfail-check-execstack): Move test-xfail-check-execstack here.
	(test-xfail-tst-execstack-ovrd1): New xfail.
	* sysdeps/unix/sysv/linux/mips/configure.ac
	(mips-has-gnustack): New var. Set to value of libc_cv_as_noexecstack.
	(mips-float): New var. Set to value of libc_mips_float.
	* sysdeps/unix/sysv/linux/mips/configure: Regenerated.
	* sysdeps/unix/sysv/linux/mips/ldsodefs.h
	(__GNU_STACK_SAFE_KERNEL_VERSION): New. Define to 4.8.0.
	(DL_EXEC_STACK_OVERRIDE): New. Check dl_osversion against previous define.
	* 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              | 26 ++++++++++++++++++----
 sysdeps/unix/sysv/linux/mips/configure.ac          |  3 +++
 sysdeps/unix/sysv/linux/mips/ldsodefs.h            | 11 +++++++++
 .../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 | 10 +++++++++
 7 files changed, 50 insertions(+), 4 deletions(-)
 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..328f0b4 100644
--- a/sysdeps/unix/sysv/linux/mips/Makefile
+++ b/sysdeps/unix/sysv/linux/mips/Makefile
@@ -64,11 +64,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>.
+ifeq ($(mips-float),hard)
+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
+endif
+# 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
+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..1ff77e7 100644
--- a/sysdeps/unix/sysv/linux/mips/configure.ac
+++ b/sysdeps/unix/sysv/linux/mips/configure.ac
@@ -118,6 +118,9 @@ 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}])
+LIBC_CONFIG_VAR([mips-float],[${libc_mips_float}])
+
 case $machine in
 mips/mips64/n64/*)
   LIBC_SLIBDIR_RTLDDIR([lib64], [lib64])
diff --git a/sysdeps/unix/sysv/linux/mips/ldsodefs.h b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
index 8dde855..803fdd1 100644
--- a/sysdeps/unix/sysv/linux/mips/ldsodefs.h
+++ b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
@@ -37,4 +37,15 @@ extern void _dl_static_init (struct link_map *map);
    || (osabi == ELFOSABI_SYSV && ver < 4)		\
    || (osabi == ELFOSABI_GNU && ver < LIBC_ABI_MAX))
 
+
+#define __GNU_STACK_SAFE_KERNEL_VERSION (0x040800)
+
+#ifdef __mips_hard_float
+# if (__LINUX_KERNEL_VERSION < __GNU_STACK_SAFE_KERNEL_VERSION)
+#  undef DL_EXEC_STACK_OVERRIDE
+#  define DL_EXEC_STACK_OVERRIDE        \
+                 ((GLRO(dl_osversion) < __GNU_STACK_SAFE_KERNEL_VERSION))
+# endif
+#endif
+
 #endif /* ldsodefs.h */
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..953dd37
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/tst-execstack-ovrd1.c
@@ -0,0 +1,10 @@
+#include <signal.h>
+
+/* This test will fail (not produce a SIGSEGV) either because
+   DL_SYSDEP_OSCHECK detects that we are running on kernel older
+   than what we specify via LD_ASSUME_KERNEL or the execution
+   environment doesn't support NX semantics (no RIXI).  */
+#define EXPECTED_SIGNAL SIGSEGV
+
+#include "tst-execstack-prog.c"
+
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/3] [RFC][MIPS] Define GNU_STACK ABI
  2019-07-16 11:15 ` [PATCH v2 1/3] [ELF] Allow the machine support to enforce executable stack Dragan Mladjenovic
  2019-07-16 11:16   ` [PATCH v2 2/3] [MIPS] Define DL_EXEC_STACK_OVERRIDE Dragan Mladjenovic
@ 2019-07-16 11:16   ` Dragan Mladjenovic
  1 sibling, 0 replies; 14+ messages in thread
From: Dragan Mladjenovic @ 2019-07-16 11:16 UTC (permalink / raw)
  To: libc-alpha@sourceware.org
  Cc: Joseph Myers, Carlos O'Donell, Maciej W. Rozycki,
	Faraz Shahbazker, 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 803fdd1..c9835b7 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))
 
 
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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/3] Mips support for PT_GNU_STACK
  2019-07-15 18:23 [PATCH v2 0/3] Mips support for PT_GNU_STACK Dragan Mladjenovic
  2019-07-16 11:15 ` [PATCH v2 1/3] [ELF] Allow the machine support to enforce executable stack Dragan Mladjenovic
@ 2019-07-17 19:43 ` Adhemerval Zanella
  2019-07-17 21:47   ` Dragan Mladjenovic
                     ` (2 more replies)
  1 sibling, 3 replies; 14+ messages in thread
From: Adhemerval Zanella @ 2019-07-17 19:43 UTC (permalink / raw)
  To: Dragan Mladjenovic, libc-alpha@sourceware.org
  Cc: Joseph Myers, Carlos O'Donell, Maciej W . Rozycki,
	Faraz Shahbazker



On 15/07/2019 15:23, Dragan Mladjenovic wrote:
> Hello everyone,
> 
> Patches in this series are a 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 an instruction in the delay slot of an 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, the 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.

I think checking the kernel version is the wrong approach, it prevents a distribution
to backport the kernel fix without also applying a out-of-tree patch to fix it on glibc
as well.  IMHO the proper way would be to make kernel advertise it through hwcap, as
other architectures do for similar kernel features and not tie it to any specific
version.

>  
> 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 a proposal for Binutils and GCC after this
> part gets finalized.

If the idea is to fallback to executable stack for the case of underlying missing
kernel support, which is the net gain in adding this requirement? My understanding
it ABI bump should be used to fail early for the cases where the new binaries
requires loader support that can not be provided (iFUNC or new relocations), not
for hardening.

> 
> Even if this part doesn't get in the next release due to issues [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.

If you don't tie it support to kernel version checks or require bump the ABI version,
it should be fair straight to backport it.

> 
> Changes from v1 [5]: Moved stack override logic behind inline _dl_exec_stack_override.
> 
> 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
> [5] https://sourceware.org/ml/libc-alpha/2019-06/msg00889.html
> 
> Dragan Mladjenovic (3):
>   [ELF] Allow the machine support to enforce executable stack
>   [MIPS] Define DL_EXEC_STACK_OVERRIDE
>   [RFC][MIPS] Define GNU_STACK ABI
> 
>  elf/dl-exec-stack-override.h                       | 36 ++++++++++++++++++++++
>  elf/dl-support.c                                   |  3 ++
>  elf/rtld.c                                         |  3 ++
>  sysdeps/generic/ldsodefs.h                         |  4 +++
>  sysdeps/unix/sysv/linux/mips/Makefile              | 26 +++++++++++++---
>  sysdeps/unix/sysv/linux/mips/configure.ac          |  3 ++
>  sysdeps/unix/sysv/linux/mips/ldsodefs.h            | 13 +++++++-
>  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 | 10 ++++++
>  12 files changed, 99 insertions(+), 5 deletions(-)
>  create mode 100644 elf/dl-exec-stack-override.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
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/3] Mips support for PT_GNU_STACK
  2019-07-17 19:43 ` [PATCH v2 0/3] Mips support for PT_GNU_STACK Adhemerval Zanella
@ 2019-07-17 21:47   ` Dragan Mladjenovic
  2019-07-18 13:33     ` Adhemerval Zanella
  2019-07-17 22:59   ` Faraz Shahbazker
  2019-07-18 14:50   ` Maciej W. Rozycki
  2 siblings, 1 reply; 14+ messages in thread
From: Dragan Mladjenovic @ 2019-07-17 21:47 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha@sourceware.org
  Cc: Joseph Myers, Carlos O'Donell, Maciej W . Rozycki,
	Faraz Shahbazker


On 17.07.2019. 21:43, Adhemerval Zanella wrote:
>
> On 15/07/2019 15:23, Dragan Mladjenovic wrote:
>> Hello everyone,
>>
>> Patches in this series are a 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 an instruction in the delay slot of an 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, the 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.
> I think checking the kernel version is the wrong approach, it prevents a distribution
> to backport the kernel fix without also applying a out-of-tree patch to fix it on glibc
> as well.  IMHO the proper way would be to make kernel advertise it through hwcap, as
> other architectures do for similar kernel features and not tie it to any specific
> version.

First, thanks for the comments.
 
I agree with everything and the original patch did propose the usage of AT_FLAGS
auxval for this purpose. But, sadly, the kernel still doesn't advertise this information
to the user mode and it would take some time for it to be back-ported even if it gets
implemented, let's say in next kernel release. Meanwhile, this approach, while lacking,
would allow for distributions to enable PT_GNU_STACK way earlier and to actually
enjoy the benefit of it on existing 4.8+ kernels while remaining compatible with pre-4.8
ones.

>>  
>> 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 a proposal for Binutils and GCC after this
>> part gets finalized.
> If the idea is to fallback to executable stack for the case of underlying missing
> kernel support, which is the net gain in adding this requirement? My understanding
> it ABI bump should be used to fail early for the cases where the new binaries
> requires loader support that can not be provided (iFUNC or new relocations), not
> for hardening.

True, and that is the reason I'm not hard pressed for this part of the patch. Though one
nice benefit form it is that it prevents you from accidentally using the GNU.stack enabled
mips toolchain to build glibc w/o this patch. I guess that can be prevented on gcc side if
glibc is built as part of the toolchain build, but otherwise, this would require some discipline
form the end user.

Without this, the dynamically liked binaries/shared objects would still work with older glibcs.
Given that existing distributions are build w/o PT_GNU_STACK they would get the executable
stack anyway. Statically linked binaries would be handled by linked libc. So we are safe on
pre-4.8 kernels.

This should work as long as there is no RW PT_GNU_STACK built glibc w/o this patch in "the wild".

From my point of view this is more to enable, at least for glibc based environment,
RW PT_GNU_STACK by default w/o having the end-user worrying on which kernel will it run
than it is to enforce the hardening. 

>> Even if this part doesn't get in the next release due to issues [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.
> If you don't tie it support to kernel version checks or require bump the ABI version,
> it should be fair straight to backport it.

Another option that came up during the v1 discussion is to actually require 4.8 as minimal
kernel version if glibc gets build with RW PT_GNU_STACK.

If it gets back-ported far enough, that approach would satisfy the requirement that there is no
RW PT_GNU_STACK glibc running on the pre-4.8 kernels which would still allow us to enable 
GNU.stack on GCC by default, at least for mips*-*-gnu* triplets.


Best regards,

Dragan


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/3] Mips support for PT_GNU_STACK
  2019-07-17 19:43 ` [PATCH v2 0/3] Mips support for PT_GNU_STACK Adhemerval Zanella
  2019-07-17 21:47   ` Dragan Mladjenovic
@ 2019-07-17 22:59   ` Faraz Shahbazker
  2019-07-18 13:38     ` Adhemerval Zanella
  2019-07-18 14:50   ` Maciej W. Rozycki
  2 siblings, 1 reply; 14+ messages in thread
From: Faraz Shahbazker @ 2019-07-17 22:59 UTC (permalink / raw)
  To: Adhemerval Zanella, Dragan Mladjenovic, libc-alpha@sourceware.org
  Cc: Joseph Myers, Carlos O'Donell, Maciej W . Rozycki

On 07/17/2019 12:43 PM, Adhemerval Zanella wrote:
> I think checking the kernel version is the wrong approach, it prevents a distribution
> to backport the kernel fix without also applying a out-of-tree patch to fix it on glibc
> as well.  IMHO the proper way would be to make kernel advertise it through hwcap, as
> other architectures do for similar kernel features and not tie it to any specific
> version.

The original proposal was to advertise through AT_FLAGS. I've heard this suggestion of using
hwcap from multiple sources, so I am curious - what other *purely* software kernel features
are advertised using hwcap?

>> 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 a proposal for Binutils and GCC after this
>> part gets finalized.
> 
> If the idea is to fallback to executable stack for the case of underlying missing
> kernel support, which is the net gain in adding this requirement? My understanding
> it ABI bump should be used to fail early for the cases where the new binaries
> requires loader support that can not be provided (iFUNC or new relocations), not
> for hardening.

It is not really hardening, the way glibc handles PT_GNU_STACK is along the lines of
'may have non-executable stack' rather than 'must have non-executable stack'. However
the MIPS backend overrides *any* incoming PT_GNU_STACK permissions using the default (RWX)
permissions for MIPS, in effect enforcing 'will not have non-executable stack'. What is
being indicated here is a change in this default behaviour. The ABI version bump would
indicate whether PT_GNU_STACK permissions will be honoured (at least to the extent it is
by other architectures) or simply ignored (as it has historically been).

IMO, the current behaviour of PT_GNU_STACK for MIPS is an anomaly in itself. What should
have been, is a rejection of non-executable PT_GNU_STACK at some level, instead of silently
overriding it in glibc. So are you of the opinion that this change in glibc behaviour is not
worth being published at all, or that it should be advertised using a different mechanism
instead of an ABI version bump?

Regards,
Faraz

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/3] Mips support for PT_GNU_STACK
  2019-07-17 21:47   ` Dragan Mladjenovic
@ 2019-07-18 13:33     ` Adhemerval Zanella
  2019-07-18 20:00       ` Dragan Mladjenovic
  0 siblings, 1 reply; 14+ messages in thread
From: Adhemerval Zanella @ 2019-07-18 13:33 UTC (permalink / raw)
  To: Dragan Mladjenovic, libc-alpha@sourceware.org
  Cc: Joseph Myers, Carlos O'Donell, Maciej W . Rozycki,
	Faraz Shahbazker, Florian Weimer



On 17/07/2019 18:47, Dragan Mladjenovic wrote:
> 
> On 17.07.2019. 21:43, Adhemerval Zanella wrote:
>>
>> On 15/07/2019 15:23, Dragan Mladjenovic wrote:
>>> Hello everyone,
>>>
>>> Patches in this series are a 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 an instruction in the delay slot of an 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, the 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.
>> I think checking the kernel version is the wrong approach, it prevents a distribution
>> to backport the kernel fix without also applying a out-of-tree patch to fix it on glibc
>> as well.  IMHO the proper way would be to make kernel advertise it through hwcap, as
>> other architectures do for similar kernel features and not tie it to any specific
>> version.
> 
> First, thanks for the comments.
>  
> I agree with everything and the original patch did propose the usage of AT_FLAGS
> auxval for this purpose. But, sadly, the kernel still doesn't advertise this information
> to the user mode and it would take some time for it to be back-ported even if it gets
> implemented, let's say in next kernel release. Meanwhile, this approach, while lacking,
> would allow for distributions to enable PT_GNU_STACK way earlier and to actually
> enjoy the benefit of it on existing 4.8+ kernels while remaining compatible with pre-4.8
> ones.

It seems the kernel commit that actually added this support on mips was 
432c6bacbd0c16ec210c43da411ccc3855c4c010.  Isn't there any other mechanism to check 
for kernel support without tying a kernel version check? 

The code seems to reserve an extra page for the delay slot emulation, could we
infer based on vdso placement, page size, and maximum stack size? 

Otherwise I think we should do as usual, which is just assumes non-executable stack if
glibc is configure for kernel higher than 4.8 (with --enable-kernel). Checking on
patch submission history, Florian also stated that runtime version check is not
an option here.

> 
>>>  
>>> 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 a proposal for Binutils and GCC after this
>>> part gets finalized.
>> If the idea is to fallback to executable stack for the case of underlying missing
>> kernel support, which is the net gain in adding this requirement? My understanding
>> it ABI bump should be used to fail early for the cases where the new binaries
>> requires loader support that can not be provided (iFUNC or new relocations), not
>> for hardening.
> 
> True, and that is the reason I'm not hard pressed for this part of the patch. Though one
> nice benefit form it is that it prevents you from accidentally using the GNU.stack enabled
> mips toolchain to build glibc w/o this patch. I guess that can be prevented on gcc side if
> glibc is built as part of the toolchain build, but otherwise, this would require some discipline
> form the end user.
> 
> Without this, the dynamically liked binaries/shared objects would still work with older glibcs.
> Given that existing distributions are build w/o PT_GNU_STACK they would get the executable
> stack anyway. Statically linked binaries would be handled by linked libc. So we are safe on
> pre-4.8 kernels.
> 
> This should work as long as there is no RW PT_GNU_STACK built glibc w/o this patch in "the wild".
> 
> From my point of view this is more to enable, at least for glibc based environment,
> RW PT_GNU_STACK by default w/o having the end-user worrying on which kernel will it run
> than it is to enforce the hardening. 

I don't have a strong opinion here, although the main issue here is runtime support
provided by underlying kernel rather than an ABI issue in fact.  

One option is just to check if compilers sets GNU.stack as default to require a
minimum kernel version of 3.8 (as we already did for 2008 NaN on d5f2798a0ac9).

> 
>>> Even if this part doesn't get in the next release due to issues [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.
>> If you don't tie it support to kernel version checks or require bump the ABI version,
>> it should be fair straight to backport it.
> 
> Another option that came up during the v1 discussion is to actually require 4.8 as minimal
> kernel version if glibc gets build with RW PT_GNU_STACK.
> 
> If it gets back-ported far enough, that approach would satisfy the requirement that there is no
> RW PT_GNU_STACK glibc running on the pre-4.8 kernels which would still allow us to enable 
> GNU.stack on GCC by default, at least for mips*-*-gnu* triplets.

If a distribution aims to backport this features, it will need to coordinate on 
multiple parts (kernel, compiler, runtime), so I prefer to avoid trying to 
provision this beforehand.  I would go to with a simpler solution, instead of
adding runtime/configure checks.

> 
> 
> Best regards,
> 
> Dragan
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/3] Mips support for PT_GNU_STACK
  2019-07-17 22:59   ` Faraz Shahbazker
@ 2019-07-18 13:38     ` Adhemerval Zanella
  2019-07-18 15:34       ` Faraz Shahbazker
  2019-07-18 19:49       ` Dragan Mladjenovic
  0 siblings, 2 replies; 14+ messages in thread
From: Adhemerval Zanella @ 2019-07-18 13:38 UTC (permalink / raw)
  To: Faraz Shahbazker, Dragan Mladjenovic, libc-alpha@sourceware.org
  Cc: Joseph Myers, Carlos O'Donell, Maciej W . Rozycki



On 17/07/2019 19:59, Faraz Shahbazker wrote:
> On 07/17/2019 12:43 PM, Adhemerval Zanella wrote:
>> I think checking the kernel version is the wrong approach, it prevents a distribution
>> to backport the kernel fix without also applying a out-of-tree patch to fix it on glibc
>> as well.  IMHO the proper way would be to make kernel advertise it through hwcap, as
>> other architectures do for similar kernel features and not tie it to any specific
>> version.
> 
> The original proposal was to advertise through AT_FLAGS. I've heard this suggestion of using
> hwcap from multiple sources, so I am curious - what other *purely* software kernel features
> are advertised using hwcap?

It is not common, but on powerpc has both PPC_FEATURE2_HTM_NOSC and 
PPC_FEATURE2_HTM_NO_SUSPEND which is kernel behaviour regarding transaction
memory state and syscall execution.

> 
>>> 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 a proposal for Binutils and GCC after this
>>> part gets finalized.
>>
>> If the idea is to fallback to executable stack for the case of underlying missing
>> kernel support, which is the net gain in adding this requirement? My understanding
>> it ABI bump should be used to fail early for the cases where the new binaries
>> requires loader support that can not be provided (iFUNC or new relocations), not
>> for hardening.
> 
> It is not really hardening, the way glibc handles PT_GNU_STACK is along the lines of
> 'may have non-executable stack' rather than 'must have non-executable stack'. However
> the MIPS backend overrides *any* incoming PT_GNU_STACK permissions using the default (RWX)
> permissions for MIPS, in effect enforcing 'will not have non-executable stack'. What is
> being indicated here is a change in this default behaviour. The ABI version bump would
> indicate whether PT_GNU_STACK permissions will be honoured (at least to the extent it is
> by other architectures) or simply ignored (as it has historically been).
> 
> IMO, the current behaviour of PT_GNU_STACK for MIPS is an anomaly in itself. What should
> have been, is a rejection of non-executable PT_GNU_STACK at some level, instead of silently
> overriding it in glibc. So are you of the opinion that this change in glibc behaviour is not
> worth being published at all, or that it should be advertised using a different mechanism
> instead of an ABI version bump?

Since non-executable stack is tied with underlying kernel support rather than 
ABI, my suggestion is just to assume non-executable stack as default, without
permission override, if glibc is configure for kernel higher than 3.8. We will
need to live with old behaviour for default builds.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/3] Mips support for PT_GNU_STACK
  2019-07-17 19:43 ` [PATCH v2 0/3] Mips support for PT_GNU_STACK Adhemerval Zanella
  2019-07-17 21:47   ` Dragan Mladjenovic
  2019-07-17 22:59   ` Faraz Shahbazker
@ 2019-07-18 14:50   ` Maciej W. Rozycki
  2 siblings, 0 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2019-07-18 14:50 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: Dragan Mladjenovic, libc-alpha@sourceware.org, Joseph Myers,
	Carlos O'Donell, Faraz Shahbazker

On Wed, 17 Jul 2019, Adhemerval Zanella 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.
> 
> I think checking the kernel version is the wrong approach, it prevents a 
> distribution to backport the kernel fix without also applying a 
> out-of-tree patch to fix it on glibc as well.

 Good point!

>  IMHO the proper way would be to make kernel advertise it through hwcap, 
> as other architectures do for similar kernel features and not tie it to 
> any specific version.

 As other people have pointed out to use AT_HWCAP* for a plain software 
feature might not be the most fortunate choice and we could use AT_FLAGS 
or even define AT_LINUX_FEATURES (or AT_MIPS_FEATURES), but that is 
secondary; it's not like we're going to run out of auxiliary vector's 
space.

 FWIW,

  Maciej

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/3] Mips support for PT_GNU_STACK
  2019-07-18 13:38     ` Adhemerval Zanella
@ 2019-07-18 15:34       ` Faraz Shahbazker
  2019-07-18 19:49       ` Dragan Mladjenovic
  1 sibling, 0 replies; 14+ messages in thread
From: Faraz Shahbazker @ 2019-07-18 15:34 UTC (permalink / raw)
  To: Adhemerval Zanella, Dragan Mladjenovic, libc-alpha@sourceware.org
  Cc: Joseph Myers, Carlos O'Donell, Maciej W . Rozycki

On 7/18/19 6:38 AM, Adhemerval Zanella wrote:> On 17/07/2019 19:59, Faraz Shahbazker wrote:
>> On 07/17/2019 12:43 PM, Adhemerval Zanella wrote:
>> IMO, the current behaviour of PT_GNU_STACK for MIPS is an anomaly in itself. What should
>> have been, is a rejection of non-executable PT_GNU_STACK at some level, instead of silently
>> overriding it in glibc. So are you of the opinion that this change in glibc behaviour is not
>> worth being published at all, or that it should be advertised using a different mechanism
>> instead of an ABI version bump?
> 
> Since non-executable stack is tied with underlying kernel support rather than 
> ABI, my suggestion is just to assume non-executable stack as default, without
> permission override, if glibc is configure for kernel higher than *4.8*. We will
> need to live with old behaviour for default builds.

Frankly, I can live with that. If someone really wants to back-port they have to do the
leg-work across multiple components to get it to work. This theoretically still leaves
the door open for the kernel to start advertising non-exec stack support to allow for
easier back-ports.
Regards,
Faraz

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/3] Mips support for PT_GNU_STACK
  2019-07-18 13:38     ` Adhemerval Zanella
  2019-07-18 15:34       ` Faraz Shahbazker
@ 2019-07-18 19:49       ` Dragan Mladjenovic
  2019-07-18 20:54         ` Adhemerval Zanella
  1 sibling, 1 reply; 14+ messages in thread
From: Dragan Mladjenovic @ 2019-07-18 19:49 UTC (permalink / raw)
  To: Adhemerval Zanella, Faraz Shahbazker, libc-alpha@sourceware.org
  Cc: Joseph Myers, Carlos O'Donell, Maciej W . Rozycki



On 18.07.2019. 15:38, Adhemerval Zanella wrote:
>
>
> On 17/07/2019 19:59, Faraz Shahbazker wrote:
>> On 07/17/2019 12:43 PM, Adhemerval Zanella wrote:
>>> I think checking the kernel version is the wrong approach, it prevents a distribution
>>> to backport the kernel fix without also applying a out-of-tree patch to fix it on glibc
>>> as well.  IMHO the proper way would be to make kernel advertise it through hwcap, as
>>> other architectures do for similar kernel features and not tie it to any specific
>>> version.
>>
>> The original proposal was to advertise through AT_FLAGS. I've heard this suggestion of using
>> hwcap from multiple sources, so I am curious - what other *purely* software kernel features
>> are advertised using hwcap?
>
> It is not common, but on powerpc has both PPC_FEATURE2_HTM_NOSC and
> PPC_FEATURE2_HTM_NO_SUSPEND which is kernel behaviour regarding transaction
> memory state and syscall execution.
>
>>
>>>> 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 a proposal for Binutils and GCC after this
>>>> part gets finalized.
>>>
>>> If the idea is to fallback to executable stack for the case of underlying missing
>>> kernel support, which is the net gain in adding this requirement? My understanding
>>> it ABI bump should be used to fail early for the cases where the new binaries
>>> requires loader support that can not be provided (iFUNC or new relocations), not
>>> for hardening.
>>
>> It is not really hardening, the way glibc handles PT_GNU_STACK is along the lines of
>> 'may have non-executable stack' rather than 'must have non-executable stack'. However
>> the MIPS backend overrides *any* incoming PT_GNU_STACK permissions using the default (RWX)
>> permissions for MIPS, in effect enforcing 'will not have non-executable stack'. What is
>> being indicated here is a change in this default behaviour. The ABI version bump would
>> indicate whether PT_GNU_STACK permissions will be honoured (at least to the extent it is
>> by other architectures) or simply ignored (as it has historically been).
>>
>> IMO, the current behaviour of PT_GNU_STACK for MIPS is an anomaly in itself. What should
>> have been, is a rejection of non-executable PT_GNU_STACK at some level, instead of silently
>> overriding it in glibc. So are you of the opinion that this change in glibc behaviour is not
>> worth being published at all, or that it should be advertised using a different mechanism
>> instead of an ABI version bump?
>
> Since non-executable stack is tied with underlying kernel support rather than
> ABI, my suggestion is just to assume non-executable stack as default, without
> permission override, if glibc is configure for kernel higher than 3.8. We will
> need to live with old behaviour for default builds.
>

If I follow correctly this can be simplified to either require 4.8 
kernel when built with non-executable stack or force executable stack at 
build time for default (< 4.8) builds?

Just to be on the same page. This "old" behavior of overriding 
PT_GNU_STACK permission to RWX on mips is introduced in the second patch 
of the series.

What I understand is that previously you could build a glibc with RW 
PT_GNU_STACK and it may or may not work at runtime depending on if the 
kernel actually needed to use you stack for emulation and if you 
hardware is capable of enforcing NX page protection.

If linker, glibc or kernel did historically disallow RW PT_GNU_STACK we 
would not have this issue.

I interpret this abi version bump more as assurance that we are not 
running in the environment that might crash just because we requested
RW PT_GNU_STACK. (I'm ignoring existence of other libc implementations 
here.)

Best regards,

Dragan









^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/3] Mips support for PT_GNU_STACK
  2019-07-18 13:33     ` Adhemerval Zanella
@ 2019-07-18 20:00       ` Dragan Mladjenovic
  0 siblings, 0 replies; 14+ messages in thread
From: Dragan Mladjenovic @ 2019-07-18 20:00 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha@sourceware.org
  Cc: Joseph Myers, Carlos O'Donell, Maciej W . Rozycki,
	Faraz Shahbazker, Florian Weimer



On 18.07.2019. 15:33, Adhemerval Zanella wrote:
>
>
> On 17/07/2019 18:47, Dragan Mladjenovic wrote:
>>
>> On 17.07.2019. 21:43, Adhemerval Zanella wrote:
>>>
>>> On 15/07/2019 15:23, Dragan Mladjenovic wrote:
>>>> Hello everyone,
>>>>
>>>> Patches in this series are a 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 an instruction in the delay slot of an 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, the 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.
>>> I think checking the kernel version is the wrong approach, it prevents a distribution
>>> to backport the kernel fix without also applying a out-of-tree patch to fix it on glibc
>>> as well.  IMHO the proper way would be to make kernel advertise it through hwcap, as
>>> other architectures do for similar kernel features and not tie it to any specific
>>> version.
>>
>> First, thanks for the comments.
>>
>> I agree with everything and the original patch did propose the usage of AT_FLAGS
>> auxval for this purpose. But, sadly, the kernel still doesn't advertise this information
>> to the user mode and it would take some time for it to be back-ported even if it gets
>> implemented, let's say in next kernel release. Meanwhile, this approach, while lacking,
>> would allow for distributions to enable PT_GNU_STACK way earlier and to actually
>> enjoy the benefit of it on existing 4.8+ kernels while remaining compatible with pre-4.8
>> ones.
>
> It seems the kernel commit that actually added this support on mips was
> 432c6bacbd0c16ec210c43da411ccc3855c4c010.  Isn't there any other mechanism to check
> for kernel support without tying a kernel version check?
>
> The code seems to reserve an extra page for the delay slot emulation, could we
> infer based on vdso placement, page size, and maximum stack size?
>

Will have to check on this. So far it doesn't seem we can. I think that 
on pre-vdso kernels the same page was used for sigreturn trampoline. I 
either way even if we could do this, we could end up painting ourselves 
in a corner similar as x86_64 with vsyscalls page. It would become the 
part of userspace ABI. For now the emutrap page is just an 
implementation detail.


Best regards,

Dragan




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/3] Mips support for PT_GNU_STACK
  2019-07-18 19:49       ` Dragan Mladjenovic
@ 2019-07-18 20:54         ` Adhemerval Zanella
  0 siblings, 0 replies; 14+ messages in thread
From: Adhemerval Zanella @ 2019-07-18 20:54 UTC (permalink / raw)
  To: Dragan Mladjenovic, Faraz Shahbazker, libc-alpha@sourceware.org
  Cc: Joseph Myers, Carlos O'Donell, Maciej W . Rozycki



On 18/07/2019 16:49, Dragan Mladjenovic wrote:
> 
> 
> On 18.07.2019. 15:38, Adhemerval Zanella wrote:
>>
>>
>> On 17/07/2019 19:59, Faraz Shahbazker wrote:
>>> On 07/17/2019 12:43 PM, Adhemerval Zanella wrote:
>>>> I think checking the kernel version is the wrong approach, it prevents a distribution
>>>> to backport the kernel fix without also applying a out-of-tree patch to fix it on glibc
>>>> as well.  IMHO the proper way would be to make kernel advertise it through hwcap, as
>>>> other architectures do for similar kernel features and not tie it to any specific
>>>> version.
>>>
>>> The original proposal was to advertise through AT_FLAGS. I've heard this suggestion of using
>>> hwcap from multiple sources, so I am curious - what other *purely* software kernel features
>>> are advertised using hwcap?
>>
>> It is not common, but on powerpc has both PPC_FEATURE2_HTM_NOSC and
>> PPC_FEATURE2_HTM_NO_SUSPEND which is kernel behaviour regarding transaction
>> memory state and syscall execution.
>>
>>>
>>>>> 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 a proposal for Binutils and GCC after this
>>>>> part gets finalized.
>>>>
>>>> If the idea is to fallback to executable stack for the case of underlying missing
>>>> kernel support, which is the net gain in adding this requirement? My understanding
>>>> it ABI bump should be used to fail early for the cases where the new binaries
>>>> requires loader support that can not be provided (iFUNC or new relocations), not
>>>> for hardening.
>>>
>>> It is not really hardening, the way glibc handles PT_GNU_STACK is along the lines of
>>> 'may have non-executable stack' rather than 'must have non-executable stack'. However
>>> the MIPS backend overrides *any* incoming PT_GNU_STACK permissions using the default (RWX)
>>> permissions for MIPS, in effect enforcing 'will not have non-executable stack'. What is
>>> being indicated here is a change in this default behaviour. The ABI version bump would
>>> indicate whether PT_GNU_STACK permissions will be honoured (at least to the extent it is
>>> by other architectures) or simply ignored (as it has historically been).
>>>
>>> IMO, the current behaviour of PT_GNU_STACK for MIPS is an anomaly in itself. What should
>>> have been, is a rejection of non-executable PT_GNU_STACK at some level, instead of silently
>>> overriding it in glibc. So are you of the opinion that this change in glibc behaviour is not
>>> worth being published at all, or that it should be advertised using a different mechanism
>>> instead of an ABI version bump?
>>
>> Since non-executable stack is tied with underlying kernel support rather than
>> ABI, my suggestion is just to assume non-executable stack as default, without
>> permission override, if glibc is configure for kernel higher than 3.8. We will
>> need to live with old behaviour for default builds.
>>
> 
> If I follow correctly this can be simplified to either require 4.8 
> kernel when built with non-executable stack or force executable stack at 
> build time for default (< 4.8) builds?
> 
> Just to be on the same page. This "old" behavior of overriding 
> PT_GNU_STACK permission to RWX on mips is introduced in the second patch 
> of the series.
> 
> What I understand is that previously you could build a glibc with RW 
> PT_GNU_STACK and it may or may not work at runtime depending on if the 
> kernel actually needed to use you stack for emulation and if you 
> hardware is capable of enforcing NX page protection.
> 
> If linker, glibc or kernel did historically disallow RW PT_GNU_STACK we 
> would not have this issue.
> 
> I interpret this abi version bump more as assurance that we are not 
> running in the environment that might crash just because we requested
> RW PT_GNU_STACK. (I'm ignoring existence of other libc implementations 
> here.)

My understanding is ELFOSABI_SYSV check is only required if we build glibc
with support for kernel older than 3.8, to make newer binaries fail early 
on kernels that might not honour the PT_GNU_STACK.

So indeed it might a good idea to bump ELFOSABI_SYSV for to handle this case.



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-07-18 20:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-15 18:23 [PATCH v2 0/3] Mips support for PT_GNU_STACK Dragan Mladjenovic
2019-07-16 11:15 ` [PATCH v2 1/3] [ELF] Allow the machine support to enforce executable stack Dragan Mladjenovic
2019-07-16 11:16   ` [PATCH v2 2/3] [MIPS] Define DL_EXEC_STACK_OVERRIDE Dragan Mladjenovic
2019-07-16 11:16   ` [PATCH v2 3/3] [RFC][MIPS] Define GNU_STACK ABI Dragan Mladjenovic
2019-07-17 19:43 ` [PATCH v2 0/3] Mips support for PT_GNU_STACK Adhemerval Zanella
2019-07-17 21:47   ` Dragan Mladjenovic
2019-07-18 13:33     ` Adhemerval Zanella
2019-07-18 20:00       ` Dragan Mladjenovic
2019-07-17 22:59   ` Faraz Shahbazker
2019-07-18 13:38     ` Adhemerval Zanella
2019-07-18 15:34       ` Faraz Shahbazker
2019-07-18 19:49       ` Dragan Mladjenovic
2019-07-18 20:54         ` Adhemerval Zanella
2019-07-18 14:50   ` Maciej W. Rozycki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).