Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 19 Jul 2017 15:58:41 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Thomas Garnier <thgarnie@...gle.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S . Miller" <davem@...emloft.net>,
        Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>, Arnd Bergmann <arnd@...db.de>,
        Matthias Kaehlcke <mka@...omium.org>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross
 <jgross@...e.com>, Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Joerg Roedel <joro@...tes.org>, Andy Lutomirski <luto@...nel.org>,
        Borislav Petkov <bp@...en8.de>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Brian Gerst <brgerst@...il.com>, Borislav Petkov <bp@...e.de>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Len Brown <len.brown@...el.com>, Pavel Machek <pavel@....cz>,
        Tejun Heo <tj@...nel.org>, Christoph Lameter <cl@...ux.com>,
        Kees Cook <keescook@...omium.org>,
        Paul Gortmaker <paul.gortmaker@...driver.com>,
        Chris Metcalf <cmetcalf@...lanox.com>,
        "Paul E . McKenney" <paulmck@...ux.vnet.ibm.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Christopher Li <sparse@...isli.org>,
        Dou Liyang <douly.fnst@...fujitsu.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Markus Trippelsdorf <markus@...ppelsdorf.de>,
        Peter Foley <pefoley2@...oley.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Matthew Wilcox <mawilcox@...rosoft.com>,
        Michal Hocko <mhocko@...e.com>, Rob Landley <rob@...dley.net>,
        Jiri Kosina <jkosina@...e.cz>, "H . J . Lu" <hjl.tools@...il.com>,
        Paul Bolle <pebolle@...cali.nl>, Baoquan He <bhe@...hat.com>,
        Daniel Micay <danielmicay@...il.com>
Cc: x86@...nel.org, linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
        xen-devel@...ts.xenproject.org, kvm@...r.kernel.org,
        linux-pm@...r.kernel.org, linux-arch@...r.kernel.org,
        linux-sparse@...r.kernel.org, kernel-hardening@...ts.openwall.com
Subject: Re: [RFC 07/22] x86: relocate_kernel - Adapt assembly for PIE support

On 07/18/17 15:33, Thomas Garnier wrote:
> Change the assembly code to use only relative references of symbols for the
> kernel to be PIE compatible.
> 
> Position Independent Executable (PIE) support will allow to extended the
> KASLR randomization range below the -2G memory limit.
> 
> Signed-off-by: Thomas Garnier <thgarnie@...gle.com>
> ---
>  arch/x86/kernel/relocate_kernel_64.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
> index 98111b38ebfd..da817d1628ac 100644
> --- a/arch/x86/kernel/relocate_kernel_64.S
> +++ b/arch/x86/kernel/relocate_kernel_64.S
> @@ -186,7 +186,7 @@ identity_mapped:
>  	movq	%rax, %cr3
>  	lea	PAGE_SIZE(%r8), %rsp
>  	call	swap_pages
> -	movq	$virtual_mapped, %rax
> +	leaq	virtual_mapped(%rip), %rax
>  	pushq	%rax
>  	ret
>  

This is completely wrong.  The whole point is that %rip here is on an
identity-mapped page, which means that its offset to the actual symbol
is ill-defined.

The use of pushq/ret to do an indirect jump is bizarre, though, instead of:

	pushq %r8
	ret

one ought to simply do

	jmpq *%r8

I think the author of this code was confused by the fact that we have to
use this construct to do a *far* jump.

There are some other very bizarre constructs in this file, that I can
only assume comes from clumsy porting from 32 bits, for example:

	call 1f
1:
	popq %r8
	subq $(1b - relocate_kernel), %r8

... instead of the much simpler ...

	leaq relocate_kernel(%rip), %r8

With this value in %r8 anyway, you can simply do:

	leaq (virtual_mapped - relocate_kernel)(%r8), %rax
	jmpq *%rax

This patchset scares me.  There seems to be a lot of places where you
have not been very aware of what is actually happening in the code, nor
have done research about how the ABIs actually work and affect things.

Sorry.

	-hpa

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.