Date: Tue, 10 Oct 2017 18:19:00 -0700 From: Laura Abbott <labbott@...hat.com> To: Alexander Popov <alex.popov@...ux.com>, kernel-hardening@...ts.openwall.com, keescook@...omium.org, pageexec@...email.hu, spender@...ecurity.net, tycho@...ker.com, Mark Rutland <mark.rutland@....com>, Ard Biesheuvel <ard.biesheuvel@...aro.org>, Andy Lutomirski <luto@...capital.net>, x86@...nel.org Subject: Re: [PATCH RFC v4 0/3] Introduce the STACKLEAK feature and a test for it On 10/04/2017 03:55 PM, Alexander Popov wrote: > This is the 4th version of the patch introducing STACKLEAK to the mainline > kernel. STACKLEAK is a security feature developed by Grsecurity/PaX (kudos > to them), which: > - reduces the information that can be revealed by some kernel stack leak bugs > (e.g. CVE-2016-4569 and CVE-2016-4578); > - blocks some uninitialized stack variable attacks (e.g. CVE-2010-2963); > - introduces some runtime checks for kernel stack overflow detection. > > Further work: > - think of erasing the kernel stack after invoking EFI runtime services; > - try to port STACKLEAK to arm64 (Laura Abbott is working on it). > > Changes in v4 > > 1. Introduced the CONFIG_STACKLEAK_TRACK_MIN_SIZE parameter instead of > hard-coded track-lowest-sp. > > 2. Carefully looked into the assertions in track_stack() and check_alloca(). > - Fixed the incorrect BUG() condition in track_stack(), thanks to Tycho > Andersen. Also disabled that check if CONFIG_VMAP_STACK is enabled. > - Fixed the surplus and erroneous code for calculating stack_left in > check_alloca() on x86_64. That code repeats the work which is already > done in get_stack_info() and it misses the fact that different > exception stacks on x86_64 have different size. > > 3. Introduced two lkdtm tests for the STACKLEAK feature (developed together > with Tycho). > > 4. Added info about STACKLEAK to Documentation/security/self-protection.rst. > > 5. Improved the comments. > > 6. Decided not to change "unsigned long sp = (unsigned long)&sp" to > current_stack_pointer. The original variant is more platform independent > since current_stack_pointer has different type on x86 and arm. > > Changes in v3 > > 1. Added the detailed comments describing the STACKLEAK gcc plugin. > Read the plugin from bottom up, like you do for Linux kernel drivers. > Additional information: > - gcc internals documentation, which is available from gcc sources; > - wonderful slides by Diego Novillo > https://www.airs.com/dnovillo/200711-GCC-Internals/ > - nice introduction to gcc plugins at LWN > https://lwn.net/Articles/457543/ > > 2. Improved the commit message and Kconfig description according the > feedback from Kees Cook. Also added the original notice describing > the performance impact of the STACKLEAK feature. > > 3. Removed arch-specific ix86_cmodel check from stackleak_track_stack_gate(). > It caused skipping the kernel code instrumentation for i386. > > 4. Fixed a minor mistake in stackleak_tree_instrument_execute(). > First versions of the plugin used ENTRY_BLOCK_PTR_FOR_FN(cfun)->next_bb > to get the basic block with the function prologue. That was not correct > since the control flow graph edge from the ENTRY_BLOCK_PTR doesn't always > go to that basic block. > > So later it was changed it to single_succ(ENTRY_BLOCK_PTR_FOR_FN(cfun)), > but not completely. next_bb was still used for entry_bb assignment, > which could cause the wrong value of the prologue_instrumented variable. > > I've reported this issue to Grsecurity and proposed the fix for it, but > unfortunately didn't get any reply. > > 5. Introduced the STACKLEAK_POISON macro and renamed the config option > according the feedback from Kees Cook. > > > Alexander Popov (3): > gcc-plugins: Add STACKLEAK erasing the kernel stack at the end of > syscalls > lkdtm: Add a test for STACKLEAK > doc: self-protection: Add information about STACKLEAK feature > > Documentation/security/self-protection.rst | 23 +- > arch/Kconfig | 39 +++ > arch/x86/Kconfig | 1 + > arch/x86/entry/common.c | 17 +- > arch/x86/entry/entry_32.S | 69 +++++ > arch/x86/entry/entry_64.S | 95 +++++++ > arch/x86/entry/entry_64_compat.S | 8 + > arch/x86/include/asm/processor.h | 4 + > arch/x86/kernel/asm-offsets.c | 9 + > arch/x86/kernel/dumpstack_32.c | 12 + > arch/x86/kernel/dumpstack_64.c | 15 ++ > arch/x86/kernel/process_32.c | 5 + > arch/x86/kernel/process_64.c | 5 + > drivers/misc/Makefile | 3 + > drivers/misc/lkdtm.h | 4 + > drivers/misc/lkdtm_core.c | 2 + > drivers/misc/lkdtm_stackleak.c | 139 ++++++++++ > fs/exec.c | 30 +++ > include/linux/compiler.h | 4 + > scripts/Makefile.gcc-plugins | 3 + > scripts/gcc-plugins/stackleak_plugin.c | 397 +++++++++++++++++++++++++++++ > 21 files changed, 872 insertions(+), 12 deletions(-) > create mode 100644 drivers/misc/lkdtm_stackleak.c > create mode 100644 scripts/gcc-plugins/stackleak_plugin.c > I tried this series with CVE-2017-14954 . That particular bug is not helped here because the poisoning has already been overwritten by other leaf functions. Given the call stack this may be a particularly bad case but I'm wondering how common this might be if we're only erasing at the end of a system call. One previous copy_to_user which has to go through the fault path can get fairly deep. Thanks, Laura
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.