linux-kernel - Re: [RFC/INCOMPLETE 00/13] x86: Rewrite exit-to-userspace code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 17 Jun 2015 11:48:57 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Andy Lutomirski <luto@...nel.org>
Cc:	x86@...nel.org, linux-kernel@...r.kernel.org,
	Frédéric Weisbecker <fweisbec@...il.com>,
	Rik van Riel <riel@...hat.com>,
	Oleg Nesterov <oleg@...hat.com>,
	Denys Vlasenko <vda.linux@...glemail.com>,
	Borislav Petkov <bp@...en8.de>,
	Kees Cook <keescook@...omium.org>,
	Brian Gerst <brgerst@...il.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Denys Vlasenko <dvlasenk@...hat.com>
Subject: Re: [RFC/INCOMPLETE 00/13] x86: Rewrite exit-to-userspace code


* Andy Lutomirski <luto@...nel.org> wrote:

> This is incomplete, but it's finally good enough that I think it's
> time to get other opinions on it.  It is a complete rewrite of the
> slow path code that handles exits to user mode.

Modulo the small comments I made about the debug checks interface plus naming 
details the structure and intention of this series gives me warm fuzzy feelings.

> The exit-to-usermode code is copied in several places and is written in a nasty 
> combination of asm and C.  It's not at all clear what it's supposed to do, and 
> the way it's structured makes it very hard to work with.  For example, it's not 
> even clear why syscall exit hooks are called only once per syscall right now.  
> (It seems to be a side effect of the way that rdi and rdx are handled in the asm 
> loop, and it seems reliable, but it's still pointlessly complicated.)  The 
> existing code also makes context tracking overly complicated and hard to 
> understand.  Finally, it's nearly impossible for anyone to change what happens 
> on exit to usermode, since the existing code is so fragile.

Amen.

> I tried to clean it up incrementally, but I decided it was too hard. Instead, 
> this series just replaces the code.  It seems to work.

Any known bugs beyond UML build breakage?

> Context tracking in particular works very differently now.  The low-level entry 
> code checks that we're in CONTEXT_USER and switches to CONTEXT_KERNEL.  The exit 
> code does the reverse.  There is no need to track what CONTEXT_XYZ state we came 
> from, because we already know.  Similarly, SCHEDULE_USER is gone, since we can 
> reschedule if needed by simply calling schedule() from C code.
> 
> The main things that are missing are that I haven't done the 32-bit parts 
> (anyone want to help?) and therefore I haven't deleted the old C code.  I also 
> think this may break UML for trivial reasons.
> 
> Because I haven't converted the 32-bit code yet, all of the now-unnecessary 
> unnecessary calls to exception_enter are still present in traps.c.
> 
> IRQ context tracking is still duplicated.  We should probably clean it up by 
> changing the core code to supply something like 
> irq_enter_we_are_already_in_context_kernel.
> 
> Thoughts?

So assuming you fix the UML build I'm inclined to go for it, even in this 
incomplete form, to increase testing coverage.

Doing that will also decrease ongoing merge friction between your work and other 
entry code cleanups ...

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/