[Xen-devel] [PATCH v6.5 08/26] x86/entry: Erase guest GPR state on entry to Xen

2018-01-03 Thread Andrew Cooper
This reduces the number of code gadgets which can be attacked with arbitrary
guest-controlled GPR values.

Signed-off-by: Andrew Cooper 
Reviewed-by: Jan Beulich 
Reviewed-by: Wei Liu 
---
 xen/include/asm-x86/asm_defns.h | 16 
 1 file changed, 16 insertions(+)

diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index fa62c54..7e8838e 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -217,22 +217,34 @@ static always_inline void stac(void)
 addq  $-(UREGS_error_code-UREGS_r15), %rsp
 cld
 movq  %rdi,UREGS_rdi(%rsp)
+xor   %edi, %edi
 movq  %rsi,UREGS_rsi(%rsp)
+xor   %esi, %esi
 movq  %rdx,UREGS_rdx(%rsp)
+xor   %edx, %edx
 movq  %rcx,UREGS_rcx(%rsp)
+xor   %ecx, %ecx
 movq  %rax,UREGS_rax(%rsp)
+xor   %eax, %eax
 .if !\compat
 movq  %r8,UREGS_r8(%rsp)
 movq  %r9,UREGS_r9(%rsp)
 movq  %r10,UREGS_r10(%rsp)
 movq  %r11,UREGS_r11(%rsp)
 .endif
+xor   %r8, %r8
+xor   %r9, %r9
+xor   %r10, %r10
+xor   %r11, %r11
 movq  %rbx,UREGS_rbx(%rsp)
+xor   %ebx, %ebx
 movq  %rbp,UREGS_rbp(%rsp)
 #ifdef CONFIG_FRAME_POINTER
 /* Indicate special exception stack frame by inverting the frame pointer. */
 leaq  UREGS_rbp(%rsp), %rbp
 notq  %rbp
+#else
+xor   %ebp, %ebp
 #endif
 .if !\compat
 movq  %r12,UREGS_r12(%rsp)
@@ -240,6 +252,10 @@ static always_inline void stac(void)
 movq  %r14,UREGS_r14(%rsp)
 movq  %r15,UREGS_r15(%rsp)
 .endif
+xor   %r12, %r12
+xor   %r13, %r13
+xor   %r14, %r14
+xor   %r15, %r15
 .endm
 
 #define LOAD_ONE_REG(reg, compat) \
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6.5 08/26] x86/entry: Erase guest GPR state on entry to Xen

2018-01-22 Thread David Woodhouse
On Thu, 2018-01-04 at 00:15 +, Andrew Cooper wrote:
> 
> --- a/xen/include/asm-x86/asm_defns.h
> +++ b/xen/include/asm-x86/asm_defns.h
> @@ -217,22 +217,34 @@ static always_inline void stac(void)
>  addq  $-(UREGS_error_code-UREGS_r15), %rsp
>  cld
>  movq  %rdi,UREGS_rdi(%rsp)
> +    xor   %edi, %edi
>  movq  %rsi,UREGS_rsi(%rsp)
> +    xor   %esi, %esi
>  movq  %rdx,UREGS_rdx(%rsp)
> +    xor   %edx, %edx
>  movq  %rcx,UREGS_rcx(%rsp)
> +    xor   %ecx, %ecx
>  movq  %rax,UREGS_rax(%rsp)
> +    xor   %eax, %eax

You didn't want to erase all 64 bits?


smime.p7s
Description: S/MIME cryptographic signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6.5 08/26] x86/entry: Erase guest GPR state on entry to Xen

2018-01-22 Thread Andrew Cooper
On 22/01/2018 10:04, David Woodhouse wrote:
> On Thu, 2018-01-04 at 00:15 +, Andrew Cooper wrote:
>> --- a/xen/include/asm-x86/asm_defns.h
>> +++ b/xen/include/asm-x86/asm_defns.h
>> @@ -217,22 +217,34 @@ static always_inline void stac(void)
>>  addq  $-(UREGS_error_code-UREGS_r15), %rsp
>>  cld
>>  movq  %rdi,UREGS_rdi(%rsp)
>> +    xor   %edi, %edi
>>  movq  %rsi,UREGS_rsi(%rsp)
>> +    xor   %esi, %esi
>>  movq  %rdx,UREGS_rdx(%rsp)
>> +    xor   %edx, %edx
>>  movq  %rcx,UREGS_rcx(%rsp)
>> +    xor   %ecx, %ecx
>>  movq  %rax,UREGS_rax(%rsp)
>> +    xor   %eax, %eax
> You didn't want to erase all 64 bits?

This does erase all 64 bits.  (We're in long mode, so the upper 32 bits
are implicitly zeroed, without an added rex prefix.)

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v6.5 08/26] x86/entry: Erase guest GPR state on entry to Xen

2018-01-22 Thread David Woodhouse
On Mon, 2018-01-22 at 10:18 +, Andrew Cooper wrote:
> On 22/01/2018 10:04, David Woodhouse wrote:
> > 
> > On Thu, 2018-01-04 at 00:15 +, Andrew Cooper wrote:
> > > 
> > > --- a/xen/include/asm-x86/asm_defns.h
> > > +++ b/xen/include/asm-x86/asm_defns.h
> > > @@ -217,22 +217,34 @@ static always_inline void stac(void)
> > >  addq  $-(UREGS_error_code-UREGS_r15), %rsp
> > >  cld
> > >  movq  %rdi,UREGS_rdi(%rsp)
> > > +    xor   %edi, %edi
> > >  movq  %rsi,UREGS_rsi(%rsp)
> > > +    xor   %esi, %esi
> > >  movq  %rdx,UREGS_rdx(%rsp)
> > > +    xor   %edx, %edx
> > >  movq  %rcx,UREGS_rcx(%rsp)
> > > +    xor   %ecx, %ecx
> > >  movq  %rax,UREGS_rax(%rsp)
> > > +    xor   %eax, %eax
> > You didn't want to erase all 64 bits?
>
> This does erase all 64 bits.  (We're in long mode, so the upper 32 bits
> are implicitly zeroed, without an added rex prefix.)

Eww. In the grand scheme of things, I'd rather the assembler knew that
(and happily omitted the rex prefix all by itself to use the more
efficient encoding of the instruction), and not me.

smime.p7s
Description: S/MIME cryptographic signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel