[<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
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
