Re: [PATCH v6 05/10] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 07/05/14 09:19, Feng Wu wrote:
> Clear AC bit in RFLAGS at the beginning of exception, interrupt, hypercall,
> so Xen itself can be protected by SMAP mechanism.
>
> Signed-off-by: Feng Wu <feng.wu@intel.com>
> ---
>  xen/arch/x86/x86_64/compat/entry.S | 3 ++-
>  xen/arch/x86/x86_64/entry.S        | 7 +++++--
>  xen/arch/x86/x86_64/traps.c        | 2 +-
>  xen/include/asm-x86/asm_defns.h    | 5 ++++-
>  4 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
> index 32b3bcc..00dd0f3 100644
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -13,6 +13,7 @@
>  #include <irq_vectors.h>
>  
>  ENTRY(compat_hypercall)
> +        ASM_CLAC
>          pushq $0
>          SAVE_VOLATILE type=TRAP_syscall compat=1
>  
> @@ -178,7 +179,7 @@ ENTRY(compat_restore_all_guest)
>  
>  .section .fixup,"ax"
>  .Lfx0:  sti
> -        SAVE_ALL
> +        SAVE_ALL 0
>          movq  UREGS_error_code(%rsp),%rsi
>          movq  %rsp,%rax
>          andq  $~0xf,%rsp
> diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
> index 1c81852..205251d 100644
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -68,7 +68,7 @@ iret_exit_to_guest:
>  
>  .section .fixup,"ax"
>  .Lfx0:  sti
> -        SAVE_ALL
> +        SAVE_ALL 0
>          movq  UREGS_error_code(%rsp),%rsi
>          movq  %rsp,%rax
>          andq  $~0xf,%rsp
> @@ -273,6 +273,7 @@ ENTRY(sysenter_entry)
>          pushq $0
>          pushfq
>  GLOBAL(sysenter_eflags_saved)
> +        ASM_CLAC
>          pushq $3 /* ring 3 null cs */
>          pushq $0 /* null rip */
>          pushq $0
> @@ -309,6 +310,7 @@ UNLIKELY_END(sysenter_gpf)
>          jmp   .Lbounce_exception
>  
>  ENTRY(int80_direct_trap)
> +        ASM_CLAC
>          pushq $0
>          SAVE_VOLATILE 0x80
>  
> @@ -621,7 +623,7 @@ ENTRY(double_fault)
>  
>          .pushsection .init.text, "ax", @progbits
>  ENTRY(early_page_fault)
> -        SAVE_ALL
> +        SAVE_ALL 0
>          movq  %rsp,%rdi
>          call  do_early_page_fault
>          jmp   restore_all_xen
> @@ -673,6 +675,7 @@ ENTRY(nmi_crash)
>          ud2
>  
>  ENTRY(machine_check)
> +        ASM_CLAC

This is not needed.  the start of handle_ist_exception has a SAVE_ALL,
which also covers the nmi entry point.

On the subject of IST exceptions, perhaps the double fault explicitly
wants a STAC to reduce the likelihood of taking a further fault while
trying to dump state. ?

>          pushq $0
>          movl  $TRAP_machine_check,4(%rsp)
>          jmp   handle_ist_exception
> diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
> index 90072c1..b87b33e 100644
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -437,7 +437,7 @@ void __devinit subarch_percpu_traps_init(void)
>      /* Common SYSCALL parameters. */
>      wrmsr(MSR_STAR, 0, (FLAT_RING3_CS32<<16) | __HYPERVISOR_CS);
>      wrmsr(MSR_SYSCALL_MASK,
> -          X86_EFLAGS_VM|X86_EFLAGS_RF|X86_EFLAGS_NT|
> +          X86_EFLAGS_AC|X86_EFLAGS_VM|X86_EFLAGS_RF|X86_EFLAGS_NT|
>            X86_EFLAGS_DF|X86_EFLAGS_IF|X86_EFLAGS_TF,
>            0U);
>  }
> diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
> index 0302836..9aa3657 100644
> --- a/xen/include/asm-x86/asm_defns.h
> +++ b/xen/include/asm-x86/asm_defns.h
> @@ -190,7 +190,10 @@ static inline void stac(void)
>  #endif
>  
>  #ifdef __ASSEMBLY__
> -.macro SAVE_ALL
> +.macro SAVE_ALL clear_ac=1
> +.if \clear_ac
> +        ASM_CLAC
> +.endif
>          addq  $-(UREGS_error_code-UREGS_r15), %rsp
>          cld
>          movq  %rdi,UREGS_rdi(%rsp)