Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Thu, 26 Jul 2018 09:08:08 -0700
From: Kees Cook <keescook@...gle.com>
To: Alexander Popov <alex.popov@...ux.com>
Cc: Kernel Hardening <kernel-hardening@...ts.openwall.com>, PaX Team <pageexec@...email.hu>, 
	Brad Spengler <spender@...ecurity.net>, Ingo Molnar <mingo@...nel.org>, 
	Andy Lutomirski <luto@...nel.org>, Tycho Andersen <tycho@...ho.ws>, Laura Abbott <labbott@...hat.com>, 
	Mark Rutland <mark.rutland@....com>, Ard Biesheuvel <ard.biesheuvel@...aro.org>, 
	Borislav Petkov <bp@...en8.de>, Richard Sandiford <richard.sandiford@....com>, 
	Thomas Gleixner <tglx@...utronix.de>, "H . Peter Anvin" <hpa@...or.com>, 
	Peter Zijlstra <a.p.zijlstra@...llo.nl>, "Dmitry V . Levin" <ldv@...linux.org>, 
	Emese Revfy <re.emese@...il.com>, Jonathan Corbet <corbet@....net>, 
	Andrey Ryabinin <aryabinin@...tuozzo.com>, 
	"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>, Thomas Garnier <thgarnie@...gle.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Alexei Starovoitov <ast@...nel.org>, Josef Bacik <jbacik@...com>, 
	Masami Hiramatsu <mhiramat@...nel.org>, Nicholas Piggin <npiggin@...il.com>, 
	Al Viro <viro@...iv.linux.org.uk>, "David S . Miller" <davem@...emloft.net>, 
	Ding Tianhong <dingtianhong@...wei.com>, David Woodhouse <dwmw@...zon.co.uk>, 
	Josh Poimboeuf <jpoimboe@...hat.com>, Steven Rostedt <rostedt@...dmis.org>, 
	Dominik Brodowski <linux@...inikbrodowski.net>, Juergen Gross <jgross@...e.com>, 
	Linus Torvalds <torvalds@...ux-foundation.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Dan Williams <dan.j.williams@...el.com>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, Mathias Krause <minipli@...glemail.com>, 
	Vikas Shivappa <vikas.shivappa@...ux.intel.com>, Kyle Huey <me@...ehuey.com>, 
	Dmitry Safonov <dsafonov@...tuozzo.com>, Will Deacon <will.deacon@....com>, 
	Arnd Bergmann <arnd@...db.de>, Florian Weimer <fweimer@...hat.com>, 
	Boris Lukashev <blukashev@...pervictus.com>, Andrey Konovalov <andreyknvl@...gle.com>, 
	X86 ML <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v14 7/7] stackleak: Allow runtime disabling of kernel
 stack erasing

On Thu, Jul 26, 2018 at 4:11 AM, Alexander Popov <alex.popov@...ux.com> wrote:
> Introduce CONFIG_STACKLEAK_RUNTIME_DISABLE option, which provides
> 'stack_erasing' sysctl. It can be used in runtime to control kernel
> stack erasing for kernels built with CONFIG_GCC_PLUGIN_STACKLEAK.
>
> Suggested-by: Ingo Molnar <mingo@...nel.org>
> Signed-off-by: Alexander Popov <alex.popov@...ux.com>
> ---
>  Documentation/sysctl/kernel.txt | 18 ++++++++++++++++++
>  include/linux/stackleak.h       |  6 ++++++
>  kernel/stackleak.c              | 38 ++++++++++++++++++++++++++++++++++++++
>  kernel/sysctl.c                 | 15 ++++++++++++++-
>  scripts/gcc-plugins/Kconfig     |  8 ++++++++
>  5 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index eded671d..1feae79 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -87,6 +87,7 @@ show up in /proc/sys/kernel:
>  - shmmni
>  - softlockup_all_cpu_backtrace
>  - soft_watchdog
> +- stack_erasing

I like the renaming to avoid the double-negative. ("disable bypassing"
is not as clear as "feature enabled or not")

