linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Popov <alex.popov@linux.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Laura Abbott <labbott@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@kernel.org>
Cc: PaX Team <pageexec@freemail.hu>,
	Brad Spengler <spender@grsecurity.net>,
	Ingo Molnar <mingo@kernel.org>, Tycho Andersen <tycho@tycho.ws>,
	Mark Rutland <mark.rutland@arm.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Borislav Petkov <bp@alien8.de>,
	Richard Sandiford <richard.sandiford@arm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H . Peter Anvin" <hpa@zytor.com>,
	"Dmitry V . Levin" <ldv@altlinux.org>,
	Emese Revfy <re.emese@gmail.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
	Thomas Garnier <thgarnie@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Alexei Starovoitov <ast@kernel.org>, Josef Bacik <jbacik@fb.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Nicholas Piggin <npiggin@gmail.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	"David S . Miller" <davem@davemloft.net>,
	Ding Tianhong <dingtianhong@huawei.com>,
	David Woodhouse <dwmw@amazon.co.uk>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Dominik Brodowski <linux@dominikbrodowski.net>,
	Juergen Gross <jgross@suse.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Dan Williams <dan.j.williams@intel.com>,
	Mathias Krause <minipli@googlemail.com>,
	Vikas Shivappa <vikas.shivappa@linux.intel.com>,
	Kyle Huey <me@kylehuey.com>,
	Dmitry Safonov <dsafonov@virtuozzo.com>,
	Will Deacon <will.deacon@arm.com>, Arnd Bergmann <arnd@arndb.de>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	"kernel-hardening@lists.openwall.com"
	<kernel-hardening@lists.openwall.com>
Subject: Re: [PATCH RFC v9 2/7] x86/entry: Add STACKLEAK erasing the kernel stack at the end of syscalls
Date: Wed, 21 Mar 2018 14:04:07 +0300	[thread overview]
Message-ID: <e043e0d6-3265-4bc2-ae11-ab430832a8e2@linux.com> (raw)
In-Reply-To: <20180305202535.GX25201@hirez.programming.kicks-ass.net>

On 05.03.2018 23:25, Peter Zijlstra wrote:
> On Mon, Mar 05, 2018 at 11:43:19AM -0800, Laura Abbott wrote:
>> On 03/05/2018 08:41 AM, Dave Hansen wrote:
>>> On 03/03/2018 12:00 PM, Alexander Popov wrote:
>>>>   Documentation/x86/x86_64/mm.txt  |   2 +
>>>>   arch/Kconfig                     |  27 ++++++++++
>>>>   arch/x86/Kconfig                 |   1 +
>>>>   arch/x86/entry/entry_32.S        |  88 +++++++++++++++++++++++++++++++
>>>>   arch/x86/entry/entry_64.S        | 108 +++++++++++++++++++++++++++++++++++++++
>>>>   arch/x86/entry/entry_64_compat.S |  11 ++++
>>>
>>> This is a *lot* of assembly.  I wonder if you tried at all to get more
>>> of this into C or whether you just inherited the assembly from the
>>> original code?
>>>
>>
>> This came up previously http://www.openwall.com/lists/kernel-hardening/2017/10/23/5
>> there were concerns about trusting C to do the right thing as well as
>> speed.
> 
> And therefore the answer to this obvious question should've been part of
> the Changelog :-)
> 
> Dave is last in a long line of people asking this same question.

Hello! I've decided to share the details (and ask for advice) regardless of the
destiny of this patch series.

I've rewritten the assembly part in C, please see the code below. That is
erase_kstack() function, which is called at the end of syscall just before
returning to the userspace.

The generated asm doesn't look nice (and might be somewhat slower), but I don't
care now.

The main obstacle:
erase_kstack() must save and restore any modified registers, because it is
called from the trampoline stack (introduced by Andy Lutomirski), when all
registers except RDI are live.

Laura had a similar issue with C code on ARM:
http://www.openwall.com/lists/kernel-hardening/2017/10/10/3

