* [PATCH v4 0/2] PVH GDT fixes
@ 2018-05-22 3:54 Boris Ostrovsky
2018-05-22 3:54 ` [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
` (3 more replies)
0 siblings, 4 replies; 27+ messages in thread
From: Boris Ostrovsky @ 2018-05-22 3:54 UTC (permalink / raw)
To: xen-devel, linux-kernel; +Cc: jgross, JBeulich, Boris Ostrovsky
Fix stack canary handling (in the first patch) and re-index PVH GDT to
make it explicit that the GDT PVH-specific
v4:
* Load %gs after base address is calculated
* Increase stack canary segment size to 48 bytes for long 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] 27+ messages in thread
* [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 3:54 [PATCH v4 0/2] PVH GDT fixes Boris Ostrovsky
@ 2018-05-22 3:54 ` Boris Ostrovsky
2018-05-22 10:13 ` Jan Beulich
` (5 more replies)
2018-05-22 3:54 ` Boris Ostrovsky
` (2 subsequent siblings)
3 siblings, 6 replies; 27+ messages in thread
From: Boris Ostrovsky @ 2018-05-22 3:54 UTC (permalink / raw)
To: xen-devel, linux-kernel; +Cc: jgross, JBeulich, 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>
---
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..0169374 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 $canary, %rax
+ cdq
+ 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] 27+ messages in thread
* [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 3:54 [PATCH v4 0/2] PVH GDT fixes Boris Ostrovsky
2018-05-22 3:54 ` [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
@ 2018-05-22 3:54 ` Boris Ostrovsky
2018-05-22 3:54 ` [PATCH v4 2/2] xen/PVH: Make GDT selectors PVH-specific Boris Ostrovsky
2018-05-22 3:54 ` Boris Ostrovsky
3 siblings, 0 replies; 27+ messages in thread
From: Boris Ostrovsky @ 2018-05-22 3:54 UTC (permalink / raw)
To: xen-devel, linux-kernel; +Cc: jgross, Boris Ostrovsky, JBeulich
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>
---
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..0169374 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 $canary, %rax
+ cdq
+ 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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v4 2/2] xen/PVH: Make GDT selectors PVH-specific
2018-05-22 3:54 [PATCH v4 0/2] PVH GDT fixes Boris Ostrovsky
2018-05-22 3:54 ` [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
2018-05-22 3:54 ` Boris Ostrovsky
@ 2018-05-22 3:54 ` Boris Ostrovsky
2018-05-23 12:58 ` Juergen Gross
2018-05-23 12:58 ` Juergen Gross
2018-05-22 3:54 ` Boris Ostrovsky
3 siblings, 2 replies; 27+ messages in thread
From: Boris Ostrovsky @ 2018-05-22 3:54 UTC (permalink / raw)
To: xen-devel, linux-kernel; +Cc: jgross, JBeulich, 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>
---
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 0169374..e7a5bf4 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] 27+ messages in thread
* [PATCH v4 2/2] xen/PVH: Make GDT selectors PVH-specific
2018-05-22 3:54 [PATCH v4 0/2] PVH GDT fixes Boris Ostrovsky
` (2 preceding siblings ...)
2018-05-22 3:54 ` [PATCH v4 2/2] xen/PVH: Make GDT selectors PVH-specific Boris Ostrovsky
@ 2018-05-22 3:54 ` Boris Ostrovsky
3 siblings, 0 replies; 27+ messages in thread
From: Boris Ostrovsky @ 2018-05-22 3:54 UTC (permalink / raw)
To: xen-devel, linux-kernel; +Cc: jgross, Boris Ostrovsky, JBeulich
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>
---
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 0169374..e7a5bf4 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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 3:54 ` [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
@ 2018-05-22 10:13 ` Jan Beulich
2018-05-22 10:13 ` Jan Beulich
` (4 subsequent siblings)
5 siblings, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2018-05-22 10:13 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: xen-devel, Juergen Gross, linux-kernel
>>> On 22.05.18 at 05:54, <boris.ostrovsky@oracle.com> wrote:
> 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: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 3:54 ` [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
2018-05-22 10:13 ` Jan Beulich
@ 2018-05-22 10:13 ` Jan Beulich
2018-05-22 13:45 ` Brian Gerst
` (3 subsequent siblings)
5 siblings, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2018-05-22 10:13 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: Juergen Gross, xen-devel, linux-kernel
>>> On 22.05.18 at 05:54, <boris.ostrovsky@oracle.com> wrote:
> 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: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 3:54 ` [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
` (2 preceding siblings ...)
2018-05-22 13:45 ` Brian Gerst
@ 2018-05-22 13:45 ` Brian Gerst
2018-05-22 13:57 ` Jan Beulich
2018-05-22 13:57 ` Jan Beulich
2018-05-23 12:54 ` Juergen Gross
2018-05-23 12:54 ` Juergen Gross
5 siblings, 2 replies; 27+ messages in thread
From: Brian Gerst @ 2018-05-22 13:45 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: xen-devel, Linux Kernel Mailing List, Juergen Gross, Jan Beulich
On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
> 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>
> ---
> 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..0169374 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 $canary, %rax
> + cdq
> + wrmsr
CDQ only sign-extends EAX to RAX. What you really want is to move the
high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
below 4G).
--
Brian Gerst
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 3:54 ` [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
2018-05-22 10:13 ` Jan Beulich
2018-05-22 10:13 ` Jan Beulich
@ 2018-05-22 13:45 ` Brian Gerst
2018-05-22 13:45 ` Brian Gerst
` (2 subsequent siblings)
5 siblings, 0 replies; 27+ messages in thread
From: Brian Gerst @ 2018-05-22 13:45 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: Juergen Gross, xen-devel, Linux Kernel Mailing List, Jan Beulich
On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
> 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>
> ---
> 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..0169374 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 $canary, %rax
> + cdq
> + wrmsr
CDQ only sign-extends EAX to RAX. What you really want is to move the
high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
below 4G).
--
Brian Gerst
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 13:45 ` Brian Gerst
@ 2018-05-22 13:57 ` Jan Beulich
2018-05-22 15:15 ` Brian Gerst
2018-05-22 15:15 ` Brian Gerst
2018-05-22 13:57 ` Jan Beulich
1 sibling, 2 replies; 27+ messages in thread
From: Jan Beulich @ 2018-05-22 13:57 UTC (permalink / raw)
To: brgerst; +Cc: xen-devel, Boris Ostrovsky, Juergen Gross, linux-kernel
>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote:
> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <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 $canary, %rax
>> + cdq
>> + wrmsr
>
> CDQ only sign-extends EAX to RAX. What you really want is to move the
> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
> below 4G).
What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ)
sign-extends EAX to EDX:EAX.
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 13:45 ` Brian Gerst
2018-05-22 13:57 ` Jan Beulich
@ 2018-05-22 13:57 ` Jan Beulich
1 sibling, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2018-05-22 13:57 UTC (permalink / raw)
To: brgerst; +Cc: Juergen Gross, xen-devel, Boris Ostrovsky, linux-kernel
>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote:
> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <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 $canary, %rax
>> + cdq
>> + wrmsr
>
> CDQ only sign-extends EAX to RAX. What you really want is to move the
> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
> below 4G).
What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ)
sign-extends EAX to EDX:EAX.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 13:57 ` Jan Beulich
@ 2018-05-22 15:15 ` Brian Gerst
2018-05-22 16:10 ` Jan Beulich
2018-05-22 16:10 ` Jan Beulich
2018-05-22 15:15 ` Brian Gerst
1 sibling, 2 replies; 27+ messages in thread
From: Brian Gerst @ 2018-05-22 15:15 UTC (permalink / raw)
To: Jan Beulich
Cc: xen-devel, Boris Ostrovsky, Juergen Gross, Linux Kernel Mailing List
On Tue, May 22, 2018 at 9:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote:
>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <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 $canary, %rax
>>> + cdq
>>> + wrmsr
>>
>> CDQ only sign-extends EAX to RAX. What you really want is to move the
>> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
>> below 4G).
>
> What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ)
> sign-extends EAX to EDX:EAX.
But that would still be wrong, as it would set EDX to 0xFFFFFFFF if
the kernel was loaded between 2G and 4G. Looking closer at the code,
we just left 32-bit mode, so we must have been loaded below 4G,
therefore EDX must be zero.
--
Brian Gerst
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 13:57 ` Jan Beulich
2018-05-22 15:15 ` Brian Gerst
@ 2018-05-22 15:15 ` Brian Gerst
1 sibling, 0 replies; 27+ messages in thread
From: Brian Gerst @ 2018-05-22 15:15 UTC (permalink / raw)
To: Jan Beulich
Cc: Juergen Gross, xen-devel, Boris Ostrovsky, Linux Kernel Mailing List
On Tue, May 22, 2018 at 9:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote:
>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <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 $canary, %rax
>>> + cdq
>>> + wrmsr
>>
>> CDQ only sign-extends EAX to RAX. What you really want is to move the
>> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
>> below 4G).
>
> What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ)
> sign-extends EAX to EDX:EAX.
But that would still be wrong, as it would set EDX to 0xFFFFFFFF if
the kernel was loaded between 2G and 4G. Looking closer at the code,
we just left 32-bit mode, so we must have been loaded below 4G,
therefore EDX must be zero.
--
Brian Gerst
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 15:15 ` Brian Gerst
@ 2018-05-22 16:10 ` Jan Beulich
2018-05-22 16:20 ` Boris Ostrovsky
2018-05-22 16:20 ` Boris Ostrovsky
2018-05-22 16:10 ` Jan Beulich
1 sibling, 2 replies; 27+ messages in thread
From: Jan Beulich @ 2018-05-22 16:10 UTC (permalink / raw)
To: brgerst; +Cc: xen-devel, Boris Ostrovsky, Juergen Gross, linux-kernel
>>> On 22.05.18 at 17:15, <brgerst@gmail.com> wrote:
> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote:
>>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <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 $canary, %rax
>>>> + cdq
>>>> + wrmsr
>>>
>>> CDQ only sign-extends EAX to RAX. What you really want is to move the
>>> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
>>> below 4G).
>>
>> What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ)
>> sign-extends EAX to EDX:EAX.
>
> But that would still be wrong, as it would set EDX to 0xFFFFFFFF if
> the kernel was loaded between 2G and 4G. Looking closer at the code,
> we just left 32-bit mode, so we must have been loaded below 4G,
> therefore EDX must be zero.
Ah, yes, indeed.
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 15:15 ` Brian Gerst
2018-05-22 16:10 ` Jan Beulich
@ 2018-05-22 16:10 ` Jan Beulich
1 sibling, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2018-05-22 16:10 UTC (permalink / raw)
To: brgerst; +Cc: Juergen Gross, xen-devel, Boris Ostrovsky, linux-kernel
>>> On 22.05.18 at 17:15, <brgerst@gmail.com> wrote:
> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote:
>>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <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 $canary, %rax
>>>> + cdq
>>>> + wrmsr
>>>
>>> CDQ only sign-extends EAX to RAX. What you really want is to move the
>>> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
>>> below 4G).
>>
>> What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ)
>> sign-extends EAX to EDX:EAX.
>
> But that would still be wrong, as it would set EDX to 0xFFFFFFFF if
> the kernel was loaded between 2G and 4G. Looking closer at the code,
> we just left 32-bit mode, so we must have been loaded below 4G,
> therefore EDX must be zero.
Ah, yes, indeed.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 16:10 ` Jan Beulich
2018-05-22 16:20 ` Boris Ostrovsky
@ 2018-05-22 16:20 ` Boris Ostrovsky
2018-05-22 16:32 ` Jan Beulich
2018-05-22 16:32 ` Jan Beulich
1 sibling, 2 replies; 27+ messages in thread
From: Boris Ostrovsky @ 2018-05-22 16:20 UTC (permalink / raw)
To: Jan Beulich, brgerst; +Cc: xen-devel, Juergen Gross, linux-kernel
On 05/22/2018 12:10 PM, Jan Beulich wrote:
>>>> On 22.05.18 at 17:15, <brgerst@gmail.com> wrote:
>> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote:
>>>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <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 $canary, %rax
>>>>> + cdq
>>>>> + wrmsr
>>>> CDQ only sign-extends EAX to RAX. What you really want is to move the
>>>> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
>>>> below 4G).
>>> What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ)
>>> sign-extends EAX to EDX:EAX.
>> But that would still be wrong, as it would set EDX to 0xFFFFFFFF if
>> the kernel was loaded between 2G and 4G. Looking closer at the code,
>> we just left 32-bit mode, so we must have been loaded below 4G,
>> therefore EDX must be zero.
> Ah, yes, indeed.
We are loading virtual address for $canary so we will always have EDX
set to 0xffffffff. Isn't that what we want?
-borsi
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 16:10 ` Jan Beulich
@ 2018-05-22 16:20 ` Boris Ostrovsky
2018-05-22 16:20 ` Boris Ostrovsky
1 sibling, 0 replies; 27+ messages in thread
From: Boris Ostrovsky @ 2018-05-22 16:20 UTC (permalink / raw)
To: Jan Beulich, brgerst; +Cc: Juergen Gross, xen-devel, linux-kernel
On 05/22/2018 12:10 PM, Jan Beulich wrote:
>>>> On 22.05.18 at 17:15, <brgerst@gmail.com> wrote:
>> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote:
>>>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <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 $canary, %rax
>>>>> + cdq
>>>>> + wrmsr
>>>> CDQ only sign-extends EAX to RAX. What you really want is to move the
>>>> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
>>>> below 4G).
>>> What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ)
>>> sign-extends EAX to EDX:EAX.
>> But that would still be wrong, as it would set EDX to 0xFFFFFFFF if
>> the kernel was loaded between 2G and 4G. Looking closer at the code,
>> we just left 32-bit mode, so we must have been loaded below 4G,
>> therefore EDX must be zero.
> Ah, yes, indeed.
We are loading virtual address for $canary so we will always have EDX
set to 0xffffffff. Isn't that what we want?
-borsi
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 16:20 ` Boris Ostrovsky
@ 2018-05-22 16:32 ` Jan Beulich
2018-05-22 17:10 ` Boris Ostrovsky
2018-05-22 17:10 ` Boris Ostrovsky
2018-05-22 16:32 ` Jan Beulich
1 sibling, 2 replies; 27+ messages in thread
From: Jan Beulich @ 2018-05-22 16:32 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: brgerst, xen-devel, Juergen Gross, linux-kernel
>>> On 22.05.18 at 18:20, <boris.ostrovsky@oracle.com> wrote:
> On 05/22/2018 12:10 PM, Jan Beulich wrote:
>>>>> On 22.05.18 at 17:15, <brgerst@gmail.com> wrote:
>>> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote:
>>>>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <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 $canary, %rax
>>>>>> + cdq
>>>>>> + wrmsr
>>>>> CDQ only sign-extends EAX to RAX. What you really want is to move the
>>>>> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
>>>>> below 4G).
>>>> What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ)
>>>> sign-extends EAX to EDX:EAX.
>>> But that would still be wrong, as it would set EDX to 0xFFFFFFFF if
>>> the kernel was loaded between 2G and 4G. Looking closer at the code,
>>> we just left 32-bit mode, so we must have been loaded below 4G,
>>> therefore EDX must be zero.
>> Ah, yes, indeed.
>
> We are loading virtual address for $canary so we will always have EDX
> set to 0xffffffff. Isn't that what we want?
Oh, that's rather confusing - we're still running on the low 1:1
mapping when we're here. But yes, by the time we enter C code
(where the GS base starts to matter) we ought to be on the high
mappings - if only there wasn't xen_prepare_pvh().
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 16:20 ` Boris Ostrovsky
2018-05-22 16:32 ` Jan Beulich
@ 2018-05-22 16:32 ` Jan Beulich
1 sibling, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2018-05-22 16:32 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: Juergen Gross, brgerst, linux-kernel, xen-devel
>>> On 22.05.18 at 18:20, <boris.ostrovsky@oracle.com> wrote:
> On 05/22/2018 12:10 PM, Jan Beulich wrote:
>>>>> On 22.05.18 at 17:15, <brgerst@gmail.com> wrote:
>>> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote:
>>>>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <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 $canary, %rax
>>>>>> + cdq
>>>>>> + wrmsr
>>>>> CDQ only sign-extends EAX to RAX. What you really want is to move the
>>>>> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
>>>>> below 4G).
>>>> What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ)
>>>> sign-extends EAX to EDX:EAX.
>>> But that would still be wrong, as it would set EDX to 0xFFFFFFFF if
>>> the kernel was loaded between 2G and 4G. Looking closer at the code,
>>> we just left 32-bit mode, so we must have been loaded below 4G,
>>> therefore EDX must be zero.
>> Ah, yes, indeed.
>
> We are loading virtual address for $canary so we will always have EDX
> set to 0xffffffff. Isn't that what we want?
Oh, that's rather confusing - we're still running on the low 1:1
mapping when we're here. But yes, by the time we enter C code
(where the GS base starts to matter) we ought to be on the high
mappings - if only there wasn't xen_prepare_pvh().
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 16:32 ` Jan Beulich
2018-05-22 17:10 ` Boris Ostrovsky
@ 2018-05-22 17:10 ` Boris Ostrovsky
2018-05-23 7:17 ` Jan Beulich
2018-05-23 7:17 ` Jan Beulich
1 sibling, 2 replies; 27+ messages in thread
From: Boris Ostrovsky @ 2018-05-22 17:10 UTC (permalink / raw)
To: Jan Beulich; +Cc: brgerst, xen-devel, Juergen Gross, linux-kernel
On 05/22/2018 12:32 PM, Jan Beulich wrote:
>>>> On 22.05.18 at 18:20, <boris.ostrovsky@oracle.com> wrote:
>> On 05/22/2018 12:10 PM, Jan Beulich wrote:
>>>>>> On 22.05.18 at 17:15, <brgerst@gmail.com> wrote:
>>>> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote:
>>>>>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <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 $canary, %rax
>>>>>>> + cdq
>>>>>>> + wrmsr
>>>>>> CDQ only sign-extends EAX to RAX. What you really want is to move the
>>>>>> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
>>>>>> below 4G).
>>>>> What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ)
>>>>> sign-extends EAX to EDX:EAX.
>>>> But that would still be wrong, as it would set EDX to 0xFFFFFFFF if
>>>> the kernel was loaded between 2G and 4G. Looking closer at the code,
>>>> we just left 32-bit mode, so we must have been loaded below 4G,
>>>> therefore EDX must be zero.
>>> Ah, yes, indeed.
>> We are loading virtual address for $canary so we will always have EDX
>> set to 0xffffffff. Isn't that what we want?
> Oh, that's rather confusing - we're still running on the low 1:1
> mapping when we're here. But yes, by the time we enter C code
> (where the GS base starts to matter) we ought to be on the high
> mappings - if only there wasn't xen_prepare_pvh().
xen_prepare_pvh() (and whatever it might call) is the only reason for
this patch to exist. It's the only C call that we are making before
jumping to startup_64, which I assume will have to set up GS itself
before calling into C.
I didn't realize we are still on identity mapping. I'll clear EDX (and
load $_pa(canary)) then.
BTW, don't we have the same issue in startup_xen()?
-boris
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 16:32 ` Jan Beulich
@ 2018-05-22 17:10 ` Boris Ostrovsky
2018-05-22 17:10 ` Boris Ostrovsky
1 sibling, 0 replies; 27+ messages in thread
From: Boris Ostrovsky @ 2018-05-22 17:10 UTC (permalink / raw)
To: Jan Beulich; +Cc: Juergen Gross, brgerst, linux-kernel, xen-devel
On 05/22/2018 12:32 PM, Jan Beulich wrote:
>>>> On 22.05.18 at 18:20, <boris.ostrovsky@oracle.com> wrote:
>> On 05/22/2018 12:10 PM, Jan Beulich wrote:
>>>>>> On 22.05.18 at 17:15, <brgerst@gmail.com> wrote:
>>>> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote:
>>>>>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <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 $canary, %rax
>>>>>>> + cdq
>>>>>>> + wrmsr
>>>>>> CDQ only sign-extends EAX to RAX. What you really want is to move the
>>>>>> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded
>>>>>> below 4G).
>>>>> What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ)
>>>>> sign-extends EAX to EDX:EAX.
>>>> But that would still be wrong, as it would set EDX to 0xFFFFFFFF if
>>>> the kernel was loaded between 2G and 4G. Looking closer at the code,
>>>> we just left 32-bit mode, so we must have been loaded below 4G,
>>>> therefore EDX must be zero.
>>> Ah, yes, indeed.
>> We are loading virtual address for $canary so we will always have EDX
>> set to 0xffffffff. Isn't that what we want?
> Oh, that's rather confusing - we're still running on the low 1:1
> mapping when we're here. But yes, by the time we enter C code
> (where the GS base starts to matter) we ought to be on the high
> mappings - if only there wasn't xen_prepare_pvh().
xen_prepare_pvh() (and whatever it might call) is the only reason for
this patch to exist. It's the only C call that we are making before
jumping to startup_64, which I assume will have to set up GS itself
before calling into C.
I didn't realize we are still on identity mapping. I'll clear EDX (and
load $_pa(canary)) then.
BTW, don't we have the same issue in startup_xen()?
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 17:10 ` Boris Ostrovsky
2018-05-23 7:17 ` Jan Beulich
@ 2018-05-23 7:17 ` Jan Beulich
1 sibling, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2018-05-23 7:17 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: brgerst, xen-devel, Juergen Gross, linux-kernel
>>> On 22.05.18 at 19:10, <boris.ostrovsky@oracle.com> wrote:
> On 05/22/2018 12:32 PM, Jan Beulich wrote:
>>>>> On 22.05.18 at 18:20, <boris.ostrovsky@oracle.com> wrote:
>>> We are loading virtual address for $canary so we will always have EDX
>>> set to 0xffffffff. Isn't that what we want?
>> Oh, that's rather confusing - we're still running on the low 1:1
>> mapping when we're here. But yes, by the time we enter C code
>> (where the GS base starts to matter) we ought to be on the high
>> mappings - if only there wasn't xen_prepare_pvh().
>
> xen_prepare_pvh() (and whatever it might call) is the only reason for
> this patch to exist. It's the only C call that we are making before
> jumping to startup_64, which I assume will have to set up GS itself
> before calling into C.
>
> I didn't realize we are still on identity mapping. I'll clear EDX (and
> load $_pa(canary)) then.
>
> BTW, don't we have the same issue in startup_xen()?
I don't think so, no - there we're on the high mappings already (the
ELF note specifies the virtual address of the entry point, after all).
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 17:10 ` Boris Ostrovsky
@ 2018-05-23 7:17 ` Jan Beulich
2018-05-23 7:17 ` Jan Beulich
1 sibling, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2018-05-23 7:17 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: Juergen Gross, brgerst, linux-kernel, xen-devel
>>> On 22.05.18 at 19:10, <boris.ostrovsky@oracle.com> wrote:
> On 05/22/2018 12:32 PM, Jan Beulich wrote:
>>>>> On 22.05.18 at 18:20, <boris.ostrovsky@oracle.com> wrote:
>>> We are loading virtual address for $canary so we will always have EDX
>>> set to 0xffffffff. Isn't that what we want?
>> Oh, that's rather confusing - we're still running on the low 1:1
>> mapping when we're here. But yes, by the time we enter C code
>> (where the GS base starts to matter) we ought to be on the high
>> mappings - if only there wasn't xen_prepare_pvh().
>
> xen_prepare_pvh() (and whatever it might call) is the only reason for
> this patch to exist. It's the only C call that we are making before
> jumping to startup_64, which I assume will have to set up GS itself
> before calling into C.
>
> I didn't realize we are still on identity mapping. I'll clear EDX (and
> load $_pa(canary)) then.
>
> BTW, don't we have the same issue in startup_xen()?
I don't think so, no - there we're on the high mappings already (the
ELF note specifies the virtual address of the entry point, after all).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 3:54 ` [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
` (4 preceding siblings ...)
2018-05-23 12:54 ` Juergen Gross
@ 2018-05-23 12:54 ` Juergen Gross
5 siblings, 0 replies; 27+ messages in thread
From: Juergen Gross @ 2018-05-23 12:54 UTC (permalink / raw)
To: Boris Ostrovsky, xen-devel, linux-kernel; +Cc: JBeulich
On 22/05/18 05:54, Boris Ostrovsky wrote:
> 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>
With the clearing of EDX (instead using CDQ) you can add my
Reviewed-by: Juergen Gross <jgross@suse.com>
Juergen
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary
2018-05-22 3:54 ` [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
` (3 preceding siblings ...)
2018-05-22 13:45 ` Brian Gerst
@ 2018-05-23 12:54 ` Juergen Gross
2018-05-23 12:54 ` Juergen Gross
5 siblings, 0 replies; 27+ messages in thread
From: Juergen Gross @ 2018-05-23 12:54 UTC (permalink / raw)
To: Boris Ostrovsky, xen-devel, linux-kernel; +Cc: JBeulich
On 22/05/18 05:54, Boris Ostrovsky wrote:
> 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>
With the clearing of EDX (instead using CDQ) you can add my
Reviewed-by: Juergen Gross <jgross@suse.com>
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 2/2] xen/PVH: Make GDT selectors PVH-specific
2018-05-22 3:54 ` [PATCH v4 2/2] xen/PVH: Make GDT selectors PVH-specific Boris Ostrovsky
@ 2018-05-23 12:58 ` Juergen Gross
2018-05-23 12:58 ` Juergen Gross
1 sibling, 0 replies; 27+ messages in thread
From: Juergen Gross @ 2018-05-23 12:58 UTC (permalink / raw)
To: Boris Ostrovsky, xen-devel, linux-kernel; +Cc: JBeulich
On 22/05/18 05:54, Boris Ostrovsky wrote:
> 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>
Juergen
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 2/2] xen/PVH: Make GDT selectors PVH-specific
2018-05-22 3:54 ` [PATCH v4 2/2] xen/PVH: Make GDT selectors PVH-specific Boris Ostrovsky
2018-05-23 12:58 ` Juergen Gross
@ 2018-05-23 12:58 ` Juergen Gross
1 sibling, 0 replies; 27+ messages in thread
From: Juergen Gross @ 2018-05-23 12:58 UTC (permalink / raw)
To: Boris Ostrovsky, xen-devel, linux-kernel; +Cc: JBeulich
On 22/05/18 05:54, Boris Ostrovsky wrote:
> 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>
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2018-05-23 12:58 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-22 3:54 [PATCH v4 0/2] PVH GDT fixes Boris Ostrovsky
2018-05-22 3:54 ` [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary Boris Ostrovsky
2018-05-22 10:13 ` Jan Beulich
2018-05-22 10:13 ` Jan Beulich
2018-05-22 13:45 ` Brian Gerst
2018-05-22 13:45 ` Brian Gerst
2018-05-22 13:57 ` Jan Beulich
2018-05-22 15:15 ` Brian Gerst
2018-05-22 16:10 ` Jan Beulich
2018-05-22 16:20 ` Boris Ostrovsky
2018-05-22 16:20 ` Boris Ostrovsky
2018-05-22 16:32 ` Jan Beulich
2018-05-22 17:10 ` Boris Ostrovsky
2018-05-22 17:10 ` Boris Ostrovsky
2018-05-23 7:17 ` Jan Beulich
2018-05-23 7:17 ` Jan Beulich
2018-05-22 16:32 ` Jan Beulich
2018-05-22 16:10 ` Jan Beulich
2018-05-22 15:15 ` Brian Gerst
2018-05-22 13:57 ` Jan Beulich
2018-05-23 12:54 ` Juergen Gross
2018-05-23 12:54 ` Juergen Gross
2018-05-22 3:54 ` Boris Ostrovsky
2018-05-22 3:54 ` [PATCH v4 2/2] xen/PVH: Make GDT selectors PVH-specific Boris Ostrovsky
2018-05-23 12:58 ` Juergen Gross
2018-05-23 12:58 ` Juergen Gross
2018-05-22 3:54 ` Boris Ostrovsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.