Fix stack canary handling (in the first patch) and re-index PVH GDT to
make it explicit that the GDT PVH-specific
v2:
* Dropped first patch as this was indeed issue with my tools
* Dropped second patch since the last patch should be sufficient
* Added real base address for stack canary segment
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 | 40 ++++++++++++++++++++++++++++++----------
1 file changed, 30 insertions(+), 10 deletions(-)
--
2.9.3
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 <[email protected]>
---
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 22d43eb..da8589f 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
@@ -107,7 +111,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
@@ -130,13 +134,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
@@ -145,7 +149,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)
@@ -157,13 +161,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
We are making calls to C code (e.g. xen_prepare_pvh()) which may use
stack canary (stored in GS segment).
(We have to set the segment base to @canary at runtime just like
head_32.S does, from where the code fragment is taken)
Signed-off-by: Boris Ostrovsky <[email protected]>
---
arch/x86/xen/xen-pvh.S | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S
index e1a5fbe..22d43eb 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
@@ -64,6 +67,17 @@ ENTRY(pvh_start_xen)
mov %eax,%es
mov %eax,%ss
+ /* Set base address in stack canary descriptor. */
+ movl _pa(gdt_start),%eax
+ movl $_pa(canary),%ecx
+ movw %cx, (PVH_GDT_ENTRY_CANARY * 8) + 0(%eax)
+ shrl $16, %ecx
+ movb %cl, (PVH_GDT_ENTRY_CANARY * 8) + 2(%eax)
+ movb %ch, (PVH_GDT_ENTRY_CANARY * 8) + 5(%eax)
+
+ mov $PVH_CANARY_SEL,%eax
+ mov %eax,%gs
+
/* Stash hvm_start_info. */
mov $_pa(pvh_start_info), %edi
mov %ebx, %esi
@@ -150,9 +164,12 @@ 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 24, 1, 0
early_stack:
.fill 256, 1, 0
early_stack_end:
--
2.9.3
>>> On 09.05.18 at 22:33, <[email protected]> wrote:
> @@ -64,6 +67,17 @@ ENTRY(pvh_start_xen)
> mov %eax,%es
> mov %eax,%ss
>
> + /* Set base address in stack canary descriptor. */
> + movl _pa(gdt_start),%eax
> + movl $_pa(canary),%ecx
> + movw %cx, (PVH_GDT_ENTRY_CANARY * 8) + 0(%eax)
> + shrl $16, %ecx
> + movb %cl, (PVH_GDT_ENTRY_CANARY * 8) + 2(%eax)
> + movb %ch, (PVH_GDT_ENTRY_CANARY * 8) + 5(%eax)
Is this meaningful / correct for the 64-bit case? I'd rather expect you to
write the GS base address MSR there.
Jan
On 05/14/2018 08:50 AM, Jan Beulich wrote:
>>>> On 09.05.18 at 22:33, <[email protected]> wrote:
>> @@ -64,6 +67,17 @@ ENTRY(pvh_start_xen)
>> mov %eax,%es
>> mov %eax,%ss
>>
>> + /* Set base address in stack canary descriptor. */
>> + movl _pa(gdt_start),%eax
>> + movl $_pa(canary),%ecx
>> + movw %cx, (PVH_GDT_ENTRY_CANARY * 8) + 0(%eax)
>> + shrl $16, %ecx
>> + movb %cl, (PVH_GDT_ENTRY_CANARY * 8) + 2(%eax)
>> + movb %ch, (PVH_GDT_ENTRY_CANARY * 8) + 5(%eax)
> Is this meaningful / correct for the 64-bit case? I'd rather expect you to
> write the GS base address MSR there.
Yes, I should use the MSR for 64-bit mode and existing code for 32-bit mode.
-boris