On 02.05.2020 00:58, Andrew Cooper wrote:
> The SYSCALL/SYSEXIT paths need to use {SET,CLR}SSBSY.
I take it you mean SYSRET, not SYSEXIT. I do think though that you
also need to deal with the SYSENTER entry point we have.
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -198,7 +198,7 @@ ENTRY(cr4_pv32_restore)
>
> /* See lstar_enter for entry register state. */
> ENTRY(cstar_enter)
> - /* sti could live here when we don't switch page tables below. */
> + ALTERNATIVE "", "setssbsy", X86_FEATURE_XEN_SHSTK
I don't see why you delete the comment here (or elsewhere). While
I recall you not really wanting them there, I still think they're
useful to have, and they shouldn't be deleted as a side effect of
an entirely unrelated change. Of course they need to live after
your insertions then.
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -194,6 +194,15 @@ restore_all_guest:
> movq 8(%rsp),%rcx # RIP
> ja iret_exit_to_guest
>
> + /* Clear the supervisor shadow stack token busy bit. */
> +.macro rag_clrssbsy
> + push %rax
> + rdsspq %rax
> + clrssbsy (%rax)
> + pop %rax
> +.endm
> + ALTERNATIVE "", rag_clrssbsy, X86_FEATURE_XEN_SHSTK
In principle you could get away without spilling %rax:
cmpl $1,%ecx
ja iret_exit_to_guest
/* Clear the supervisor shadow stack token busy bit. */
.macro rag_clrssbsy
rdsspq %rcx
clrssbsy (%rcx)
.endm
ALTERNATIVE "", rag_clrssbsy, X86_FEATURE_XEN_SHSTK
movq 8(%rsp),%rcx # RIP
cmpw $FLAT_USER_CS32,16(%rsp)# CS
movq 32(%rsp),%rsp # RSP
je 1f
sysretq
1: sysretl
ALIGN
/* No special register assumptions. */
iret_exit_to_guest:
movq 8(%rsp),%rcx # RIP
andl $~(X86_EFLAGS_IOPL|X86_EFLAGS_NT|X86_EFLAGS_VM),24(%rsp)
...
Also - what about CLRSSBSY failing? It would seem easier to diagnose
this right here than when getting presumably #DF upon next entry into
Xen. At the very least I think it deserves a comment if an error case
does not get handled.
Somewhat similar for SETSSBSY, except there things get complicated by
it raising #CP instead of setting EFLAGS.CF: Aiui it would require us
to handle #CP on an IST stack in order to avoid #DF there.
> @@ -877,6 +886,14 @@ handle_ist_exception:
> movl $UREGS_kernel_sizeof/8,%ecx
> movq %rdi,%rsp
> rep movsq
> +
> + /* Switch Shadow Stacks */
> +.macro ist_switch_shstk
> + rdsspq %rdi
> + clrssbsy (%rdi)
> + setssbsy
> +.endm
Could you extend the comment to mention the caveat that you point
out in the description?
Jan
