From patchwork Wed Aug 16 21:16:36 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tycho Andersen X-Patchwork-Id: 9904611 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B4EE960244 for ; Wed, 16 Aug 2017 21:17:08 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A603228A47 for ; Wed, 16 Aug 2017 21:17:08 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9A60328A55; Wed, 16 Aug 2017 21:17:08 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id 2867F28A47 for ; Wed, 16 Aug 2017 21:17:06 +0000 (UTC) Received: (qmail 5338 invoked by uid 550); 16 Aug 2017 21:16:50 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 5314 invoked from network); 16 Aug 2017 21:16:50 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=docker.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=34MkiXrK+wkSoaiBOSnFf8n3Gf4oYQpzj6P3bhcTyhQ=; b=Bsr1vKp8nlbGVrMOyRjW6T03NMdVyMHpRn+ur4Ifieo4akkTJEv+1VbaBeZe4ejjXg cbLXnB91swntbADeEPeSy1f/2tjgEEkCm4KA65kup8XqB9nY6thrYERiIxQmCnsKdxQT M7GjHNPVcQEsUqyFBxKp5XHf0tfeU0bGiVtPo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=34MkiXrK+wkSoaiBOSnFf8n3Gf4oYQpzj6P3bhcTyhQ=; b=rhUVnY5fSkjzwtyHlFQRoOmcidqqHJADDeX92YF9A8FFDklixcnmd9HNJ/4XZ+OLRz mwef51H/77ZJGomRHcPqBZ7gV75hhRugipEjKuzdldTgQiWpvvR81U25lUsLdVfYLlYX RO8XU0ogGzajTYuCgM0AhsejQwokv4wd8v+entKPvwECZQG156+P1nU+Ggb7IrU1vF0M 5mC+3KtOOJn4fOhzGtMQKPd2BPuilLYO7xcGtaUACLBH6TQerQBRbScY6rB7esYK9AMk jq0TSWplkWpQbbK4RZ8Ug0vMq6wc6WWHcxdsoRZsntHjfGbbRIGrnzTJOsHiam/riQaJ zGNw== X-Gm-Message-State: AHYfb5hSBRb/3jNo4d7sh+PrSlVCsDYeiRaY+ziew5E7PxxzamaoNq4g YzVLPkFbxwjvs9wE X-Received: by 10.36.51.212 with SMTP id k203mr393139itk.136.1502918197995; Wed, 16 Aug 2017 14:16:37 -0700 (PDT) Date: Wed, 16 Aug 2017 15:16:36 -0600 From: Tycho Andersen To: Alexander Popov Cc: kernel-hardening@lists.openwall.com, keescook@chromium.org, pageexec@freemail.hu, spender@grsecurity.net, Laura Abbott , Mark Rutland , Ard Biesheuvel , x86@kernel.org, Andy Lutomirski Message-ID: <20170816211636.4zwublkvpn6hbo5n@smitten> References: <1499883471-23822-1-git-send-email-alex.popov@linux.com> <20170815033834.2qbjj2of62udqyz3@smitten> <93382ef8-105f-eed4-0d97-0b1a66a047e6@linux.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <93382ef8-105f-eed4-0d97-0b1a66a047e6@linux.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: [kernel-hardening] Re: [PATCH RFC v3 1/1] gcc-plugins: Add stackleak feature erasing the kernel stack at the end of syscalls X-Virus-Scanned: ClamAV using ClamSMTP Hi Alexander, On Wed, Aug 16, 2017 at 11:47:44PM +0300, Alexander Popov wrote: > Hello Tycho, > > On 15.08.2017 06:38, Tycho Andersen wrote: > > On Wed, Jul 12, 2017 at 09:17:51PM +0300, Alexander Popov wrote: > >> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK > >> +void __used track_stack(void) > >> +{ > >> + unsigned long sp = (unsigned long)&sp; > >> + > >> + if (sp < current->thread.lowest_stack && > >> + sp >= (unsigned long)task_stack_page(current) + > >> + 2 * sizeof(unsigned long)) { > >> + current->thread.lowest_stack = sp; > >> + } > >> + > >> + if (unlikely((sp & ~(THREAD_SIZE - 1)) < (THREAD_SIZE / 16))) > > > > I think this check is wrong, the lhs should be > > (sp & (THREAD_SIZE - 1)). Otherwise, we just check that the upper bits > > of the stack are < THREAD_SIZE / 16, which they never will be. > > Thank you, I think you are right! > > I can additionally notice that this erroneous check is not a part of PaX patch, > it is introduced by Grsecurity patch. > > Thanks again, I'll fix and annotate it in the next version of the patch. > > Did you manage to create a test for the correct check which hits the BUG()? Yes, see below. I've fixed all of the review feedback from the last time I posted it too. Feel free to add it to your tree and post it w/ the next version if that makes the most sense. Cheers, Tycho From 2c8a8f96a331b63e3aa52388fab9f111c516bf1c Mon Sep 17 00:00:00 2001 From: Tycho Andersen Date: Thu, 8 Jun 2017 12:43:07 -0600 Subject: [PATCH] lkdtm: add a test for STACKLEAK plugin There are two tests here, one to test that the BUG() in check_alloca is hit correctly, and the other to test that the BUG() in track_stack is hit correctly. Ideally we'd also be able to check end-to-end that a syscall results in an entirely poisoned stack, but I'm not sure how to do a syscall from lkdtm. Signed-off-by: Tycho Andersen --- drivers/misc/Makefile | 1 + drivers/misc/lkdtm.h | 4 ++ drivers/misc/lkdtm_core.c | 2 + drivers/misc/lkdtm_stackleak.c | 135 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 142 insertions(+) diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 81ef3e67acc9..805e4f06011a 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -61,6 +61,7 @@ lkdtm-$(CONFIG_LKDTM) += lkdtm_heap.o lkdtm-$(CONFIG_LKDTM) += lkdtm_perms.o lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o lkdtm-$(CONFIG_LKDTM) += lkdtm_usercopy.o +lkdtm-$(CONFIG_LKDTM) += lkdtm_stackleak.o KCOV_INSTRUMENT_lkdtm_rodata.o := n diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h index 3b4976396ec4..3b67cc4a070b 100644 --- a/drivers/misc/lkdtm.h +++ b/drivers/misc/lkdtm.h @@ -64,4 +64,8 @@ void lkdtm_USERCOPY_STACK_FRAME_FROM(void); void lkdtm_USERCOPY_STACK_BEYOND(void); void lkdtm_USERCOPY_KERNEL(void); +/* lkdtm_stackleak.c */ +void lkdtm_STACKLEAK_ALLOCA(void); +void lkdtm_STACKLEAK_BIG_FRAME(void); + #endif diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c index 42d2b8e31e6b..f42b346bdf5c 100644 --- a/drivers/misc/lkdtm_core.c +++ b/drivers/misc/lkdtm_core.c @@ -235,6 +235,8 @@ struct crashtype crashtypes[] = { CRASHTYPE(USERCOPY_STACK_FRAME_FROM), CRASHTYPE(USERCOPY_STACK_BEYOND), CRASHTYPE(USERCOPY_KERNEL), + CRASHTYPE(STACKLEAK_ALLOCA), + CRASHTYPE(STACKLEAK_BIG_FRAME), }; diff --git a/drivers/misc/lkdtm_stackleak.c b/drivers/misc/lkdtm_stackleak.c new file mode 100644 index 000000000000..daae36e0432e --- /dev/null +++ b/drivers/misc/lkdtm_stackleak.c @@ -0,0 +1,135 @@ +/* + * This file tests a few aspects of the stackleak compiler plugin: + * - the current task stack somewhere below lowest_stack is properly canaried + * - small allocas are allowed properly via check_alloca() + * - big allocations that exhaust the stack are BUG()s + * - function calls whose stack frames blow the stack are BUG()s + * + * Copyright (C) Docker, Inc. 2017 + * + * Author: Tycho Andersen + */ + +#include "lkdtm.h" + +#include +#include + +/* for security_inode_init_security */ +#include + +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK +static bool check_poison(unsigned long *ptr, unsigned long n) +{ + unsigned long i; + + for (i = 1; i < n; i++) { + if (*(ptr - i) != STACKLEAK_POISON) + return false; + } + + return true; +} + +static bool check_my_stack(void) +{ + unsigned long *lowest, left, i; + + lowest = &i; + if ((unsigned long *) current->thread.lowest_stack < lowest) + lowest = (unsigned long *) current->thread.lowest_stack; + + left = (unsigned long) lowest % THREAD_SIZE; + + /* See note in arch/x86/entry/entry_64.S about the or; the bottom two + * qwords are not + */ + left -= 2 * sizeof(unsigned long); + + /* let's count the number of canaries, not bytes */ + left /= sizeof(unsigned long); + + for (i = 0; i < left; i++) { + if (*(lowest - i) != STACKLEAK_POISON) + continue; + + if (i > 32) + pr_warn_once("More than 256 bytes not canaried?"); + + if (!check_poison(lowest - i, 16)) + continue; + + break; + } + + if (i == left) { + pr_warn("didn't find canary?"); + return false; + } + + if (check_poison((unsigned long *) lowest - i, left - i)) { + pr_info("current stack poisoned correctly\n"); + return true; + } else { + pr_err("current stack not poisoned correctly\n"); + return false; + } +} +#else +bool check_my_stack(void) +{ + return false; +} +#endif + +static noinline void do_alloca(unsigned long size, void (*todo)(void)) +{ + char buf[size]; + + if (todo) + todo(); + + /* so this doesn't get inlined or optimized out */ + snprintf(buf, size, "hello world\n"); +} + +/* Check the BUG() in check_alloca() */ +void lkdtm_STACKLEAK_ALLOCA(void) +{ + unsigned long left = (unsigned long) &left % THREAD_SIZE; + + if (!check_my_stack()) + return; + + // try a small allocation to see if it works + do_alloca(16, NULL); + pr_info("small allocation successful\n"); + + + pr_info("attempting large alloca of %lu\n", left); + do_alloca(left, NULL); + pr_warn("alloca succeded?\n"); +} + +static void use_some_stack(void) { + + /* Note: this needs to be a(n exported) function that has track_stack + * inserted, i.e. it isn't in the various sections restricted by + * stackleak_track_stack_gate. + */ + security_inode_init_security(NULL, NULL, NULL, NULL, NULL); +} + +/* Note that the way this test fails is kind of ugly; it hits the BUG() in + * track_stack, but then the BUG() handler blows the stack and hits the stack + * guard page. + */ +void lkdtm_STACKLEAK_BIG_FRAME(void) +{ + unsigned long left = (unsigned long) &left % THREAD_SIZE; + + /* use almost all of the stack, minus the buffer space allowed in + * track_stack and the space used by track_stack itself + */ + do_alloca(left - THREAD_SIZE / 16 - sizeof(unsigned long), use_some_stack); +}