2019-07-08 22:46:20

by Thomas Garnier

[permalink] [raw]
Subject: [PATCH v8 06/11] x86/CPU: Adapt assembly for PIE support

Change the assembly code to use only relative references of symbols for the
kernel to be PIE compatible. Use the new _ASM_MOVABS macro instead of
the 'mov $symbol, %dst' construct.

Position Independent Executable (PIE) support will allow to extend the
KASLR randomization range below 0xffffffff80000000.

Signed-off-by: Thomas Garnier <[email protected]>
---
arch/x86/include/asm/processor.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 3eab6ece52b4..3e2154b0e09f 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -713,11 +713,13 @@ static inline void sync_core(void)
"pushfq\n\t"
"mov %%cs, %0\n\t"
"pushq %q0\n\t"
- "pushq $1f\n\t"
+ "movabsq $1f, %q0\n\t"
+ "pushq %q0\n\t"
"iretq\n\t"
UNWIND_HINT_RESTORE
"1:"
- : "=&r" (tmp), ASM_CALL_CONSTRAINT : : "cc", "memory");
+ : "=&r" (tmp), ASM_CALL_CONSTRAINT
+ : : "cc", "memory");
#endif
}

--
2.22.0.410.gd8fdbe21b5-goog


2019-07-08 22:48:06

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH v8 06/11] x86/CPU: Adapt assembly for PIE support

Thomas Garnier wrote:
> - "pushq $1f\n\t"
> + "movabsq $1f, %q0\n\t"
> + "pushq %q0\n\t"
> "iretq\n\t"
> UNWIND_HINT_RESTORE
> "1:"

Fake PIE. True PIE looks like this:

ffffffff81022d70 <do_sync_core>:
ffffffff81022d70: 8c d0 mov eax,ss
ffffffff81022d72: 50 push rax
ffffffff81022d73: 54 push rsp
ffffffff81022d74: 48 83 04 24 08 add QWORD PTR [rsp],0x8
ffffffff81022d79: 9c pushf
ffffffff81022d7a: 8c c8 mov eax,cs
ffffffff81022d7c: 50 push rax
ffffffff81022d7d: ===> 48 8d 05 03 00 00 00 lea rax,[rip+0x3] # ffffffff81022d87 <do_sync_core+0x17>
ffffffff81022d84: 50 push rax
ffffffff81022d85: 48 cf iretq
ffffffff81022d87: c3 ret

Signed-off-by: Alexey Dobriyan <[email protected]>

--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -710,7 +710,8 @@ static inline void sync_core(void)
"pushfq\n\t"
"mov %%cs, %0\n\t"
"pushq %q0\n\t"
- "pushq $1f\n\t"
+ "leaq 1f(%%rip), %q0\n\t"
+ "pushq %q0\n\t"
"iretq\n\t"
UNWIND_HINT_RESTORE
"1:"

2019-07-08 22:48:23

by Thomas Garnier

[permalink] [raw]
Subject: Re: [PATCH v8 06/11] x86/CPU: Adapt assembly for PIE support

On Mon, Jul 8, 2019 at 12:09 PM Alexey Dobriyan <[email protected]> wrote:
>
> Thomas Garnier wrote:
> > - "pushq $1f\n\t"
> > + "movabsq $1f, %q0\n\t"
> > + "pushq %q0\n\t"
> > "iretq\n\t"
> > UNWIND_HINT_RESTORE
> > "1:"
>
> Fake PIE. True PIE looks like this:

I used movabsq in couple assembly changes where the memory context is
unclear and relative reference might lead to issues. It happened on
early boot and hibernation save/restore paths. Do you think a relative
reference in this function will always be accurate?

>
> ffffffff81022d70 <do_sync_core>:
> ffffffff81022d70: 8c d0 mov eax,ss
> ffffffff81022d72: 50 push rax
> ffffffff81022d73: 54 push rsp
> ffffffff81022d74: 48 83 04 24 08 add QWORD PTR [rsp],0x8
> ffffffff81022d79: 9c pushf
> ffffffff81022d7a: 8c c8 mov eax,cs
> ffffffff81022d7c: 50 push rax
> ffffffff81022d7d: ===> 48 8d 05 03 00 00 00 lea rax,[rip+0x3] # ffffffff81022d87 <do_sync_core+0x17>
> ffffffff81022d84: 50 push rax
> ffffffff81022d85: 48 cf iretq
> ffffffff81022d87: c3 ret
>
> Signed-off-by: Alexey Dobriyan <[email protected]>
>
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -710,7 +710,8 @@ static inline void sync_core(void)
> "pushfq\n\t"
> "mov %%cs, %0\n\t"
> "pushq %q0\n\t"
> - "pushq $1f\n\t"
> + "leaq 1f(%%rip), %q0\n\t"
> + "pushq %q0\n\t"
> "iretq\n\t"
> UNWIND_HINT_RESTORE
> "1:"

2019-07-09 18:41:00

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH v8 06/11] x86/CPU: Adapt assembly for PIE support

