[PATCH] i386: Properly pop restore token in signal frame

Uros Bizjak ubizjak@gmail.com
Sun Feb 9 17:25:00 GMT 2020


On Sat, Feb 8, 2020 at 2:43 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> Linux CET kernel places a restore token on shadow stack for signal
> handler to enhance security.  The restore token is 8 byte and aligned
> to 8 bytes.  It is usually transparent to user programs since kernel
> will pop the restore token when signal handler returns.  But when an
> exception is thrown from a signal handler, now we need to pop the
> restore token from shadow stack.  For x86-64, we just need to treat
> the signal frame as normal frame.  For i386, we need to search for
> the restore token to check if the original shadow stack is 8 byte
> aligned.  If the original shadow stack is 8 byte aligned, we just
> need to pop 2 slots, one restore token, from shadow stack.  Otherwise,
> we need to pop 3 slots, one restore token + 4 byte pading, from
> shadow stack.
>
> This patch also includes 2 tests, one has a restore token with 4 byte
> padding and one without.
>
> Tested on Linux/x86-64 CET machine with and without -m32.  OK for master
> and backport to GCC 8/9 branches?
>
> Thanks.
>
> H.J.
> ---
> libgcc/
>
>         PR libgcc/85334
>         * config/i386/shadow-stack-unwind.h (_Unwind_Frames_Increment):
>         New.
>
> gcc/testsuite/
>
>         PR libgcc/85334
>         * g++.target/i386/pr85334-1.C: New test.
>         * g++.target/i386/pr85334-2.C: Likewise.

LGTM, with a couple of nits.

Disclaimer: I'm not expert with CET, so no thorough review here.

Thanks,
Uros.

