* [PATCH v5 0/2] PVH GDT fixes
@ 2018-05-23 14:30 Boris Ostrovsky
2018-05-23 14:30 ` [PATCH v5 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
2018-05-23 14:30 ` [PATCH v5 2/2] xen/PVH: Make GDT selectors PVH-specific Boris Ostrovsky
0 siblings, 2 replies; 5+ messages in thread
From: Boris Ostrovsky @ 2018-05-23 14:30 UTC (permalink / raw)
To: linux-kernel, xen-devel; +Cc: jgross, JBeulich, brgerst, Boris Ostrovsky
Fix stack canary handling (in the first patch) and re-index PVH GDT to
make it explicit that the GDT PVH-specific
v5:
- Load canary's physical address and clear %edx for 64-bit mode
Boris Ostrovsky (2):
xen/PVH: Set up GS segment for stack canary
xen/PVH: Make GDT selectors PVH-specific
arch/x86/xen/xen-pvh.S | 47 +++++++++++++++++++++++++++++++++++++----------
1 file changed, 37 insertions(+), 10 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v5 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-23 14:30 [PATCH v5 0/2] PVH GDT fixes Boris Ostrovsky
@ 2018-05-23 14:30 ` Boris Ostrovsky
2018-05-23 15:41 ` Jan Beulich
2018-05-23 14:30 ` [PATCH v5 2/2] xen/PVH: Make GDT selectors PVH-specific Boris Ostrovsky
1 sibling, 1 reply; 5+ messages in thread
From: Boris Ostrovsky @ 2018-05-23 14:30 UTC (permalink / raw)
To: linux-kernel, xen-devel; +Cc: jgross, JBeulich, brgerst, Boris Ostrovsky
We are making calls to C code (e.g. xen_prepare_pvh()) which may use
stack canary (stored in GS segment).
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
---
arch/x86/xen/xen-pvh.S | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S
index e1a5fbe..d6a17b9 100644
--- a/arch/x86/xen/xen-pvh.S
+++ b/arch/x86/xen/xen-pvh.S
@@ -54,6 +54,9 @@
* charge of setting up it's own stack, GDT and IDT.
*/
+#define PVH_GDT_ENTRY_CANARY 4
+#define PVH_CANARY_SEL (PVH_GDT_ENTRY_CANARY * 8)
+
ENTRY(pvh_start_xen)
cld
@@ -98,6 +101,12 @@ ENTRY(pvh_start_xen)
/* 64-bit entry point. */
.code64
1:
+ /* Set base address in stack canary descriptor. */
+ mov $MSR_GS_BASE,%ecx
+ mov $_pa(canary), %rax
+ xor %rdx, %rdx
+ wrmsr
+
call xen_prepare_pvh
/* startup_64 expects boot_params in %rsi. */
@@ -107,6 +116,17 @@ ENTRY(pvh_start_xen)
#else /* CONFIG_X86_64 */
+ /* Set base address in stack canary descriptor. */
+ movl $_pa(gdt_start),%eax
+ movl $_pa(canary),%ecx
+ movw %cx, (PVH_GDT_ENTRY_CANARY * 8) + 2(%eax)
+ shrl $16, %ecx
+ movb %cl, (PVH_GDT_ENTRY_CANARY * 8) + 4(%eax)
+ movb %ch, (PVH_GDT_ENTRY_CANARY * 8) + 7(%eax)
+
+ mov $PVH_CANARY_SEL,%eax
+ mov %eax,%gs
+
call mk_early_pgtbl_32
mov $_pa(initial_page_table), %eax
@@ -150,9 +170,13 @@ gdt_start:
.quad GDT_ENTRY(0xc09a, 0, 0xfffff) /* __KERNEL_CS */
#endif
.quad GDT_ENTRY(0xc092, 0, 0xfffff) /* __KERNEL_DS */
+ .quad GDT_ENTRY(0x4090, 0, 0x18) /* PVH_CANARY_SEL */
gdt_end:
- .balign 4
+ .balign 16
+canary:
+ .fill 48, 1, 0
+
early_stack:
.fill 256, 1, 0
early_stack_end:
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5 2/2] xen/PVH: Make GDT selectors PVH-specific
2018-05-23 14:30 [PATCH v5 0/2] PVH GDT fixes Boris Ostrovsky
2018-05-23 14:30 ` [PATCH v5 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
@ 2018-05-23 14:30 ` Boris Ostrovsky
1 sibling, 0 replies; 5+ messages in thread
From: Boris Ostrovsky @ 2018-05-23 14:30 UTC (permalink / raw)
To: linux-kernel, xen-devel; +Cc: jgross, JBeulich, brgerst, Boris Ostrovsky
We don't need to share PVH GDT layout with other GDTs, especially
since we now have a PVH-speciific entry (for stack canary segment).
Define PVH's own selectors.
(As a side effect of this change we are also fixing improper
reference to __KERNEL_CS)
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
---
arch/x86/xen/xen-pvh.S | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S
index d6a17b9..dd852ac 100644
--- a/arch/x86/xen/xen-pvh.S
+++ b/arch/x86/xen/xen-pvh.S
@@ -54,7 +54,11 @@
* charge of setting up it's own stack, GDT and IDT.
*/
-#define PVH_GDT_ENTRY_CANARY 4
+#define PVH_GDT_ENTRY_CS 1
+#define PVH_GDT_ENTRY_DS 2
+#define PVH_GDT_ENTRY_CANARY 3
+#define PVH_CS_SEL (PVH_GDT_ENTRY_CS * 8)
+#define PVH_DS_SEL (PVH_GDT_ENTRY_DS * 8)
#define PVH_CANARY_SEL (PVH_GDT_ENTRY_CANARY * 8)
ENTRY(pvh_start_xen)
@@ -62,7 +66,7 @@ ENTRY(pvh_start_xen)
lgdt (_pa(gdt))
- mov $(__BOOT_DS),%eax
+ mov $PVH_DS_SEL,%eax
mov %eax,%ds
mov %eax,%es
mov %eax,%ss
@@ -96,7 +100,7 @@ ENTRY(pvh_start_xen)
mov %eax, %cr0
/* Jump to 64-bit mode. */
- ljmp $__KERNEL_CS, $_pa(1f)
+ ljmp $PVH_CS_SEL, $_pa(1f)
/* 64-bit entry point. */
.code64
@@ -136,13 +140,13 @@ ENTRY(pvh_start_xen)
or $(X86_CR0_PG | X86_CR0_PE), %eax
mov %eax, %cr0
- ljmp $__BOOT_CS, $1f
+ ljmp $PVH_CS_SEL, $1f
1:
call xen_prepare_pvh
mov $_pa(pvh_bootparams), %esi
/* startup_32 doesn't expect paging and PAE to be on. */
- ljmp $__BOOT_CS, $_pa(2f)
+ ljmp $PVH_CS_SEL, $_pa(2f)
2:
mov %cr0, %eax
and $~X86_CR0_PG, %eax
@@ -151,7 +155,7 @@ ENTRY(pvh_start_xen)
and $~X86_CR4_PAE, %eax
mov %eax, %cr4
- ljmp $__BOOT_CS, $_pa(startup_32)
+ ljmp $PVH_CS_SEL, $_pa(startup_32)
#endif
END(pvh_start_xen)
@@ -163,13 +167,12 @@ gdt:
.word 0
gdt_start:
.quad 0x0000000000000000 /* NULL descriptor */
- .quad 0x0000000000000000 /* reserved */
#ifdef CONFIG_X86_64
- .quad GDT_ENTRY(0xa09a, 0, 0xfffff) /* __KERNEL_CS */
+ .quad GDT_ENTRY(0xa09a, 0, 0xfffff) /* PVH_CS_SEL */
#else
- .quad GDT_ENTRY(0xc09a, 0, 0xfffff) /* __KERNEL_CS */
+ .quad GDT_ENTRY(0xc09a, 0, 0xfffff) /* PVH_CS_SEL */
#endif
- .quad GDT_ENTRY(0xc092, 0, 0xfffff) /* __KERNEL_DS */
+ .quad GDT_ENTRY(0xc092, 0, 0xfffff) /* PVH_DS_SEL */
.quad GDT_ENTRY(0x4090, 0, 0x18) /* PVH_CANARY_SEL */
gdt_end:
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v5 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-23 14:30 ` [PATCH v5 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
@ 2018-05-23 15:41 ` Jan Beulich
2018-05-23 16:30 ` Boris Ostrovsky
0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2018-05-23 15:41 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: brgerst, xen-devel, Juergen Gross, linux-kernel
>>> On 23.05.18 at 16:30, <boris.ostrovsky@oracle.com> wrote:
> @@ -98,6 +101,12 @@ ENTRY(pvh_start_xen)
> /* 64-bit entry point. */
> .code64
> 1:
> + /* Set base address in stack canary descriptor. */
> + mov $MSR_GS_BASE,%ecx
> + mov $_pa(canary), %rax
> + xor %rdx, %rdx
Why rax and rdx instead of eax and edx? In the former case, the
relocation produced might confuse whatever entity processing it
(it'll have a sign-extended 32-bit quantity to deal with, which
wouldn't allow representing an address in the [2Gb, 4Gb) range).
In the latter case, while surely neither performance nor code size
matter much here, it's still a bad precedent (people copy-and-paste
code all the time): Zero-ing of registers should generally use the
32-bit forms of the insn. Gas has actually gained an optimization
mode recently (upon request from Linus and the x86 maintainers)
to silently "repair" such inefficiencies.
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-23 15:41 ` Jan Beulich
@ 2018-05-23 16:30 ` Boris Ostrovsky
0 siblings, 0 replies; 5+ messages in thread
From: Boris Ostrovsky @ 2018-05-23 16:30 UTC (permalink / raw)
To: Jan Beulich; +Cc: brgerst, xen-devel, Juergen Gross, linux-kernel
On 05/23/2018 11:41 AM, Jan Beulich wrote:
>>>> On 23.05.18 at 16:30, <boris.ostrovsky@oracle.com> wrote:
>> @@ -98,6 +101,12 @@ ENTRY(pvh_start_xen)
>> /* 64-bit entry point. */
>> .code64
>> 1:
>> + /* Set base address in stack canary descriptor. */
>> + mov $MSR_GS_BASE,%ecx
>> + mov $_pa(canary), %rax
>> + xor %rdx, %rdx
> Why rax and rdx instead of eax and edx? In the former case, the
> relocation produced might confuse whatever entity processing it
> (it'll have a sign-extended 32-bit quantity to deal with, which
> wouldn't allow representing an address in the [2Gb, 4Gb) range).
> In the latter case, while surely neither performance nor code size
> matter much here, it's still a bad precedent (people copy-and-paste
> code all the time): Zero-ing of registers should generally use the
> 32-bit forms of the insn. Gas has actually gained an optimization
> mode recently (upon request from Linus and the x86 maintainers)
> to silently "repair" such inefficiencies.
Sure, I can replace these two with 32-bit variants. If there are no
other comments I won't re-send this again.
-boris
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-23 16:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-23 14:30 [PATCH v5 0/2] PVH GDT fixes Boris Ostrovsky
2018-05-23 14:30 ` [PATCH v5 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
2018-05-23 15:41 ` Jan Beulich
2018-05-23 16:30 ` Boris Ostrovsky
2018-05-23 14:30 ` [PATCH v5 2/2] xen/PVH: Make GDT selectors PVH-specific Boris Ostrovsky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).