Re: [PATCH] x86-64/cet: Check the restore token in longjmp

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, Jan 4, 2024 at 11:12 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> On Tue, Jan 2, 2024 at 6:31 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Mon, Jan 1, 2024 at 10:03 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> > >
> > > On Mon, Jan 1, 2024 at 6:03 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > setcontext and swapcontext put a restore token on the old shadow stack
> > > > which is used to restore the target shadow stack when switching user
> > > > contexts.  When longjmp from a user context, the target shadow stack
> > > > can be different from the current shadow stack and INCSSP can't be
> > > > used to restore the shadow stack pointer to the target shadow stack.
> > > >
> > > > Update longjmp to search for a restore token.  If found, use the token
> > > > to restore the shadow stack pointer before using INCSSP to pop the
> > > > shadow stack.  Stop the token search and use INCSSP if the shadow stack
> > > > entry value is the same as the current shadow stack pointer.
> > > >
> > > > It is a user error if there is a shadow stack switch without leaving a
> > > > restore token on the old shadow stack.
> > > > ---
> > > >  .../unix/sysv/linux/x86_64/____longjmp_chk.S  | 30 ++++++++++++++++++-
> > > >  sysdeps/x86_64/__longjmp.S                    | 30 ++++++++++++++++++-
> > > >  2 files changed, 58 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S b/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S
> > > > index 1b735bbbb2..855c934218 100644
> > > > --- a/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S
> > > > +++ b/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S
> > > > @@ -121,9 +121,37 @@ ENTRY(____longjmp_chk)
> > > >  # endif
> > > >         /* Check and adjust the Shadow-Stack-Pointer.  */
> > > >         rdsspq  %rax
> > > > +       /* Save the current ssp.  */
> > > > +       movq    %rax, %r10
> > > > +       movq    SHADOW_STACK_POINTER_OFFSET(%rdi), %rcx
> > > >         /* And compare it with the saved ssp value.  */
> > > > -       subq    SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
> > > > +       subq    %rcx, %rax
> > > >         je      L(skip_ssp)
> > > > +
> > > > +L(find_restore_token_loop):
> > > > +       /* Look for a restore token.  */
> > > > +       movq    -8(%rcx), %rbx
> > > > +       andq    $-8, %rbx
> > > > +       cmpq    %rcx, %rbx
> > > > +       /* Find the restore token.  */
> > > > +       je      L(restore_shadow_stack)
> > > > +
> > > > +       /* Try the next slot.  */
> > > > +       subq    $8, %rcx
> > > > +       /* Stop if the current ssp is found.  */
> > > > +       cmpq    %rcx, %r10
> > > > +       je      L(no_shadow_stack_token)
> > > > +       jmp     L(find_restore_token_loop)
> > > > +
> > > > +L(restore_shadow_stack):
> > > > +       /* Restore the target shadow stack.  */
> > > > +       rstorssp -8(%rcx)
> > > > +       /* Save the restore token on the old shadow stack.  */
> > > > +       saveprevssp
> > > > +       rdsspq  %rax
> > > > +       subq    SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
> > > > +
> > > > +L(no_shadow_stack_token):
> > > >         /* Count the number of frames to adjust and adjust it
> > > >            with incssp instruction.  The instruction can adjust
> > > >            the ssp by [0..255] value only thus use a loop if
> > > > diff --git a/sysdeps/x86_64/__longjmp.S b/sysdeps/x86_64/__longjmp.S
> > > > index 9ac075e0a8..4f449115e6 100644
> > > > --- a/sysdeps/x86_64/__longjmp.S
> > > > +++ b/sysdeps/x86_64/__longjmp.S
> > > > @@ -63,9 +63,37 @@ ENTRY(__longjmp)
> > > >         /* Check and adjust the Shadow-Stack-Pointer.  */
> > > >         /* Get the current ssp.  */
> > > >         rdsspq %rax
> > > > +       /* Save the current ssp.  */
> > > > +       movq %rax, %r10
> > > >         /* And compare it with the saved ssp value.  */
> > > > -       subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
> > > > +       movq SHADOW_STACK_POINTER_OFFSET(%rdi), %rcx
> > > > +       subq %rcx, %rax
> > > cmpq?
> >
> > %rax is used at L(no_shadow_stack_token):
> >          negq %rax
> >
> > which is the difference between the current ssp and the target
> > ssp.
> >
> > > >         je L(skip_ssp)
> > > > +
> > > > +L(find_restore_token_loop):
> > > > +       /* Look for a restore token.  */
> > > > +       movq -8(%rcx), %rbx
> > > > +       andq $-8, %rbx
> > > > +       cmpq %rcx, %rbx
> > > > +       /* Find the restore token.  */
> > > > +       je L(restore_shadow_stack)
> > > > +
> > > > +       /* Try the next slot.  */
> > > > +       subq $8, %rcx
> > > > +       /* Stop if the current ssp is found.  */
> > > > +       cmpq %rcx, %r10
> > > > +       je L(no_shadow_stack_token)
> > > > +       jmp L(find_restore_token_loop)
> > > jne  L(find_restore_token_loop)
> > > jmp L(no_shadow_stack_token)
> > > to save a branch in the loop itself.
> >
> > Fixed in v2.
> >
> > > > +
> > > > +L(restore_shadow_stack):
> > > > +       /* Restore the target shadow stack.  */
> > > > +       rstorssp -8(%rcx)
> > >
> > > Does this not need to be the aligned address?
> >
> > Shadow stack is always 8-byte aligned.
>
> If thats the case, can you use `rbx` instead of `-8(%rcx)` here?
> Also do you need the `and $-8, %rbx`?

The restore token has a special format.

This “Shadow stack restore token” is a 64-bit value formatted as follows:
 • Bit 63:2 – 4-byte aligned SSP for which this restore point was created.
   This SSP must be at an address that is 8 or 12 byte above the address
   where this token itself is found. The RSTORSSP instruction verifies
this property.
• Bit 1 – reserved. Must be zero
• Bit 0 – Mode bit. If 0 then this shadow stack restore token can be used
  by RSTORSSP instruction in 32-bit mode.    If 1 then this shadow stack
  restore token can be used by the RSTORSSP   instruction in 64-bit mode.

`and $-8, %rbx` will mark out the lower 8-bit bits before comparison
against the current SSP.

> >
> > > > +       /* Save the restore token on the old shadow stack.  */
> > > > +       saveprevssp
> > > > +       rdsspq %rax
> > > > +       subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax
> > >
> > > maybe cache `SHADOW_STACK_POINTER_OFFSET(%rdi)` in some
> > > free register above?
> >
> > Fixed in v2.
> >
> > > > +
> > > > +L(no_shadow_stack_token):
> > > Looks like duplicate between __longjmp.S and __longjmp_chk.S.
> > > Can we either 1) make these functions or 2) define them as a macro
> > > to be included by both?
> >
> > Fixed in v2.
> >
> > > >         /* Count the number of frames to adjust and adjust it
> > > >            with incssp instruction.  The instruction can adjust
> > > >            the ssp by [0..255] value only thus use a loop if
> > > > --
> > > > 2.43.0
> > > >
> >
> > Thanks.
> >
> > --
> > H.J.



-- 
H.J.