>  - stop-a                      [ SPARC only ]
>  - sysrq                       ==> Documentation/admin-guide/sysrq.rst
>  - sysctl_writes_strict
> @@ -962,6 +963,23 @@ detect a hard lockup condition.
>
>  ==============================================================
>
> +stack_erasing
> +
> +This parameter can be used to control kernel stack erasing at the end
> +of syscalls for kernels built with CONFIG_GCC_PLUGIN_STACKLEAK.
> +
> +That erasing reduces the information which kernel stack leak bugs
> +can reveal and blocks some uninitialized stack variable attacks.
> +The tradeoff is the performance impact: on a single CPU system kernel
> +compilation sees a 1% slowdown, other systems and workloads may vary.
> +
> +  0: kernel stack erasing is disabled, STACKLEAK_METRICS are not updated.
> +
> +  1: kernel stack erasing is enabled (default), it is performed before
> +     returning to the userspace at the end of syscalls.
> +
> +==============================================================
> +
>  tainted:
>
>  Non-zero if the kernel has been tainted. Numeric values, which can be
> diff --git a/include/linux/stackleak.h b/include/linux/stackleak.h
> index b911b97..3d5c327 100644
> --- a/include/linux/stackleak.h
> +++ b/include/linux/stackleak.h
> @@ -22,6 +22,12 @@ static inline void stackleak_task_init(struct task_struct *t)
>         t->prev_lowest_stack = t->lowest_stack;
>  # endif
>  }
> +
> +#ifdef CONFIG_STACKLEAK_RUNTIME_DISABLE
> +int stack_erasing_sysctl(struct ctl_table *table, int write,
> +                       void __user *buffer, size_t *lenp, loff_t *ppos);
> +#endif
> +
>  #else /* !CONFIG_GCC_PLUGIN_STACKLEAK */
>  static inline void stackleak_task_init(struct task_struct *t) { }
>  #endif
> diff --git a/kernel/stackleak.c b/kernel/stackleak.c
> index f5c4111..2d21372 100644
> --- a/kernel/stackleak.c
> +++ b/kernel/stackleak.c
> @@ -14,6 +14,41 @@
>
>  #include <linux/stackleak.h>
>
> +#ifdef CONFIG_STACKLEAK_RUNTIME_DISABLE
> +#include <linux/jump_label.h>
> +#include <linux/sysctl.h>
> +
> +static DEFINE_STATIC_KEY_FALSE(stack_erasing_bypass);
> +
> +int stack_erasing_sysctl(struct ctl_table *table, int write,
> +                       void __user *buffer, size_t *lenp, loff_t *ppos)
> +{
> +       int ret = 0;
> +       int state = !static_branch_unlikely(&stack_erasing_bypass);
> +       int prev_state = state;
> +
> +       table->data = &state;
> +       table->maxlen = sizeof(int);
> +       ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
> +       state = !!state;
> +       if (ret || !write || state == prev_state)
> +               return ret;
> +
> +       if (state)
> +               static_branch_disable(&stack_erasing_bypass);
> +       else
> +               static_branch_enable(&stack_erasing_bypass);
> +
> +       pr_warn("stackleak: kernel stack erasing is %s\n",
> +                                       state ? "enabled" : "disabled");

Looks good to me. I've updated the patch for -next.

> +       return ret;
> +}
> +
> +#define skip_erasing() static_branch_unlikely(&stack_erasing_bypass)
> +#else
> +#define skip_erasing() false
> +#endif /* CONFIG_STACKLEAK_RUNTIME_DISABLE */
> +
>  asmlinkage void stackleak_erase(void)
>  {
>         /* It would be nice not to have 'kstack_ptr' and 'boundary' on stack */
> @@ -22,6 +57,9 @@ asmlinkage void stackleak_erase(void)
>         unsigned int poison_count = 0;
>         const unsigned int depth = STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long);
>
> +       if (skip_erasing())
> +               return;
> +
>         /* Search for the poison value in the kernel stack */
>         while (kstack_ptr > boundary && poison_count <= depth) {
>                 if (*(unsigned long *)kstack_ptr == STACKLEAK_POISON)

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

Powered by blists - more mailing lists

Your e-mail address:

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.