Skip to contentSkip to conversation

x86/xen: Delay get_cpu_cap until stack canary is established

PATCH Applied Jason Andryuk 3 participants 12 messages Mar 19, 2018 – Mar 31, 2018

Subsystems (3)
Stable branch x86 Xen hypervisor x86
lore
Share & tools
lore

Jason Andryuk

author
#0 lore raw

PATCH 1/1 x86/xen: Delay get_cpu_cap until stack canary is established

Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings") introduced a call to get_cpu_cap, which is fstack-protected. This is works on x86-64 as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page faults with stack protector") ensures the stack protector is configured, but it it did not cover x86-32.

Delay calling get_cpu_cap until after xen_setup_gdt has initialized the stack canary. Without this, a 32bit PV machine crashes early in boot.
(XEN) Domain 0 (vcpu#0) crashed on cpu#0:
(XEN) ----[ Xen-4.6.6-xc x86_64 debug=n Tainted: C ]----
(XEN) CPU: 0
(XEN) RIP: e019:[<00000000c10362f8>]

And the PV kernel IP corresponds to init_scattered_cpuid_features
0xc10362f8 <+24>: mov %gs:0x14,%eax

Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")

Signed-off-by Jason Andryuk <[email protected]>
---
 arch/x86/xen/enlighten_pv.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/xen/enlighten_pv.c +4 -4
index 3c2c2530737e..c36d23aa6c35 100644
@@ -1259,10 +1259,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
 	 */
 	__userpte_alloc_gfp &= ~__GFP_HIGHMEM;
 
-	/* Work out if we support NX */
-	get_cpu_cap(&boot_cpu_data);
-	x86_configure_nx();
-
 	/* Get mfn list */
 	xen_build_dynamic_phys_to_machine();
 

@@ -1272,6 +1268,10 @@ asmlinkage __visible void __init xen_start_kernel(void)
 	 */
 	xen_setup_gdt(0);
 
+	/* Work out if we support NX */
+	get_cpu_cap(&boot_cpu_data);
+	x86_configure_nx();
+
 	xen_init_irq_ops();
 
 	/* Let's presume PV guests always boot on vCPU with id 0. */
Signature · 1 line
2.14.3
reviewer Reviewed-by on v1 1/1
#1 lore raw
Quoted 2 lines · Jason Andryuk

Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings") introduced a call to get_cpu_cap, which is fstack-protected. This is works on x86-64

s/This is works/This works/

Reviewed-by Boris Ostrovsky <[email protected]>

Do we still need 4f277295e54?

-boris

Quoted 38 lines

as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page faults with stack protector") ensures the stack protector is configured, but it it did not cover x86-32.

Delay calling get_cpu_cap until after xen_setup_gdt has initialized the stack canary. Without this, a 32bit PV machine crashes early in boot.
(XEN) Domain 0 (vcpu#0) crashed on cpu#0:
(XEN) ----[ Xen-4.6.6-xc x86_64 debug=n Tainted: C ]----
(XEN) CPU: 0
(XEN) RIP: e019:[<00000000c10362f8>]

And the PV kernel IP corresponds to init_scattered_cpuid_features
0xc10362f8 <+24>: mov %gs:0x14,%eax

Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")

Signed-off-by Jason Andryuk <[email protected]>
---
 arch/x86/xen/enlighten_pv.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/xen/enlighten_pv.c +0 -4
index 3c2c2530737e..c36d23aa6c35 100644
@@ -1259,10 +1259,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
 	 */
 	__userpte_alloc_gfp &= ~__GFP_HIGHMEM;
 
-	/* Work out if we support NX */
-	get_cpu_cap(&boot_cpu_data);
-	x86_configure_nx();
-
 	/* Get mfn list */
 	xen_build_dynamic_phys_to_machine();

arch/x86/xen/enlighten_pv.c asmlinkage __visible void __init xen_start_kernel(void) · line 1277

Quoted 10 lines
arch/x86/xen/enlighten_pv.c +4 -0
@@ -1272,6 +1268,10 @@ asmlinkage __visible void __init xen_start_kernel(void)
 	 */
 	xen_setup_gdt(0);
 
+	/* Work out if we support NX */
+	get_cpu_cap(&boot_cpu_data);
+	x86_configure_nx();
+
 	xen_init_irq_ops();
 
 	/* Let's presume PV guests always boot on vCPU with id 0. */

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

reviewer Reviewed-by on v1 1/1
#2 lore raw
In reply to Jason Andryuk
Quoted 2 lines · Jason Andryuk

Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings") introduced a call to get_cpu_cap, which is fstack-protected. This is works on x86-64

s/This is works/This works/

Reviewed-by Boris Ostrovsky <[email protected]>

Do we still need 4f277295e54?

-boris

Quoted 48 lines

as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page faults with stack protector") ensures the stack protector is configured, but it it did not cover x86-32.

Delay calling get_cpu_cap until after xen_setup_gdt has initialized the stack canary. Without this, a 32bit PV machine crashes early in boot.
(XEN) Domain 0 (vcpu#0) crashed on cpu#0:
(XEN) ----[ Xen-4.6.6-xc x86_64 debug=n Tainted: C ]----
(XEN) CPU: 0
(XEN) RIP: e019:[<00000000c10362f8>]

And the PV kernel IP corresponds to init_scattered_cpuid_features
0xc10362f8 <+24>: mov %gs:0x14,%eax

Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")

Signed-off-by Jason Andryuk <[email protected]>
---
 arch/x86/xen/enlighten_pv.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/xen/enlighten_pv.c +4 -4
index 3c2c2530737e..c36d23aa6c35 100644
@@ -1259,10 +1259,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
 	 */
 	__userpte_alloc_gfp &= ~__GFP_HIGHMEM;
 
-	/* Work out if we support NX */
-	get_cpu_cap(&boot_cpu_data);
-	x86_configure_nx();
-
 	/* Get mfn list */
 	xen_build_dynamic_phys_to_machine();
 

@@ -1272,6 +1268,10 @@ asmlinkage __visible void __init xen_start_kernel(void)
 	 */
 	xen_setup_gdt(0);
 
+	/* Work out if we support NX */
+	get_cpu_cap(&boot_cpu_data);
+	x86_configure_nx();
+
 	xen_init_irq_ops();
 
 	/* Let's presume PV guests always boot on vCPU with id 0. */
maintainer
#3 lore raw
Quoted 9 lines · Boris Ostrovsky
Quoted 2 lines · Jason Andryuk

Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings") introduced a call to get_cpu_cap, which is fstack-protected. This is works on x86-64

s/This is works/This works/

Reviewed-by Boris Ostrovsky <[email protected]>

Do we still need 4f277295e54?

I'd rather keep it in order to avoid nasty problems in case something changes. After all we are trying to do an initialization in C code which should be done in assembly before entering the C part. Doing this properly for 32-bit pv-kernels would be rather difficult, but this is no reason to drop the correct solution for the 64-bit case.

Juergen

maintainer
#4 lore raw
In reply to Boris Ostrovsky
Quoted 9 lines · Boris Ostrovsky
Quoted 2 lines · Jason Andryuk

Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings") introduced a call to get_cpu_cap, which is fstack-protected. This is works on x86-64

s/This is works/This works/

Reviewed-by Boris Ostrovsky <[email protected]>

Do we still need 4f277295e54?

I'd rather keep it in order to avoid nasty problems in case something changes. After all we are trying to do an initialization in C code which should be done in assembly before entering the C part. Doing this properly for 32-bit pv-kernels would be rather difficult, but this is no reason to drop the correct solution for the 64-bit case.

Juergen

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

reviewer Applied
#5 lore raw
In reply to Jason Andryuk
Quoted 14 lines · Jason Andryuk

Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings") introduced a call to get_cpu_cap, which is fstack-protected. This is works on x86-64 as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page faults with stack protector") ensures the stack protector is configured, but it it did not cover x86-32.

Delay calling get_cpu_cap until after xen_setup_gdt has initialized the stack canary. Without this, a 32bit PV machine crashes early in boot.
(XEN) Domain 0 (vcpu#0) crashed on cpu#0:
(XEN) ----[ Xen-4.6.6-xc x86_64 debug=n Tainted: C ]----
(XEN) CPU: 0
(XEN) RIP: e019:[<00000000c10362f8>]

Quoted 7 lines

And the PV kernel IP corresponds to init_scattered_cpuid_features
0xc10362f8 <+24>: mov %gs:0x14,%eax

Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")

Signed-off-by Jason Andryuk <[email protected]>

Applied to for-linus-4.17

-boris

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

reviewer Applied
#6 lore raw
In reply to Jason Andryuk
Quoted 14 lines · Jason Andryuk

Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings") introduced a call to get_cpu_cap, which is fstack-protected. This is works on x86-64 as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page faults with stack protector") ensures the stack protector is configured, but it it did not cover x86-32.

Delay calling get_cpu_cap until after xen_setup_gdt has initialized the stack canary. Without this, a 32bit PV machine crashes early in boot.
(XEN) Domain 0 (vcpu#0) crashed on cpu#0:
(XEN) ----[ Xen-4.6.6-xc x86_64 debug=n Tainted: C ]----
(XEN) CPU: 0
(XEN) RIP: e019:[<00000000c10362f8>]

Quoted 7 lines

And the PV kernel IP corresponds to init_scattered_cpuid_features
0xc10362f8 <+24>: mov %gs:0x14,%eax

Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")

Signed-off-by Jason Andryuk <[email protected]>

Applied to for-linus-4.17

-boris

author
#7 lore raw
Quoted 18 lines · Boris Ostrovsky
Quoted 17 lines · Jason Andryuk

Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings") introduced a call to get_cpu_cap, which is fstack-protected. This is works on x86-64 as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page faults with stack protector") ensures the stack protector is configured, but it it did not cover x86-32.

Delay calling get_cpu_cap until after xen_setup_gdt has initialized the stack canary. Without this, a 32bit PV machine crashes early in boot.
(XEN) Domain 0 (vcpu#0) crashed on cpu#0:
(XEN) ----[ Xen-4.6.6-xc x86_64 debug=n Tainted: C ]----
(XEN) CPU: 0
(XEN) RIP: e019:[<00000000c10362f8>]

And the PV kernel IP corresponds to init_scattered_cpuid_features
0xc10362f8 <+24>: mov %gs:0x14,%eax

Quoted 8 lines
Quoted 4 lines

Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")

Signed-off-by Jason Andryuk <[email protected]>

Applied to for-linus-4.17

Thanks. If it's not too late, can this be cc: stable? If not, I'll submit the request after it is in Linus's tree.

-Jason

Quoted 1 line

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

reviewer
#8 lore raw
Quoted 34 lines · Jason Andryuk

On Wed, Mar 21, 2018, 5:12 PM Boris Ostrovsky <[email protected] <mailto:[email protected]>> wrote:

    On 03/19/2018 12:58 PM, Jason Andryuk wrote:
     > Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")
    introduced a
     > call to get_cpu_cap, which is fstack-protected.  This is works on
    x86-64
     > as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page
     > faults with stack protector") ensures the stack protector is
    configured,
     > but it it did not cover x86-32.
     >
     > Delay calling get_cpu_cap until after xen_setup_gdt has
    initialized the
     > stack canary.  Without this, a 32bit PV machine crashes early
     > in boot.
     > (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
     > (XEN) ----[ Xen-4.6.6-xc  x86_64  debug=n  Tainted:    C ]----
     > (XEN) CPU:    0
     > (XEN) RIP:    e019:[<00000000c10362f8>]
     >
     > And the PV kernel IP corresponds to init_scattered_cpuid_features
     >    0xc10362f8 <+24>:    mov    %gs:0x14,%eax
     >
     > Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")
     >
     > Signed-off-by: Jason Andryuk <[email protected]
    <mailto:[email protected]>>
     >


    Applied to for-linus-4.17
Quoted 1 line

Thanks. If it's not too late, can this be cc: stable?

We can always try ;-)

This is 4.15 and 4.16 only, I believe.

-boris

Quoted 4 lines

If not, I'll submit the request after it is in Linus's tree.

-Jason

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

reviewer
#9 lore raw
In reply to Jason Andryuk
Quoted 34 lines · Jason Andryuk

On Wed, Mar 21, 2018, 5:12 PM Boris Ostrovsky <[email protected] <mailto:[email protected]>> wrote:

    On 03/19/2018 12:58 PM, Jason Andryuk wrote:
     > Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")
    introduced a
     > call to get_cpu_cap, which is fstack-protected.  This is works on
    x86-64
     > as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page
     > faults with stack protector") ensures the stack protector is
    configured,
     > but it it did not cover x86-32.
     >
     > Delay calling get_cpu_cap until after xen_setup_gdt has
    initialized the
     > stack canary.  Without this, a 32bit PV machine crashes early
     > in boot.
     > (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
     > (XEN) ----[ Xen-4.6.6-xc  x86_64  debug=n  Tainted:    C ]----
     > (XEN) CPU:    0
     > (XEN) RIP:    e019:[<00000000c10362f8>]
     >
     > And the PV kernel IP corresponds to init_scattered_cpuid_features
     >    0xc10362f8 <+24>:    mov    %gs:0x14,%eax
     >
     > Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")
     >
     > Signed-off-by: Jason Andryuk <[email protected]
    <mailto:[email protected]>>
     >


    Applied to for-linus-4.17
Quoted 1 line

Thanks. If it's not too late, can this be cc: stable?

We can always try ;-)

This is 4.15 and 4.16 only, I believe.

-boris

Quoted 4 lines

If not, I'll submit the request after it is in Linus's tree.

-Jason

author
#10 lore raw
Quoted 33 lines · Boris Ostrovsky
Quoted 32 lines · Jason Andryuk

On Wed, Mar 21, 2018, 5:12 PM Boris Ostrovsky <[email protected] <mailto:[email protected]>> wrote:

    On 03/19/2018 12:58 PM, Jason Andryuk wrote:
     > Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")
    introduced a
     > call to get_cpu_cap, which is fstack-protected.  This is works on
    x86-64
     > as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page
     > faults with stack protector") ensures the stack protector is
    configured,
     > but it it did not cover x86-32.
     >
     > Delay calling get_cpu_cap until after xen_setup_gdt has
    initialized the
     > stack canary.  Without this, a 32bit PV machine crashes early
     > in boot.
     > (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
     > (XEN) ----[ Xen-4.6.6-xc  x86_64  debug=n  Tainted:    C ]----
     > (XEN) CPU:    0
     > (XEN) RIP:    e019:[<00000000c10362f8>]
     >
     > And the PV kernel IP corresponds to init_scattered_cpuid_features
     >    0xc10362f8 <+24>:    mov    %gs:0x14,%eax
     >
     > Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")
     >
     > Signed-off-by: Jason Andryuk <[email protected]
    <mailto:[email protected]>>
     >
Quoted 8 lines
Quoted 4 lines

Applied to for-linus-4.17

Thanks. If it's not too late, can this be cc: stable?

We can always try ;-)

This is 4.15 and 4.16 only, I believe.

I'm using this patch on 4.14, so there as well.

-Jason

author
#11 lore raw
In reply to Boris Ostrovsky
Quoted 33 lines · Boris Ostrovsky
Quoted 32 lines · Jason Andryuk

On Wed, Mar 21, 2018, 5:12 PM Boris Ostrovsky <[email protected] <mailto:[email protected]>> wrote:

    On 03/19/2018 12:58 PM, Jason Andryuk wrote:
     > Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")
    introduced a
     > call to get_cpu_cap, which is fstack-protected.  This is works on
    x86-64
     > as commit 4f277295e54c ("x86/xen: init %gs very early to avoid page
     > faults with stack protector") ensures the stack protector is
    configured,
     > but it it did not cover x86-32.
     >
     > Delay calling get_cpu_cap until after xen_setup_gdt has
    initialized the
     > stack canary.  Without this, a 32bit PV machine crashes early
     > in boot.
     > (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
     > (XEN) ----[ Xen-4.6.6-xc  x86_64  debug=n  Tainted:    C ]----
     > (XEN) CPU:    0
     > (XEN) RIP:    e019:[<00000000c10362f8>]
     >
     > And the PV kernel IP corresponds to init_scattered_cpuid_features
     >    0xc10362f8 <+24>:    mov    %gs:0x14,%eax
     >
     > Fixes 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings")
     >
     > Signed-off-by: Jason Andryuk <[email protected]
    <mailto:[email protected]>>
     >
Quoted 8 lines
Quoted 4 lines

Applied to for-linus-4.17

Thanks. If it's not too late, can this be cc: stable?

We can always try ;-)

This is 4.15 and 4.16 only, I believe.

I'm using this patch on 4.14, so there as well.

-Jason

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel