[PATCH 11/17] x86/cet: Sync with Linux kernel 6.6 shadow stack interface
szabolcs.nagy@arm.com
szabolcs.nagy@arm.com
Thu Dec 14 17:13:31 GMT 2023
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
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)
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.
(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.
> +
> +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
>
More information about the Libc-alpha
mailing list