[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 19 Jul 2017 16:23:23 -0700
From: Thomas Garnier <thgarnie@...gle.com>
To: "H. Peter Anvin" <hpa@...or.com>
Cc: 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>,
"the arch/x86 maintainers" <x86@...nel.org>, linux-crypto@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>, xen-devel@...ts.xenproject.org,
kvm list <kvm@...r.kernel.org>, Linux PM list <linux-pm@...r.kernel.org>,
linux-arch <linux-arch@...r.kernel.org>, linux-sparse@...r.kernel.org,
Kernel Hardening <kernel-hardening@...ts.openwall.com>
Subject: Re: [RFC 07/22] x86: relocate_kernel - Adapt assembly for PIE support
On Wed, Jul 19, 2017 at 3:58 PM, H. Peter Anvin <hpa@...or.com> wrote:
> 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
>
Thanks I will look into that.
> 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.
There is a lot of assembly that needed to be change. It was easier to
understand parts that are directly exercised like boot or percpu.
That's why I value people's feedback and will improve the patchset.
Thanks!
>
> Sorry.
>
> -hpa
--
Thomas
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.
