lkml.org 
[lkml]   [2022]   [Feb]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] MIPS: Modernize READ_IMPLIES_EXEC
Friendly ping. :)

On Wed, Sep 01, 2021 at 12:42:08PM -0700, Kees Cook wrote:
> I'm doing some thread necromancy of
> https://lore.kernel.org/lkml/202007081624.82FA0CC1EA@keescook/
>
> x86, arm64, and arm32 adjusted their READ_IMPLIES_EXEC logic to better
> align with the safer defaults and the interactions with other mappings,
> which I illustrated with this comment on x86:
>
> /*
> * An executable for which elf_read_implies_exec() returns TRUE will
> * have the READ_IMPLIES_EXEC personality flag set automatically.
> *
> * The decision process for determining the results are:
> *
> * CPU: | lacks NX* | has NX, ia32 | has NX, x86_64 |
> * ELF: | | | |
> * ---------------------|------------|------------------|----------------|
> * missing PT_GNU_STACK | exec-all | exec-all | exec-none |
> * PT_GNU_STACK == RWX | exec-stack | exec-stack | exec-stack |
> * PT_GNU_STACK == RW | exec-none | exec-none | exec-none |
> *
> * exec-all : all PROT_READ user mappings are executable, except when
> * backed by files on a noexec-filesystem.
> * exec-none : only PROT_EXEC user mappings are executable.
> * exec-stack: only the stack and PROT_EXEC user mappings are
> * executable.
> *
> * *this column has no architectural effect: NX markings are ignored by
> * hardware, but may have behavioral effects when "wants X" collides with
> * "cannot be X" constraints in memory permission flags, as in
> * https://lkml.kernel.org/r/20190418055759.GA3155@mellanox.com
> *
> */
>
> For MIPS, the "lacks NX" above is the "!cpu_has_rixi" check. On x86,
> we decided that the READ_IMPLIES_EXEC flag needed to reflect the
> expectations, not the architectural behavior due to bad interactions
> as noted above, as always returning "1" on non-NX hardware breaks
> some mappings.
>
> The other part of the issue is "what does the MIPS toolchain do for
> PT_GNU_STACK?" The answer seems to be "by default, include PT_GNU_STACK,
> but mark it executable" (likely due to concerns over non-NX hardware):
>
> $ mipsel-linux-gnu-gcc -o hello_world hello_world.c
> $ llvm-readelf -lW hellow_world | grep GNU_STACK
> GNU_STACK 0x000000 0x00000000 0x00000000 0x00000 0x00000 RWE 0x10
>
> Given that older hardware doesn't support non-executable memory, it
> seems safe to make the "PT_GNU_STACK is absent" logic mean "assume
> non-executable", but this might break very old software running on
> modern MIPS. This situation matches the ia32-on-x86_64 logic x86
> uses (which assumes needing READ_IMPLIES_EXEC in that situation). But
> modern toolchains on modern MIPS hardware should follow a safer default
> (assume NX stack).
>
> A follow-up to this change would be to switch the MIPS toolchain to emit
> a non-executable PT_GNU_STACK, as this seems to be unneeded.
>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
> Cc: Xuefeng Li <lixuefeng@loongson.cn>
> Cc: Juxin Gao <gaojuxin@loongson.cn>
> Cc: linux-mips@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> arch/mips/kernel/elf.c | 16 +++++-----------
> 1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
> index 7b045d2a0b51..5582a4ca1e9e 100644
> --- a/arch/mips/kernel/elf.c
> +++ b/arch/mips/kernel/elf.c
> @@ -328,16 +328,10 @@ void mips_set_personality_nan(struct arch_elf_state *state)
>
> int mips_elf_read_implies_exec(void *elf_ex, int exstack)
> {
> - if (exstack != EXSTACK_DISABLE_X) {
> - /* The binary doesn't request a non-executable stack */
> - return 1;
> - }
> -
> - if (!cpu_has_rixi) {
> - /* The CPU doesn't support non-executable memory */
> - return 1;
> - }
> -
> - return 0;
> + /*
> + * Set READ_IMPLIES_EXEC only on non-NX systems that
> + * do not request a specific state via PT_GNU_STACK.
> + */
> + return (!cpu_has_rixi && exstack == EXSTACK_DEFAULT);
> }
> EXPORT_SYMBOL(mips_elf_read_implies_exec);
> --
> 2.30.2
>

--
Kees Cook

\
 
 \ /
  Last update: 2022-02-23 00:50    [W:0.091 / U:0.236 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site