[kernel-hardening] Re: [PATCH RFC v4 0/3] Introduce the STACKLEAK feature and a test for it

[Laura Abbott <labbott@redhat.com>]

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