On Mon, Jul 08, 2019 at 12:35:13PM -0700, Thomas Garnier wrote:
> On Mon, Jul 8, 2019 at 12:09 PM Alexey Dobriyan <[email protected]> wrote:
> >
> > Thomas Garnier wrote:
> > > - "pushq $1f\n\t"
> > > + "movabsq $1f, %q0\n\t"
> > > + "pushq %q0\n\t"
> > > "iretq\n\t"
> > > UNWIND_HINT_RESTORE
> > > "1:"
> >
> > Fake PIE. True PIE looks like this:
>
> I used movabsq in couple assembly changes where the memory context is
> unclear and relative reference might lead to issues. It happened on
> early boot and hibernation save/restore paths. Do you think a relative
> reference in this function will always be accurate?

As long as iretq target is not too far it should be OK.

I'm not really sure which issues can pop up.

IRETQ is 64-bit only, RIP-relative addressing is 64-bit only.
Assembler (hopefully) will error compilation if target is too far.

And it is shorter than movabsq.

> > ffffffff81022d70 <do_sync_core>:
> > ffffffff81022d70: 8c d0 mov eax,ss
> > ffffffff81022d72: 50 push rax
> > ffffffff81022d73: 54 push rsp
> > ffffffff81022d74: 48 83 04 24 08 add QWORD PTR [rsp],0x8
> > ffffffff81022d79: 9c pushf
> > ffffffff81022d7a: 8c c8 mov eax,cs
> > ffffffff81022d7c: 50 push rax
> > ffffffff81022d7d: ===> 48 8d 05 03 00 00 00 lea rax,[rip+0x3] # ffffffff81022d87 <do_sync_core+0x17>
> > ffffffff81022d84: 50 push rax
> > ffffffff81022d85: 48 cf iretq
> > ffffffff81022d87: c3 ret
> >
> > Signed-off-by: Alexey Dobriyan <[email protected]>
> >
> > --- a/arch/x86/include/asm/processor.h
> > +++ b/arch/x86/include/asm/processor.h
> > @@ -710,7 +710,8 @@ static inline void sync_core(void)
> > "pushfq\n\t"
> > "mov %%cs, %0\n\t"
> > "pushq %q0\n\t"
> > - "pushq $1f\n\t"
> > + "leaq 1f(%%rip), %q0\n\t"
> > + "pushq %q0\n\t"
> > "iretq\n\t"
> > UNWIND_HINT_RESTORE
> > "1:"

2019-07-09 19:09:34

by Thomas Garnier

[permalink] [raw]
Subject: Re: [PATCH v8 06/11] x86/CPU: Adapt assembly for PIE support

On Tue, Jul 9, 2019 at 11:39 AM Alexey Dobriyan <[email protected]> wrote:
>
> On Mon, Jul 08, 2019 at 12:35:13PM -0700, Thomas Garnier wrote:
> > On Mon, Jul 8, 2019 at 12:09 PM Alexey Dobriyan <[email protected]> wrote:
> > >
> > > Thomas Garnier wrote:
> > > > - "pushq $1f\n\t"
> > > > + "movabsq $1f, %q0\n\t"
> > > > + "pushq %q0\n\t"
> > > > "iretq\n\t"
> > > > UNWIND_HINT_RESTORE
> > > > "1:"
> > >
> > > Fake PIE. True PIE looks like this:
> >
> > I used movabsq in couple assembly changes where the memory context is
> > unclear and relative reference might lead to issues. It happened on
> > early boot and hibernation save/restore paths. Do you think a relative
> > reference in this function will always be accurate?
>
> As long as iretq target is not too far it should be OK.
>
> I'm not really sure which issues can pop up.
>
> IRETQ is 64-bit only, RIP-relative addressing is 64-bit only.
> Assembler (hopefully) will error compilation if target is too far.
>
> And it is shorter than movabsq.

Agree, I will change it and run some tests for the next iteration.

>
> > > ffffffff81022d70 <do_sync_core>:
> > > ffffffff81022d70: 8c d0 mov eax,ss
> > > ffffffff81022d72: 50 push rax
> > > ffffffff81022d73: 54 push rsp
> > > ffffffff81022d74: 48 83 04 24 08 add QWORD PTR [rsp],0x8
> > > ffffffff81022d79: 9c pushf
> > > ffffffff81022d7a: 8c c8 mov eax,cs
> > > ffffffff81022d7c: 50 push rax
> > > ffffffff81022d7d: ===> 48 8d 05 03 00 00 00 lea rax,[rip+0x3] # ffffffff81022d87 <do_sync_core+0x17>
> > > ffffffff81022d84: 50 push rax
> > > ffffffff81022d85: 48 cf iretq
> > > ffffffff81022d87: c3 ret
> > >
> > > Signed-off-by: Alexey Dobriyan <[email protected]>
> > >
> > > --- a/arch/x86/include/asm/processor.h
> > > +++ b/arch/x86/include/asm/processor.h
> > > @@ -710,7 +710,8 @@ static inline void sync_core(void)
> > > "pushfq\n\t"
> > > "mov %%cs, %0\n\t"
> > > "pushq %q0\n\t"
> > > - "pushq $1f\n\t"
> > > + "leaq 1f(%%rip), %q0\n\t"
> > > + "pushq %q0\n\t"
> > > "iretq\n\t"
> > > UNWIND_HINT_RESTORE
> > > "1:"