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