[PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
H.J. Lu
hjl.tools@gmail.com
Thu Dec 14 17:40:24 GMT 2023
On Thu, Dec 14, 2023 at 9:13 AM szabolcs.nagy@arm.com
<szabolcs.nagy@arm.com> wrote:
>
> The 12/13/2023 18:21, H.J. Lu wrote:
> > On Wed, Dec 13, 2023 at 4:20 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Wed, Dec 13, 2023 at 3:54 PM Edgecombe, Rick P
> > > <rick.p.edgecombe@intel.com> wrote:
> > > >
> > > > On Wed, 2023-12-13 at 14:45 -0800, H.J. Lu wrote:
> > > > > > longjmp cannot target arbitrary setjmp that happened on another
> > > > > > stack: if that stack is still in use by another thread then the
> > > > > > two threads clobber each other's stack. it can only work if that
> > > > > > stack is switched away from and at that point a token was placed.
> > > > > > we can make this the abi: you can only longjmp to a stack which
> > > > > > is switched away from such that a token is placed on it.
> > > > >
> > > > > The restore token is put on shadow stack by setcontext and
> > > > > swapcontext. When longjmp is called, it doesn't know if there
> > > > > is a token or not. If longjmp keeps searching for the token and
> > > > > doesn't find it, it will run out of shadow stack. Rick, can we
> > > > > assume that the shadow stack entries are 0, if they aren't in
> > > > > use?
> > > >
> > > > Not sure what you mean by entries not in use. You mean the restore
> > > > token? The shadow stack entries are not zeroed as they are popped by
> > > > RET.
> > > >
> > >
> > > longjmp needs to know when to stop searching the token since there
> > > won't be one if setcontext and swapcontext aren't called on the shadow
> > > stack where setjmp is called. Will checking for zero shadow stack entry
> > > value work here? It is OK if the value isn't zeroed by RET as long as
> > > zero value can be checked to avoid going beyond the shadow stack boundary.
> >
> > Here are 2 patches:
> >
> > 1. Add a test for longjmp from a user context.
> > 2. Check the restore token in longjmp.
> >
> > Do they make sense?
> >
> > > > longjmp() doesn't return an error code, so is a crash actually ok here?
> > > >
> > > >
> > > > I'm remembering another issue that came up when this idea was discussed
> > > > before. Apps might not call SAVEPREVSSP after they RSTORSSP. You can
> > > > just just swap away and not leave a token. So setjmp() cannot be
> > > > guaranteed to work with custom stack switching code. It has to be one
> > > > of the rules, at least for x86.
> > > >
> > > > I need to go dig through the mails, but I thought there were some more
> > > > limitations (that could also be rules) that we ran into.
>
> so the rule can be that if a user wants a stack to be resumable
> then a restore token must be placed on that stack when switching
That is fine.
> away from it, if the token is not there then longjmp is ub and
> may crash. (so no need to check for 0, such longjmp is a user error)
We need to check for 0. Otherwise, all current setjmp tests segfault
since there is no restore token on the target shadow stack.
> custom shadow stack switch code has to take this into account.
>
>
> > From ce1045ecfe9ff40fdc8c7c8cf766175229d475b7 Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Wed, 13 Dec 2023 17:50:47 -0800
> > Subject: [PATCH 2/2] x86-64/cet: Check the restore token in longjmp
> >
> > setcontext and swapcontext put a restore token on the old shadow stack
> > so that they switch to a different 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 if the shadow stack entry value
> > is zero.
> >
> > NB: The token search will segfault if there is no restore token and all
> > shadow stack entries are non-zero.
>
> this is reasonable except for a comment below.
>
> > ---
> > sysdeps/x86_64/__longjmp.S | 25 ++++++++++++++++++++++++-
> > 1 file changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/sysdeps/x86_64/__longjmp.S b/sysdeps/x86_64/__longjmp.S
> > index 9ac075e0a8..0d37e91af5 100644
> > --- a/sysdeps/x86_64/__longjmp.S
> > +++ b/sysdeps/x86_64/__longjmp.S
> > @@ -64,8 +64,31 @@ ENTRY(__longjmp)
> > /* Get the current ssp. */
> > rdsspq %rax
> > /* 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
> > je L(skip_ssp)
> > +
> > +L(find_restore_token_loop):
> > + /* Look for a restore token. */
> > + movq -8(%rcx), %rbx
> > + andq $-8, %rbx
> > + /* Stop if the shadow stack entry value is zero. */
> > + jz L(no_shadow_stack_token)
> > + cmpq %rcx, %rbx
> > + /* Find the restore token. */
> > + je L(restore_shadow_stack)
> > +
> > + /* Try the next slot. */
> > + subq $8, %rcx
> > + jmp L(find_restore_token_loop)
> > +
> > +L(restore_shadow_stack):
> > + /* Restore the target shadow stack. */
> > + rstorssp -8(%rcx)
>
> i think a restore token is needed on the old shadow stack,
> so if there was a setjmp on that stack one can resume to it.
Who will put a restore token on shadow stack when user context
isn't used? If kernel does it, CALL will override the restore token
when the end of shadow stack is reached.
> (essentially longjmp behaves like setcontext when it is used
> for stack switching: it leaves behind a resumable stack)
>
> > + 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
> > --
> > 2.43.0
> >
>
> > From bee9c6265f6eb1754c82464755af265e2587c2c9 Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Wed, 13 Dec 2023 11:13:16 -0800
> > Subject: [PATCH 1/2] Add a test for longjmp from user context
> >
> > Verify that longjmp works correctly after setcontext is called to switch
> > to a user context.
> > ---
> > stdlib/Makefile | 1 +
> > stdlib/tst-setcontext10.c | 87 +++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 88 insertions(+)
> > create mode 100644 stdlib/tst-setcontext10.c
> >
> > diff --git a/stdlib/Makefile b/stdlib/Makefile
> > index 0b154e57c5..8c6249aab4 100644
> > --- a/stdlib/Makefile
> > +++ b/stdlib/Makefile
> > @@ -234,6 +234,7 @@ tests := \
> > tst-setcontext7 \
> > tst-setcontext8 \
> > tst-setcontext9 \
> > + tst-setcontext10 \
> > tst-strfmon_l \
> > tst-strfrom \
> > tst-strfrom-locale \
> > diff --git a/stdlib/tst-setcontext10.c b/stdlib/tst-setcontext10.c
> > new file mode 100644
> > index 0000000000..a311d9f783
> > --- /dev/null
> > +++ b/stdlib/tst-setcontext10.c
> > @@ -0,0 +1,87 @@
> > +/* Check longjmp from user context.
> > + Copyright (C) 2023 Free Software Foundation, Inc.
> > + This file is part of the GNU C Library.
> > +
> > + The GNU C Library is free software; you can redistribute it and/or
> > + modify it under the terms of the GNU Lesser General Public
> > + License as published by the Free Software Foundation; either
> > + version 2.1 of the License, or (at your option) any later version.
> > +
> > + The GNU C Library is distributed in the hope that it will be useful,
> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + Lesser General Public License for more details.
> > +
> > + You should have received a copy of the GNU Lesser General Public
> > + License along with the GNU C Library; if not, see
> > + <https://www.gnu.org/licenses/>. */
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <setjmp.h>
> > +#include <ucontext.h>
> > +#include <unistd.h>
> > +
> > +static jmp_buf jmpbuf;
> > +static ucontext_t ctx;
> > +
> > +static void f2 (void);
> > +
> > +static void
> > +__attribute__ ((noinline, noclone))
> > +f1 (void)
> > +{
> > + printf ("start f1\n");
> > + f2 ();
> > +}
> > +
> > +static void
> > +__attribute__ ((noinline, noclone))
> > +f2 (void)
> > +{
> > + printf ("start f2\n");
> > + if (setcontext (&ctx) != 0)
> > + {
> > + printf ("%s: setcontext: %m\n", __FUNCTION__);
> > + exit (EXIT_FAILURE);
> > + }
> > +}
> > +
> > +static void
> > +f3 (void)
> > +{
> > + printf ("start f3\n");
> > + longjmp (jmpbuf, 1);
> > +}
>
> i would do a setjmp here before longjmp and jump back to that
> point from the main stack to see if longjmp left behind a
> resumable stack.
This is a different test. I will add it.
> > +
> > +static int
> > +__attribute__ ((noinline, noclone))
> > +do_test_1 (void)
> > +{
> > + char st1[32768];
> > +
> > + if (setjmp (jmpbuf) != 0)
> > + return 0;
> > +
> > + puts ("making contexts");
> > + if (getcontext (&ctx) != 0)
> > + {
> > + printf ("%s: getcontext: %m\n", __FUNCTION__);
> > + exit (EXIT_FAILURE);
> > + }
> > + ctx.uc_stack.ss_sp = st1;
> > + ctx.uc_stack.ss_size = sizeof st1;
> > + ctx.uc_link = NULL;
> > + makecontext (&ctx, (void (*) (void)) f3, 0);
> > + f1 ();
> > + puts ("FAIL: returned from f1 ()");
> > + exit (EXIT_FAILURE);
> > +}
> > +
> > +static int
> > +do_test (void)
> > +{
> > + return do_test_1 ();
> > +}
> > +
> > +#include <support/test-driver.c>
> > --
> > 2.43.0
> >
>
Thanks.
--
H.J.
More information about the Libc-alpha
mailing list