> ---
>  gcc/testsuite/g++.target/i386/pr85334-1.C | 55 +++++++++++++++++++++++
>  gcc/testsuite/g++.target/i386/pr85334-2.C | 48 ++++++++++++++++++++
>  libgcc/config/i386/shadow-stack-unwind.h  | 43 ++++++++++++++++++
>  3 files changed, 146 insertions(+)
>  create mode 100644 gcc/testsuite/g++.target/i386/pr85334-1.C
>  create mode 100644 gcc/testsuite/g++.target/i386/pr85334-2.C
>
> diff --git a/gcc/testsuite/g++.target/i386/pr85334-1.C b/gcc/testsuite/g++.target/i386/pr85334-1.C
> new file mode 100644
> index 00000000000..3c5ccad1714
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/i386/pr85334-1.C
> @@ -0,0 +1,55 @@
> +// { dg-do run }
> +// { dg-require-effective-target cet }
> +// { dg-additional-options "-fexceptions -fnon-call-exceptions -fcf-protection" }
> +
> +// Delta between numbers of call stacks of pr85334-1.C and pr85334-2.C is 1.
> +
> +#include <signal.h>
> +#include <stdlib.h>
> +
> +void sighandler (int signo, siginfo_t * si, void * uc)
> +{
> +  throw (5);
> +}
> +
> +char *
> +__attribute ((noinline, noclone))
> +dosegv ()
> +{
> +  * ((volatile int *)0) = 12;
> +  return 0;
> +}
> +
> +int
> +__attribute ((noinline, noclone))
> +func2 ()
> +{
> +  try {
> +    dosegv ();
> +  }
> +  catch (int x) {
> +    return (x != 5);
> +  }
> +  return 1;
> +}
> +
> +int
> +__attribute ((noinline, noclone))
> +func1 ()
> +{
> +  return func2 ();
> +}
> +
> +int main ()
> +{
> +  struct sigaction sa;
> +  int status;
> +
> +  sa.sa_sigaction = sighandler;
> +  sa.sa_flags = SA_SIGINFO;
> +
> +  status = sigaction (SIGSEGV, & sa, NULL);
> +  status = sigaction (SIGBUS, & sa, NULL);
> +
> +  return func1 ();
> +}
> diff --git a/gcc/testsuite/g++.target/i386/pr85334-2.C b/gcc/testsuite/g++.target/i386/pr85334-2.C
> new file mode 100644
> index 00000000000..e2b5afe78cb
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/i386/pr85334-2.C
> @@ -0,0 +1,48 @@
> +// { dg-do run }
> +// { dg-require-effective-target cet }
> +// { dg-additional-options "-fexceptions -fnon-call-exceptions -fcf-protection" }
> +
> +// Delta between numbers of call stacks of pr85334-1.C and pr85334-2.C is 1.
> +
> +#include <signal.h>
> +#include <stdlib.h>
> +
> +void sighandler (int signo, siginfo_t * si, void * uc)
> +{
> +  throw (5);
> +}
> +
> +char *
> +__attribute ((noinline, noclone))
> +dosegv ()
> +{
> +  * ((volatile int *)0) = 12;
> +  return 0;
> +}
> +
> +int
> +__attribute ((noinline, noclone))
> +func1 ()
> +{
> +  try {
> +    dosegv ();
> +  }
> +  catch (int x) {
> +    return (x != 5);
> +  }
> +  return 1;
> +}
> +
> +int main ()
> +{
> +  struct sigaction sa;
> +  int status;
> +
> +  sa.sa_sigaction = sighandler;
> +  sa.sa_flags = SA_SIGINFO;
> +
> +  status = sigaction (SIGSEGV, & sa, NULL);
> +  status = sigaction (SIGBUS, & sa, NULL);
> +
> +  return func1 ();
> +}
> diff --git a/libgcc/config/i386/shadow-stack-unwind.h b/libgcc/config/i386/shadow-stack-unwind.h
> index a0244d2ee70..a5f9235b146 100644
> --- a/libgcc/config/i386/shadow-stack-unwind.h
> +++ b/libgcc/config/i386/shadow-stack-unwind.h
> @@ -49,3 +49,46 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>         }                                       \
>      }                                          \
>      while (0)
> +
> +/* Linux CET kernel places a restore token on shadow stack for signal
> +   handler to enhance security.  The restore token is 8 byte and aligned
> +   to 8 bytes.  It is usually transparent to user programs since kernel
> +   will pop the restore token when signal handler returns.  But when an
> +   exception is thrown from a signal handler, now we need to pop the
> +   restore token from shadow stack.  For x86-64, we just need to treat
> +   the signal frame as normal frame.  For i386, we need to search for
> +   the restore token to check if the original shadow stack is 8 byte
> +   aligned.  If the original shadow stack is 8 byte aligned, we just
> +   need to pop 2 slots, one restore token, from shadow stack.  Otherwise,
> +   we need to pop 3 slots, one restore token + 4 byte pading, from

"padding"

> +   shadow stack.  */
> +#ifndef __x86_64__
> +#undef _Unwind_Frames_Increment
> +#define _Unwind_Frames_Increment(context, frames)      \
> +  if (_Unwind_IsSignalFrame (context))                 \
> +    do                                                 \
> +      {                                                        \
> +       _Unwind_Word ssp, prev_ssp, token;              \
> +       ssp = _get_ssp ();                              \
> +       if (ssp != 0)                                   \
> +         {                                             \
> +           /* Align shadow stack pointer to the next   \
> +              8 byte aligned boundary.  */             \
> +           ssp = (ssp + 4) & -8;                       \

Please use "& ~7" form of alignment, I think this form is used more
throughout the sources.

> +           do                                          \
> +             {                                         \
> +               /* Look for a restore token.  */        \
> +               token = (*(_Unwind_Word *) (ssp - 8));  \
> +               prev_ssp = token & -8;                  \
> +               if (prev_ssp == ssp)                    \
> +                 break;                                \
> +               ssp += 8;                               \
> +             }                                         \
> +           while (1);                                  \
> +           frames += (token & 0x4) ? 3 : 2;            \
> +         }                                             \
> +      }                                                        \
> +    while (0);                                         \
> +  else                                                 \
> +    frames++;
> +#endif
> --
> 2.24.1
>



More information about the Gcc-patches mailing list