Message ID | 20200208134317.3691-1-hjl.tools@gmail.com |
---|---|
State | New |
Headers | show |
Series | i386: Properly pop restore token in signal frame | expand |
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 >
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 + 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; \ + 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