[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 5/9] x86: Temporary disable SMAP to legally access user pages in kernel mode



>>> On 28.04.14 at 05:16, <feng.wu@xxxxxxxxx> wrote:
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -3754,6 +3754,8 @@ unsigned long do_get_debugreg(int reg)
>  
>  void asm_domain_crash_synchronous(unsigned long addr)
>  {
> +    clac();

I know it was us asking you to put this here, but that doesn't mean
you'll get away without adding a brief comment saying why it is here
(thus avoiding some janitor to come and remove this again).

> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -266,6 +266,7 @@ ENTRY(compat_int80_direct_trap)
>  /* On return only %rbx and %rdx are guaranteed non-clobbered.            */
>  compat_create_bounce_frame:
>          ASSERT_INTERRUPTS_ENABLED
> +        ASM_STAC
>          mov   %fs,%edi
>          testb $2,UREGS_cs+8(%rsp)
>          jz    1f

I think this should be deferred as much as possible; I even think it is
warranted to put this at two places here (in the two conditional
execution branches) just to avoid doing this too early.

> @@ -337,6 +338,7 @@ __UNLIKELY_END(compat_bounce_null_selector)
>          movl  %eax,UREGS_cs+8(%rsp)
>          movl  TRAPBOUNCE_eip(%rdx),%eax
>          movl  %eax,UREGS_rip+8(%rsp)
> +        ASM_CLAC
>          ret

And I think this one should be moved up as much as possible.

> @@ -439,6 +440,7 @@ UNLIKELY_START(z, create_bounce_frame_bad_bounce_ip)
>          jmp   asm_domain_crash_synchronous  /* Does not return */
>  __UNLIKELY_END(create_bounce_frame_bad_bounce_ip)
>          movq  %rax,UREGS_rip+8(%rsp)
> +        ASM_CLAC
>          ret

Same here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.