Re: [PATCH v1.1 61/65] x86/entry: Make syscall/sysenter entrypoints CET-IBT compatible

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.11.2021 17:37, Andrew Cooper wrote:
> Each of MSR_{L,C}STAR and MSR_SYSENTER_EIP need to land on an endbr64
> instruction.  For sysenter, this is easy.
> 
> Unfortunately for syscall, the stubs are already 29 byte long with a limit of
> 32.  endbr64 is 4 bytes.  Luckily, there is a 1 byte instruction which can
> move from the stubs into the main handlers.

Oh, you actually found a different solution to the space problem. I should
probably have looked here first, but this being a v1.1 it sits lower in my
mailbox ...

> Move the push %rax out of the stub and into {l,c}star_entry(), allowing room
> for the endbr64 instruction when appropriate.  Update the comment describing
> the entry state.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> @@ -295,6 +296,12 @@ static unsigned int write_stub_trampoline(
>  {
>      unsigned char *p = stub;
>  
> +    if ( cpu_has_xen_ibt )
> +    {
> +        place_endbr64(p);
> +        p += 4;
> +    }
> +
>      /* Store guest %rax into %ss slot */
>      /* movabsq %rax, stack_bottom - 8 */
>      *p++ = 0x48;
> @@ -315,10 +322,6 @@ static unsigned int write_stub_trampoline(
>      *(uint64_t *)p = stack_bottom - 8;
>      p += 8;
>  
> -    /* Store guest %rsp into %rsp slot */
> -    /* pushq %rax */
> -    *p++ = 0x50;
> -
>      /* jmp target_va */
>      *p++ = 0xe9;
>      *(int32_t *)p = target_va - (stub_va + (p - stub) + 4);

With this it's even less clear to me why you did remove the 16-byte
alignment in the other patch. Initially I expected you would extend
the "first half" of stub space by 2 bytes.

Jan