On 02.05.2020 00:58, Andrew Cooper wrote:
> When executing an IRET-to-self, the shadow stack must agree with the regular
> stack. We can't manipulate SSP directly, so have to fake a shadow IRET frame
> by executing 3 CALLs, then editing the result to look correct.
>
> This is not a fastpath, is called on the BSP long before CET can be set up,
> and may be called on the crash path after CET is disabled. Use the fact that
> INCSSP is allocated from the hint nop space to construct a test for CET being
> active which is safe on all processors.
>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
albeit with a question which may make a further change necessary:
> --- a/xen/include/asm-x86/processor.h
> +++ b/xen/include/asm-x86/processor.h
> @@ -544,17 +544,40 @@ static inline void enable_nmis(void)
> {
> unsigned long tmp;
>
> - asm volatile ( "mov %%rsp, %[tmp] \n\t"
> - "push %[ss] \n\t"
> - "push %[tmp] \n\t"
> - "pushf \n\t"
> - "push %[cs] \n\t"
> - "lea 1f(%%rip), %[tmp] \n\t"
> - "push %[tmp] \n\t"
> - "iretq; 1: \n\t"
> - : [tmp] "=&r" (tmp)
> + asm volatile ( "mov %%rsp, %[rsp] \n\t"
> + "lea .Ldone(%%rip), %[rip] \n\t"
> +#ifdef CONFIG_XEN_SHSTK
> + /* Check for CET-SS being active. */
> + "mov $1, %k[ssp] \n\t"
> + "rdsspq %[ssp] \n\t"
> + "cmp $1, %k[ssp] \n\t"
> + "je .Lshstk_done \n\t"
> +
> + /* Push 3 words on the shadow stack */
> + ".rept 3 \n\t"
> + "call 1f; nop; 1: \n\t"
> + ".endr \n\t"
> +
> + /* Fixup to be an IRET shadow stack frame */
> + "wrssq %q[cs], -1*8(%[ssp]) \n\t"
> + "wrssq %[rip], -2*8(%[ssp]) \n\t"
> + "wrssq %[ssp], -3*8(%[ssp]) \n\t"
> +
> + ".Lshstk_done:"
> +#endif
> + /* Write an IRET regular frame */
> + "push %[ss] \n\t"
> + "push %[rsp] \n\t"
> + "pushf \n\t"
> + "push %q[cs] \n\t"
> + "push %[rip] \n\t"
> + "iretq \n\t"
> + ".Ldone: \n\t"
> + : [rip] "=&r" (tmp),
> + [rsp] "=&r" (tmp),
> + [ssp] "=&r" (tmp)
Even after an hour of reading and searching through the gcc docs
I can't convince myself that this utilizes defined behavior. We
do tie multiple outputs to the same C variable elsewhere, yes,
but only in cases where we don't really care about the register,
or where the register is a fixed one anyway. What I can't find
is a clear statement that gcc wouldn't ever, now or in the
future, use the same register for all three outputs. I think one
can deduce this in certain ways, and experimentation also seems
to confirm it, but still.
Jan
