[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC 07/22] x86: relocate_kernel - Adapt assembly for PIE support
On Wed, Jul 19, 2017 at 3:58 PM, H. Peter Anvin <hpa@xxxxxxxxx> 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@xxxxxxxxxx> >> --- >> 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 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |