diff mbox

[v3,09/27] x86/acpi: Adapt assembly for PIE support

Message ID 20180523195421.180248-10-thgarnie@google.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thomas Garnier May 23, 2018, 7:54 p.m. UTC
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@google.com>
---
 arch/x86/kernel/acpi/wakeup_64.S | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

Comments

Pavel Machek May 24, 2018, 11:03 a.m. UTC | #1
On Wed 2018-05-23 12:54:03, 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.

What testing did this get?

> diff --git a/arch/x86/kernel/acpi/wakeup_64.S b/arch/x86/kernel/acpi/wakeup_64.S
> index 50b8ed0317a3..472659c0f811 100644
> --- a/arch/x86/kernel/acpi/wakeup_64.S
> +++ b/arch/x86/kernel/acpi/wakeup_64.S
> @@ -14,7 +14,7 @@
>  	 * Hooray, we are in Long 64-bit mode (but still running in low memory)
>  	 */
>  ENTRY(wakeup_long64)
> -	movq	saved_magic, %rax
> +	movq	saved_magic(%rip), %rax
>  	movq	$0x123456789abcdef0, %rdx
>  	cmpq	%rdx, %rax
>  	jne	bogus_64_magic

Because, as comment says, this is rather tricky code.
									Pavel
Thomas Garnier May 24, 2018, 4:35 p.m. UTC | #2
On Thu, May 24, 2018 at 4:03 AM Pavel Machek <pavel@ucw.cz> wrote:

> On Wed 2018-05-23 12:54:03, 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.

> What testing did this get?

Tested boot, hibernation and performance on qemu and dedicated machine.


> > diff --git a/arch/x86/kernel/acpi/wakeup_64.S
b/arch/x86/kernel/acpi/wakeup_64.S
> > index 50b8ed0317a3..472659c0f811 100644
> > --- a/arch/x86/kernel/acpi/wakeup_64.S
> > +++ b/arch/x86/kernel/acpi/wakeup_64.S
> > @@ -14,7 +14,7 @@
> >        * Hooray, we are in Long 64-bit mode (but still running in low
memory)
> >        */
> >  ENTRY(wakeup_long64)
> > -     movq    saved_magic, %rax
> > +     movq    saved_magic(%rip), %rax
> >       movq    $0x123456789abcdef0, %rdx
> >       cmpq    %rdx, %rax
> >       jne     bogus_64_magic

> Because, as comment says, this is rather tricky code.

I agree, I think maintainers feedback is very important for this patchset.


Pavel

> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Pavel Machek May 25, 2018, 9:14 a.m. UTC | #3
On Thu 2018-05-24 09:35:42, Thomas Garnier wrote:
> On Thu, May 24, 2018 at 4:03 AM Pavel Machek <pavel@ucw.cz> wrote:
> 
> > On Wed 2018-05-23 12:54:03, 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.
> 
> > What testing did this get?
> 
> Tested boot, hibernation and performance on qemu and dedicated machine.

Well, this is suspend, not hibernation code.

So "sudo pm-suspend" or "echo mem > /sys/power/state" would be good
way to test this.

Thanks,
							Pavel

> > > diff --git a/arch/x86/kernel/acpi/wakeup_64.S
> b/arch/x86/kernel/acpi/wakeup_64.S
> > > index 50b8ed0317a3..472659c0f811 100644
> > > --- a/arch/x86/kernel/acpi/wakeup_64.S
> > > +++ b/arch/x86/kernel/acpi/wakeup_64.S
> > > @@ -14,7 +14,7 @@
> > >        * Hooray, we are in Long 64-bit mode (but still running in low
> memory)
> > >        */
> > >  ENTRY(wakeup_long64)
> > > -     movq    saved_magic, %rax
> > > +     movq    saved_magic(%rip), %rax
> > >       movq    $0x123456789abcdef0, %rdx
> > >       cmpq    %rdx, %rax
> > >       jne     bogus_64_magic
> 
> > Because, as comment says, this is rather tricky code.
> 
> I agree, I think maintainers feedback is very important for this patchset.
Thomas Garnier May 25, 2018, 5 p.m. UTC | #4
On Fri, May 25, 2018 at 2:14 AM Pavel Machek <pavel@ucw.cz> wrote:

> On Thu 2018-05-24 09:35:42, Thomas Garnier wrote:
> > On Thu, May 24, 2018 at 4:03 AM Pavel Machek <pavel@ucw.cz> wrote:
> >
> > > On Wed 2018-05-23 12:54:03, 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.
> >
> > > What testing did this get?
> >
> > Tested boot, hibernation and performance on qemu and dedicated machine.

> Well, this is suspend, not hibernation code.

> So "sudo pm-suspend" or "echo mem > /sys/power/state" would be good
> way to test this.

Thanks, it worked. I added this to the testsuite I use for KASLR.


> Thanks,
>                                                          Pavel

> > > > diff --git a/arch/x86/kernel/acpi/wakeup_64.S
> > b/arch/x86/kernel/acpi/wakeup_64.S
> > > > index 50b8ed0317a3..472659c0f811 100644
> > > > --- a/arch/x86/kernel/acpi/wakeup_64.S
> > > > +++ b/arch/x86/kernel/acpi/wakeup_64.S
> > > > @@ -14,7 +14,7 @@
> > > >        * Hooray, we are in Long 64-bit mode (but still running in
low
> > memory)
> > > >        */
> > > >  ENTRY(wakeup_long64)
> > > > -     movq    saved_magic, %rax
> > > > +     movq    saved_magic(%rip), %rax
> > > >       movq    $0x123456789abcdef0, %rdx
> > > >       cmpq    %rdx, %rax
> > > >       jne     bogus_64_magic
> >
> > > Because, as comment says, this is rather tricky code.
> >
> > I agree, I think maintainers feedback is very important for this
patchset.


> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Pavel Machek May 29, 2018, 12:31 p.m. UTC | #5
On Fri 2018-05-25 10:00:04, Thomas Garnier wrote:
> On Fri, May 25, 2018 at 2:14 AM Pavel Machek <pavel@ucw.cz> wrote:
> 
> > On Thu 2018-05-24 09:35:42, Thomas Garnier wrote:
> > > On Thu, May 24, 2018 at 4:03 AM Pavel Machek <pavel@ucw.cz> wrote:
> > >
> > > > On Wed 2018-05-23 12:54:03, 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.
> > >
> > > > What testing did this get?
> > >
> > > Tested boot, hibernation and performance on qemu and dedicated machine.
> 
> > Well, this is suspend, not hibernation code.
> 
> > So "sudo pm-suspend" or "echo mem > /sys/power/state" would be good
> > way to test this.
> 
> Thanks, it worked. I added this to the testsuite I use for KASLR.

Thanks!

You can add my Acked-by:.

								Pavel
Thomas Garnier May 29, 2018, 3:55 p.m. UTC | #6
On Tue, May 29, 2018 at 5:31 AM Pavel Machek <pavel@ucw.cz> wrote:

> On Fri 2018-05-25 10:00:04, Thomas Garnier wrote:
> > On Fri, May 25, 2018 at 2:14 AM Pavel Machek <pavel@ucw.cz> wrote:
> >
> > > On Thu 2018-05-24 09:35:42, Thomas Garnier wrote:
> > > > On Thu, May 24, 2018 at 4:03 AM Pavel Machek <pavel@ucw.cz> wrote:
> > > >
> > > > > On Wed 2018-05-23 12:54:03, 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.
> > > >
> > > > > What testing did this get?
> > > >
> > > > Tested boot, hibernation and performance on qemu and dedicated
machine.
> >
> > > Well, this is suspend, not hibernation code.
> >
> > > So "sudo pm-suspend" or "echo mem > /sys/power/state" would be good
> > > way to test this.
> >
> > Thanks, it worked. I added this to the testsuite I use for KASLR.

> Thanks!

> You can add my Acked-by:.

Will do. Thanks for the review.


>                                                                  Pavel

> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
diff mbox

Patch

diff --git a/arch/x86/kernel/acpi/wakeup_64.S b/arch/x86/kernel/acpi/wakeup_64.S
index 50b8ed0317a3..472659c0f811 100644
--- a/arch/x86/kernel/acpi/wakeup_64.S
+++ b/arch/x86/kernel/acpi/wakeup_64.S
@@ -14,7 +14,7 @@ 
 	 * Hooray, we are in Long 64-bit mode (but still running in low memory)
 	 */
 ENTRY(wakeup_long64)
-	movq	saved_magic, %rax
+	movq	saved_magic(%rip), %rax
 	movq	$0x123456789abcdef0, %rdx
 	cmpq	%rdx, %rax
 	jne	bogus_64_magic
@@ -25,14 +25,14 @@  ENTRY(wakeup_long64)
 	movw	%ax, %es
 	movw	%ax, %fs
 	movw	%ax, %gs
-	movq	saved_rsp, %rsp
+	movq	saved_rsp(%rip), %rsp
 
-	movq	saved_rbx, %rbx
-	movq	saved_rdi, %rdi
-	movq	saved_rsi, %rsi
-	movq	saved_rbp, %rbp
+	movq	saved_rbx(%rip), %rbx
+	movq	saved_rdi(%rip), %rdi
+	movq	saved_rsi(%rip), %rsi
+	movq	saved_rbp(%rip), %rbp
 
-	movq	saved_rip, %rax
+	movq	saved_rip(%rip), %rax
 	jmp	*%rax
 ENDPROC(wakeup_long64)
 
@@ -45,7 +45,7 @@  ENTRY(do_suspend_lowlevel)
 	xorl	%eax, %eax
 	call	save_processor_state
 
-	movq	$saved_context, %rax
+	leaq	saved_context(%rip), %rax
 	movq	%rsp, pt_regs_sp(%rax)
 	movq	%rbp, pt_regs_bp(%rax)
 	movq	%rsi, pt_regs_si(%rax)
@@ -64,13 +64,14 @@  ENTRY(do_suspend_lowlevel)
 	pushfq
 	popq	pt_regs_flags(%rax)
 
-	movq	$.Lresume_point, saved_rip(%rip)
+	leaq	.Lresume_point(%rip), %rax
+	movq	%rax, saved_rip(%rip)
 
-	movq	%rsp, saved_rsp
-	movq	%rbp, saved_rbp
-	movq	%rbx, saved_rbx
-	movq	%rdi, saved_rdi
-	movq	%rsi, saved_rsi
+	movq	%rsp, saved_rsp(%rip)
+	movq	%rbp, saved_rbp(%rip)
+	movq	%rbx, saved_rbx(%rip)
+	movq	%rdi, saved_rdi(%rip)
+	movq	%rsi, saved_rsi(%rip)
 
 	addq	$8, %rsp
 	movl	$3, %edi
@@ -82,7 +83,7 @@  ENTRY(do_suspend_lowlevel)
 	.align 4
 .Lresume_point:
 	/* We don't restore %rax, it must be 0 anyway */
-	movq	$saved_context, %rax
+	leaq	saved_context(%rip), %rax
 	movq	saved_context_cr4(%rax), %rbx
 	movq	%rbx, %cr4
 	movq	saved_context_cr3(%rax), %rbx