Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 13 Oct 2017 10:26:40 -0700
From: Andy Lutomirski <luto@...capital.net>
To: Alexander Popov <alex.popov@...ux.com>
Cc: Laura Abbott <labbott@...hat.com>, 
	"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, Kees Cook <keescook@...omium.org>, 
	PaX Team <pageexec@...email.hu>, Brad Spengler <spender@...ecurity.net>, 
	Tycho Andersen <tycho@...ker.com>, Mark Rutland <mark.rutland@....com>, 
	Ard Biesheuvel <ard.biesheuvel@...aro.org>, X86 ML <x86@...nel.org>
Subject: Re: [PATCH RFC v4 0/3] Introduce the STACKLEAK feature and a test for it

On Wed, Oct 11, 2017 at 9:29 AM, Alexander Popov <alex.popov@...ux.com> wrote:
> On 11.10.2017 05:31, Andy Lutomirski wrote:
>>
>>
>>> On Oct 10, 2017, at 6:19 PM, Laura Abbott <labbott@...hat.com> wrote:
>>>
>>>> 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.
>
> Laura, thanks for your observation. I've tested Brad's PoC with STACKLEAK too
> and see similar results. There is only one STACKLEAK_POISON value in the leaked
> data. Other leaked data is put on the stack by the current syscall.
>
> I don't know any statistics on infoleaks and I can't say how many of them would
> be neutralized by STACKLEAK. But, anyway, it is be better than nothing for those
> who accept the STACKLEAK performance penalty.
>
>> On x86, the bad guys can force this is using 32-bit fast syscalls for *any*
>> syscall.  I suppose we could wipe the stack on the way out of exception
>> handlers, too...
>
> Andy, excuse me, could you elaborate on that? Do you mean that there are some
> more cases when erase_kstack() should be called?

Kinda.  I was thinking that certain exception handlers should erase
their portion of the stack (i.e. from their entry frame to the bottom)
when they return back to *kernel* mode.  On x86, this should include
at least #PF and probably #GP and #DB, too.  #DB is weird, so it might
not actually be needed.

>
>> OTOH, I think we should consider KASLR to be worthless against local
>> attackers in general.  See all the papers on TSX timing leaks, etc.  It's a
>> nice defense against a limited class of remote attack.  And the plugin is
>> still plausibly useful to protect more serious  secrets.
>
> Thanks for that remark! Could you give some examples of such secrets which
> should not leak to the userspace?

Crypto secrets.  Stuff in /dev/urandom, for example.  Keys, too.

>
> Best regards,
> Alexander



-- 
Andy Lutomirski
AMA Capital Management, LLC

Powered by blists - more mailing lists

Your e-mail address:

Powered by Openwall GNU/*/Linux - Powered by OpenVZ