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

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

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
>