| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 16 May 2016 12:58:04 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Kees Cook <keescook@...omium.org>
Cc: Hector Marco-Gisbert <hecmargi@....es>,
Andy Lutomirski <luto@...nel.org>,
Andi Kleen <ak@...ux.intel.com>,
LKML <linux-kernel@...r.kernel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>,
"x86@...nel.org" <x86@...nel.org>, Brian Gerst <brgerst@...il.com>,
Borislav Petkov <bp@...e.de>,
Huaitong Han <huaitong.han@...el.com>,
Ismael Ripoll Ripoll <iripoll@....es>
Subject: Re: [PATCH] x86_64: Disabling read-implies-exec when the stack is
executable
* Kees Cook <keescook@...omium.org> wrote:
> On Wed, May 11, 2016 at 3:45 AM, Hector Marco-Gisbert <hecmargi@....es> wrote:
> > The READ_IMPLIES_EXEC personality was removed in 2005 for 64-bit processes,
> > (commit a3cc2546a54361b86b73557df5b85c4fc3fc27c3 form history.git).
> >
> > But it's still possible to have all readable areas with EXEC permissions by
> > setting the stack as executable in 64-bit ELF executables (also in 32-bit).
>
> My memory is fuzzy here, but IIRC, RIE is needed for loading binaries
> that have no concept of no-exec permissions. In those cases, there's
> no way to tell if the process expected to need execute permissions in
> arbitrary memory regions.
>
> > This is because the macro elf_read_implies_exec() does not distinguish
> > between 32 and 64-bit executables: when the stack is executable then the
> > read-implies-exec personality is set (enabled) to the process.
>
> However, I would tend to agree: RIE should only be needed on 32-bit
> since 64-bit started its life knowing about no-exec permissions.
>
> set_personality_64bit()'s (which is confusingly just an initializer
> and not called during the personality() syscall) comment about this
> makes no sense to me:
>
> /* TBD: overwrites user setup. Should have two bits.
> But 64bit processes have always behaved this way,
> so it's not too bad. The main problem is just that
> 32bit childs are affected again. */
> current->personality &= ~READ_IMPLIES_EXEC;
JFYI, that obfuscated comment was added over a decade ago to the x86_64 tree, see
the commit from the historic git tree attached below.
Thanks,
Ingo
=================>
>From a3cc2546a54361b86b73557df5b85c4fc3fc27c3 Mon Sep 17 00:00:00 2001
From: Andi Kleen <ak@...e.de>
Date: Wed, 9 Feb 2005 22:40:57 -0800
Subject: [PATCH] [PATCH] Force read implies exec for all 32bit processes in x86-64
This effectively enables executable stack and executable heap for all 32bit
programs on x86-64, except if noexec32=on is specified.
This does not support changing this with personality right now, this would
need more intrusive changes. A 64bit process will always turn it off and a
32bit process turn it on.
Also readd the noexec32=on option to turn this off and fix a minor bug in
noexec=... (would be reported as unknown option)
Signed-off-by: Andi Kleen <ak@...e.de>
Signed-off-by: Andrew Morton <akpm@...l.org>
Signed-off-by: Linus Torvalds <torvalds@...l.org>
---
arch/x86_64/ia32/ia32_binfmt.c | 4 ++++
arch/x86_64/kernel/process.c | 6 ++++++
arch/x86_64/kernel/setup64.c | 25 +++++++++++++++++++++++--
include/asm-x86_64/pgtable.h | 2 +-
4 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/arch/x86_64/ia32/ia32_binfmt.c b/arch/x86_64/ia32/ia32_binfmt.c
index 445717b9c66b..93d568dfa762 100644
--- a/arch/x86_64/ia32/ia32_binfmt.c
+++ b/arch/x86_64/ia32/ia32_binfmt.c
@@ -249,6 +249,8 @@ elf_core_copy_task_xfpregs(struct task_struct *t, elf_fpxregset_t *xfpu)
#define elf_check_arch(x) \
((x)->e_machine == EM_386)
+extern int force_personality32;
+
#define ELF_EXEC_PAGESIZE PAGE_SIZE
#define ELF_HWCAP (boot_cpu_data.x86_capability[0])
#define ELF_PLATFORM ("i686")
@@ -262,6 +264,8 @@ do { \
set_thread_flag(TIF_ABI_PENDING); \
else \
clear_thread_flag(TIF_ABI_PENDING); \
+ /* XXX This overwrites the user set personality */ \
+ current->personality |= force_personality32; \
} while (0)
/* Override some function names */
diff --git a/arch/x86_64/kernel/process.c b/arch/x86_64/kernel/process.c
index bbe26dda5e79..3a3522b9c885 100644
--- a/arch/x86_64/kernel/process.c
+++ b/arch/x86_64/kernel/process.c
@@ -577,6 +577,12 @@ void set_personality_64bit(void)
/* Make sure to be in 64bit mode */
clear_thread_flag(TIF_IA32);
+
+ /* TBD: overwrites user setup. Should have two bits.
+ But 64bit processes have always behaved this way,
+ so it's not too bad. The main problem is just that
+ 32bit childs are affected again. */
+ current->personality &= ~READ_IMPLIES_EXEC;
}
asmlinkage long sys_fork(struct pt_regs *regs)
diff --git a/arch/x86_64/kernel/setup64.c b/arch/x86_64/kernel/setup64.c
index 248a86a80366..b5305b04bc40 100644
--- a/arch/x86_64/kernel/setup64.c
+++ b/arch/x86_64/kernel/setup64.c
@@ -50,7 +50,7 @@ Control non executable mappings for 64bit processes.
on Enable(default)
off Disable
*/
-void __init nonx_setup(const char *str)
+int __init nonx_setup(char *str)
{
if (!strncmp(str, "on", 2)) {
__supported_pte_mask |= _PAGE_NX;
@@ -58,8 +58,29 @@ void __init nonx_setup(const char *str)
} else if (!strncmp(str, "off", 3)) {
do_not_nx = 1;
__supported_pte_mask &= ~_PAGE_NX;
- }
+ }
+ return 0;
}
+__setup("noexec=", nonx_setup); /* parsed early actually */
+
+int force_personality32 = READ_IMPLIES_EXEC;
+
+/* noexec32=on|off
+Control non executable heap for 32bit processes.
+To control the stack too use noexec=off
+
+on PROT_READ does not imply PROT_EXEC for 32bit processes
+off PROT_READ implies PROT_EXEC (default)
+*/
+static int __init nonx32_setup(char *str)
+{
+ if (!strcmp(str, "on"))
+ force_personality32 &= ~READ_IMPLIES_EXEC;
+ else if (!strcmp(str, "off"))
+ force_personality32 |= READ_IMPLIES_EXEC;
+ return 0;
+}
+__setup("noexec32=", nonx32_setup);
/*
* Great future plan:
diff --git a/include/asm-x86_64/pgtable.h b/include/asm-x86_64/pgtable.h
index c5773fd9b3d4..1e2cc99aebd8 100644
--- a/include/asm-x86_64/pgtable.h
+++ b/include/asm-x86_64/pgtable.h
@@ -20,7 +20,7 @@ extern unsigned long __supported_pte_mask;
#define swapper_pg_dir init_level4_pgt
-extern void nonx_setup(const char *str);
+extern int nonx_setup(char *str);
extern void paging_init(void);
extern void clear_kernel_mapping(unsigned long addr, unsigned long size);
Powered by blists - more mailing lists