diff mbox series

[SRU,Xenial,v2,3/4] UBUNTU: SAUCE: x86/speculation: Use x86_spec_ctrl_base in entry/exit code

Message ID 20181213132102.23677-4-juergh@canonical.com
State New
Headers show
Series None | expand

Commit Message

Juerg Haefliger Dec. 13, 2018, 1:21 p.m. UTC 
Honor the value of x86_spec_ctrl_base when manipulating the
MSR_IA32_SPEC_CTRL MSR in the entry/exit code.

CVE-2017-5715

Signed-off-by: Juerg Haefliger <juergh@canonical.com>
---
 arch/x86/include/asm/spec_ctrl.h | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Stefan Bader Jan. 9, 2019, 1:47 p.m. UTC | #1 
On 13.12.18 14:21, Juerg Haefliger wrote:
> Honor the value of x86_spec_ctrl_base when manipulating the
> MSR_IA32_SPEC_CTRL MSR in the entry/exit code.
> 
> CVE-2017-5715
> 
> Signed-off-by: Juerg Haefliger <juergh@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---

Cannot really say whether there would be a nicer way (my x86 assembly foo is
neither that good). But at least I would also read to spec the way that wrmsr
writes EDX:EAX into the MSR addressed by ECX. And EDX being the upper part.

Maybe could be

	movq x86_spec_ctrl_base, %rdx
	movl %edx, %eax
	shr $32, %rdx
	orl $SPEC_CTRL_IBRS, %eax

but I think that would be even harder to understand. So I would have no issues
with your variant.


>  arch/x86/include/asm/spec_ctrl.h | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/include/asm/spec_ctrl.h b/arch/x86/include/asm/spec_ctrl.h
> index a5d93d23390e..152c0ed1833f 100644
> --- a/arch/x86/include/asm/spec_ctrl.h
> +++ b/arch/x86/include/asm/spec_ctrl.h
> @@ -9,14 +9,17 @@
>  #ifdef __ASSEMBLY__
>  
>  .extern ibrs_enabled
> +.extern x86_spec_ctrl_base
>  
>  #define __ASM_ENABLE_IBRS			\
>  	pushq %rax;				\
>  	pushq %rcx;				\
>  	pushq %rdx;				\
>  	movl $MSR_IA32_SPEC_CTRL, %ecx;		\
> -	movl $0, %edx;				\
> -	movl $SPEC_CTRL_IBRS, %eax;		\
> +	movq x86_spec_ctrl_base, %rdx;		\
> +	shr $32, %rdx;				\
> +	movq x86_spec_ctrl_base, %rax;		\
> +	orl $SPEC_CTRL_IBRS, %eax;		\
>  	wrmsr;					\
>  	popq %rdx;				\
>  	popq %rcx;				\
> @@ -24,8 +27,10 @@
>  
>  #define __ASM_ENABLE_IBRS_CLOBBER		\
>  	movl $MSR_IA32_SPEC_CTRL, %ecx;		\
> -	movl $0, %edx;				\
> -	movl $SPEC_CTRL_IBRS, %eax;		\
> +	movq x86_spec_ctrl_base, %rdx;		\
> +	shr $32, %rdx;				\
> +	movq x86_spec_ctrl_base, %rax;		\
> +	orl $SPEC_CTRL_IBRS, %eax;		\
>  	wrmsr;
>  
>  #define __ASM_DISABLE_IBRS			\
> @@ -33,8 +38,9 @@
>  	pushq %rcx;				\
>  	pushq %rdx;				\
>  	movl $MSR_IA32_SPEC_CTRL, %ecx;		\
> -	movl $0, %edx;				\
> -	movl $0, %eax;				\
> +	movq x86_spec_ctrl_base, %rdx;		\
> +	shr $32, %rdx;				\
> +	movq x86_spec_ctrl_base, %rax;		\
>  	wrmsr;					\
>  	popq %rdx;				\
>  	popq %rcx;				\
>
Kleber Sacilotto de Souza Jan. 10, 2019, 1:42 p.m. UTC | #2 
On 12/13/18 2:21 PM, Juerg Haefliger wrote:
> Honor the value of x86_spec_ctrl_base when manipulating the
> MSR_IA32_SPEC_CTRL MSR in the entry/exit code.
>
> CVE-2017-5715
>
> Signed-off-by: Juerg Haefliger <juergh@canonical.com>


Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>