I've solved that with no_caller_saved_registers attribute, which makes all
registers callee-saved. But that attribute was introduced only in gcc-7.

Does kernel have a solution for similar issues?
Thanks!

-------- >8 --------

#include <linux/bug.h>
#include <linux/sched.h>
#include <asm/current.h>
#include <asm/linkage.h>
#include <asm/processor.h>

/* This function must save and restore any modified registers */
__attribute__ ((no_caller_saved_registers)) asmlinkage void erase_kstack(void)
{
	register unsigned long p = current->thread.lowest_stack;
	register unsigned long boundary = p & ~(THREAD_SIZE - 1);
	unsigned long poison = 0;
	unsigned long check_depth = STACKLEAK_POISON_CHECK_DEPTH /
						sizeof(unsigned long);

	/*
	 * Two qwords at the bottom of the thread stack are reserved and
	 * should not be poisoned (see CONFIG_SCHED_STACK_END_CHECK).
	 */
	boundary += 2 * sizeof(unsigned long);

	/*
	 * Let's search for the poison value in the stack.
	 * Start from the lowest_stack and go to the bottom.
	 */
	while (p >= boundary && poison <= check_depth) {
		if (*(unsigned long *)p == STACKLEAK_POISON)
			poison++;
		else
			poison = 0;

		p -= sizeof(unsigned long);
	}

#ifdef CONFIG_STACKLEAK_METRICS
	current->thread.prev_lowest_stack = p;
#endif

	/*
	 * So let's write the poison value to the kernel stack. Start from
	 * the address in p and move up till the new boundary.
	 */
	if (on_thread_stack())
		boundary = current_stack_pointer;
	else
		boundary = current_top_of_stack();

	BUG_ON(boundary - p >= THREAD_SIZE);

	while (p < boundary) {
		*(unsigned long *)p = STACKLEAK_POISON;
		p += sizeof(unsigned long);
	}

	/* Reset the lowest_stack value for the next syscall */
	current->thread.lowest_stack = current_top_of_stack() - 256;
}

  parent reply	other threads:[~2018-03-21 11:04 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-03 20:00 [PATCH RFC v9 0/7] Introduce the STACKLEAK feature and a test for it Alexander Popov
2018-03-03 20:00 ` [PATCH RFC v9 1/7] gcc-plugins: Clean up the cgraph_create_edge* macros Alexander Popov
2018-03-03 20:00 ` [PATCH RFC v9 2/7] x86/entry: Add STACKLEAK erasing the kernel stack at the end of syscalls Alexander Popov
2018-03-05 16:41   ` Dave Hansen
2018-03-05 19:43     ` Laura Abbott
2018-03-05 19:50       ` Dave Hansen
2018-03-05 20:25       ` Peter Zijlstra
2018-03-05 21:21         ` Alexander Popov
2018-03-05 21:36           ` Kees Cook
2018-03-21 11:04         ` Alexander Popov [this message]
2018-03-21 15:33           ` Dave Hansen
2018-03-22 20:56             ` Alexander Popov
2018-03-26 17:32               ` Kees Cook
2018-03-26 17:43                 ` Andy Lutomirski
2018-03-03 20:00 ` [PATCH RFC v9 3/7] gcc-plugins: Add STACKLEAK plugin for tracking the kernel stack Alexander Popov
2018-03-03 20:00 ` [PATCH RFC v9 4/7] x86/entry: Erase kernel stack in syscall_trace_enter() Alexander Popov
2018-03-05 19:40   ` Dave Hansen
2018-03-05 20:06     ` Kees Cook
2018-03-05 20:15       ` Linus Torvalds
2018-03-05 21:02         ` Alexander Popov
2018-03-05 21:02         ` Kees Cook
2018-03-05 21:40           ` Linus Torvalds
2018-03-05 22:07             ` Linus Torvalds
2018-03-06  0:56             ` Kees Cook
2018-03-06  4:30               ` Linus Torvalds
2018-03-06 17:58                 ` Andy Lutomirski
2018-03-06  7:56               ` [OLD PATCH] net: recvmsg: Unconditionally zero struct sockaddr_storage " Ingo Molnar
2018-03-06  8:08           ` Ingo Molnar
2018-03-06 15:16             ` Daniel Micay
2018-03-06 15:28               ` Daniel Micay
2018-03-06 18:56               ` Linus Torvalds
2018-03-06 19:07                 ` Peter Zijlstra
2018-03-06 19:07                 ` Ard Biesheuvel
2018-03-06 19:16                   ` Linus Torvalds
2018-03-06 20:42                     ` Arnd Bergmann
2018-03-06 21:01                       ` Linus Torvalds
2018-03-06 21:21                         ` Arnd Bergmann
2018-03-06 21:29                           ` Linus Torvalds
2018-03-06 22:09                             ` Arnd Bergmann
2018-03-06 22:24                               ` Linus Torvalds
2018-03-06 21:36                         ` Steven Rostedt
2018-03-06 21:41                           ` Linus Torvalds
2018-03-06 21:47                             ` Linus Torvalds
2018-03-06 22:29                               ` Steven Rostedt
2018-03-06 22:41                                 ` Linus Torvalds
2018-03-06 22:52                                   ` Steven Rostedt
2018-03-06 23:09                                     ` Linus Torvalds
2018-03-12  8:22                               ` Ingo Molnar
2018-03-12  9:00                                 ` Ard Biesheuvel
2018-03-12  9:21                                   ` Ingo Molnar
2018-03-06 21:47                           ` Arnd Bergmann
2018-03-06 22:19                             ` Linus Torvalds
2018-03-05 20:26       ` Peter Zijlstra
2018-03-03 20:00 ` [PATCH RFC v9 5/7] lkdtm: Add a test for STACKLEAK Alexander Popov
2018-03-03 20:00 ` [PATCH RFC v9 6/7] fs/proc: Show STACKLEAK metrics in the /proc file system Alexander Popov
2018-03-03 20:00 ` [PATCH RFC v9 7/7] doc: self-protection: Add information about STACKLEAK feature Alexander Popov
2018-03-05 19:34 ` [PATCH RFC v9 0/7] Introduce the STACKLEAK feature and a test for it Kees Cook
2018-03-05 19:42   ` Dave Hansen
2018-03-05 20:02     ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e043e0d6-3265-4bc2-ae11-ab430832a8e2@linux.com \
    --to=alex.popov@linux.com \
    --cc=akpm@linux-foundation.org \
    --cc=ard.biesheuvel@linaro.org \
    --cc=arnd@arndb.de \
    --cc=aryabinin@virtuozzo.com \
    --cc=ast@kernel.org \
    --cc=bp@alien8.de \
    --cc=corbet@lwn.net \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=davem@davemloft.net \
    --cc=dingtianhong@huawei.com \
    --cc=dsafonov@virtuozzo.com \
    --cc=dwmw@amazon.co.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=jbacik@fb.com \
    --cc=jgross@suse.com \
    --cc=jpoimboe@redhat.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=labbott@redhat.com \
    --cc=ldv@altlinux.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@dominikbrodowski.net \
    --cc=luto@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=me@kylehuey.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=minipli@googlemail.com \
    --cc=npiggin@gmail.com \
    --cc=pageexec@freemail.hu \
    --cc=peterz@infradead.org \
    --cc=re.emese@gmail.com \
    --cc=richard.sandiford@arm.com \
    --cc=rostedt@goodmis.org \
    --cc=spender@grsecurity.net \
    --cc=tglx@linutronix.de \
    --cc=thgarnie@google.com \
    --cc=torvalds@linux-foundation.org \
    --cc=tycho@tycho.ws \
    --cc=vikas.shivappa@linux.intel.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=will.deacon@arm.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).