> ---
>  arch/x86/include/asm/spec_ctrl.h | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/spec_ctrl.h b/arch/x86/include/asm/spec_ctrl.h
> index a5d93d23390e..152c0ed1833f 100644
> --- a/arch/x86/include/asm/spec_ctrl.h
> +++ b/arch/x86/include/asm/spec_ctrl.h
> @@ -9,14 +9,17 @@
>  #ifdef __ASSEMBLY__
>  
>  .extern ibrs_enabled
> +.extern x86_spec_ctrl_base
>  
>  #define __ASM_ENABLE_IBRS			\
>  	pushq %rax;				\
>  	pushq %rcx;				\
>  	pushq %rdx;				\
>  	movl $MSR_IA32_SPEC_CTRL, %ecx;		\
> -	movl $0, %edx;				\
> -	movl $SPEC_CTRL_IBRS, %eax;		\
> +	movq x86_spec_ctrl_base, %rdx;		\
> +	shr $32, %rdx;				\
> +	movq x86_spec_ctrl_base, %rax;		\
> +	orl $SPEC_CTRL_IBRS, %eax;		\
>  	wrmsr;					\
>  	popq %rdx;				\
>  	popq %rcx;				\
> @@ -24,8 +27,10 @@
>  
>  #define __ASM_ENABLE_IBRS_CLOBBER		\
>  	movl $MSR_IA32_SPEC_CTRL, %ecx;		\
> -	movl $0, %edx;				\
> -	movl $SPEC_CTRL_IBRS, %eax;		\
> +	movq x86_spec_ctrl_base, %rdx;		\
> +	shr $32, %rdx;				\
> +	movq x86_spec_ctrl_base, %rax;		\
> +	orl $SPEC_CTRL_IBRS, %eax;		\
>  	wrmsr;
>  
>  #define __ASM_DISABLE_IBRS			\
> @@ -33,8 +38,9 @@
>  	pushq %rcx;				\
>  	pushq %rdx;				\
>  	movl $MSR_IA32_SPEC_CTRL, %ecx;		\
> -	movl $0, %edx;				\
> -	movl $0, %eax;				\
> +	movq x86_spec_ctrl_base, %rdx;		\
> +	shr $32, %rdx;				\
> +	movq x86_spec_ctrl_base, %rax;		\
>  	wrmsr;					\
>  	popq %rdx;				\
>  	popq %rcx;				\
Kleber Sacilotto de Souza Jan. 10, 2019, 1:47 p.m. UTC | #3 
On 12/13/18 2:21 PM, Juerg Haefliger wrote:
> Honor the value of x86_spec_ctrl_base when manipulating the
> MSR_IA32_SPEC_CTRL MSR in the entry/exit code.
>
> CVE-2017-5715
>
> Signed-off-by: Juerg Haefliger <juergh@canonical.com>
> ---
>  arch/x86/include/asm/spec_ctrl.h | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/spec_ctrl.h b/arch/x86/include/asm/spec_ctrl.h
> index a5d93d23390e..152c0ed1833f 100644
> --- a/arch/x86/include/asm/spec_ctrl.h
> +++ b/arch/x86/include/asm/spec_ctrl.h
> @@ -9,14 +9,17 @@
>  #ifdef __ASSEMBLY__
>  
>  .extern ibrs_enabled
> +.extern x86_spec_ctrl_base
>  
>  #define __ASM_ENABLE_IBRS			\
>  	pushq %rax;				\
>  	pushq %rcx;				\
>  	pushq %rdx;				\
>  	movl $MSR_IA32_SPEC_CTRL, %ecx;		\
> -	movl $0, %edx;				\
> -	movl $SPEC_CTRL_IBRS, %eax;		\
> +	movq x86_spec_ctrl_base, %rdx;		\
> +	shr $32, %rdx;				\
> +	movq x86_spec_ctrl_base, %rax;		\
> +	orl $SPEC_CTRL_IBRS, %eax;		\
>  	wrmsr;					\
>  	popq %rdx;				\
>  	popq %rcx;				\
> @@ -24,8 +27,10 @@
>  
>  #define __ASM_ENABLE_IBRS_CLOBBER		\
>  	movl $MSR_IA32_SPEC_CTRL, %ecx;		\
> -	movl $0, %edx;				\
> -	movl $SPEC_CTRL_IBRS, %eax;		\
> +	movq x86_spec_ctrl_base, %rdx;		\
> +	shr $32, %rdx;				\
> +	movq x86_spec_ctrl_base, %rax;		\
> +	orl $SPEC_CTRL_IBRS, %eax;		\
>  	wrmsr;
>  
>  #define __ASM_DISABLE_IBRS			\
> @@ -33,8 +38,9 @@
>  	pushq %rcx;				\
>  	pushq %rdx;				\
>  	movl $MSR_IA32_SPEC_CTRL, %ecx;		\
> -	movl $0, %edx;				\
> -	movl $0, %eax;				\
> +	movq x86_spec_ctrl_base, %rdx;		\
> +	shr $32, %rdx;				\
> +	movq x86_spec_ctrl_base, %rax;		\
>  	wrmsr;					\
>  	popq %rdx;				\
>  	popq %rcx;				\

Applied to xenial/master-next branch.

Thanks,
Kleber
diff mbox series

Patch

diff --git a/arch/x86/include/asm/spec_ctrl.h b/arch/x86/include/asm/spec_ctrl.h
index a5d93d23390e..152c0ed1833f 100644
--- a/arch/x86/include/asm/spec_ctrl.h
+++ b/arch/x86/include/asm/spec_ctrl.h
@@ -9,14 +9,17 @@ 
 #ifdef __ASSEMBLY__
 
 .extern ibrs_enabled
+.extern x86_spec_ctrl_base
 
 #define __ASM_ENABLE_IBRS			\
 	pushq %rax;				\
 	pushq %rcx;				\
 	pushq %rdx;				\
 	movl $MSR_IA32_SPEC_CTRL, %ecx;		\
-	movl $0, %edx;				\
-	movl $SPEC_CTRL_IBRS, %eax;		\
+	movq x86_spec_ctrl_base, %rdx;		\
+	shr $32, %rdx;				\
+	movq x86_spec_ctrl_base, %rax;		\
+	orl $SPEC_CTRL_IBRS, %eax;		\
 	wrmsr;					\
 	popq %rdx;				\
 	popq %rcx;				\
@@ -24,8 +27,10 @@ 
 
 #define __ASM_ENABLE_IBRS_CLOBBER		\
 	movl $MSR_IA32_SPEC_CTRL, %ecx;		\
-	movl $0, %edx;				\
-	movl $SPEC_CTRL_IBRS, %eax;		\
+	movq x86_spec_ctrl_base, %rdx;		\
+	shr $32, %rdx;				\
+	movq x86_spec_ctrl_base, %rax;		\
+	orl $SPEC_CTRL_IBRS, %eax;		\
 	wrmsr;
 
 #define __ASM_DISABLE_IBRS			\
@@ -33,8 +38,9 @@ 
 	pushq %rcx;				\
 	pushq %rdx;				\
 	movl $MSR_IA32_SPEC_CTRL, %ecx;		\
-	movl $0, %edx;				\
-	movl $0, %eax;				\
+	movq x86_spec_ctrl_base, %rdx;		\
+	shr $32, %rdx;				\
+	movq x86_spec_ctrl_base, %rax;		\
 	wrmsr;					\
 	popq %rdx;				\
 	popq %rcx;				\