Message ID | 20180721142035.21059-4-hjl.tools@gmail.com |
---|---|
State | New |
Headers | show |
Series | x86/CET: The last 12 patches to enable Intel CET | expand |
On 07/21/2018 10:20 AM, H.J. Lu wrote: > Add <bits/indirect-return.h> and include it in <ucontext.h>. > __INDIRECT_RETURN defined in <bits/indirect-return.h> indicates if > swapcontext requires special compiler treatment. The default > __INDIRECT_RETURN is empty. > > On x86, when shadow stack is enabled, __INDIRECT_RETURN is defined > with indirect_return attribute, which has been added to GCC 9, to > indicate that swapcontext returns via indirect branch. Otherwise > __INDIRECT_RETURN is defined with returns_twice attribute. I saw this discussion and I think a returns_twice is the right solution for older compilers since it removes possibly incorrect optimizations. > When shadow stack is enabled, remove always_inline attribute from > prepare_test_buffer in string/tst-xbzero-opt.c to avoid: > > tst-xbzero-opt.c: In function ‘prepare_test_buffer’: > tst-xbzero-opt.c:105:1: error: function ‘prepare_test_buffer’ can never be inlined because it uses setjmp > prepare_test_buffer (unsigned char *buf) OK. > when indirect_return attribute isn't available. > > * bits/indirect-return.h: New file. > * misc/sys/cdefs.h (__glibc_has_attribute): New. > * sysdeps/x86/bits/indirect-return.h: Likewise. > * stdlib/Makefile (headers): Add bits/indirect-return.h. > * stdlib/ucontext.h: Include <bits/indirect-return.h>. > (swapcontext): Add __INDIRECT_RETURN. > * string/tst-xbzero-opt.c (ALWAYS_INLINE): New. > (prepare_test_buffer): Use it. OK with suggestion. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > --- > bits/indirect-return.h | 25 +++++++++++++++++++++ > misc/sys/cdefs.h | 6 +++++ > stdlib/Makefile | 2 +- > stdlib/ucontext.h | 6 ++++- > string/tst-xbzero-opt.c | 10 ++++++++- > sysdeps/x86/bits/indirect-return.h | 35 ++++++++++++++++++++++++++++++ > 6 files changed, 81 insertions(+), 3 deletions(-) > create mode 100644 bits/indirect-return.h > create mode 100644 sysdeps/x86/bits/indirect-return.h > > diff --git a/bits/indirect-return.h b/bits/indirect-return.h > new file mode 100644 > index 0000000000..47f6f15a6e > --- /dev/null > +++ b/bits/indirect-return.h > @@ -0,0 +1,25 @@ > +/* Definition of __INDIRECT_RETURN. Generic version. > + Copyright (C) 2018 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 > + <http://www.gnu.org/licenses/>. */ > + > +#ifndef _UCONTEXT_H > +# error "Never include <bits/indirect-return.h> directly; use <ucontext.h> instead." > +#endif > + > +/* __INDIRECT_RETURN is used on swapcontext to indicate if it requires > + special compiler treatment. */ > +#define __INDIRECT_RETURN OK. Does nothing. > diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h > index e80a45ca68..3f6fe3cc85 100644 > --- a/misc/sys/cdefs.h > +++ b/misc/sys/cdefs.h > @@ -406,6 +406,12 @@ > # define __glibc_likely(cond) (cond) > #endif > > +#ifdef __has_attribute > +# define __glibc_has_attribute(attr) __has_attribute (attr) > +#else > +# define __glibc_has_attribute(attr) 0 > +#endif OK. In gcc < 5 which lacks __has_attribute this will resolve to 0, and with that we will get returns_twice attribute being used, and that's fine. > + > #if (!defined _Noreturn \ > && (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) < 201112 \ > && !__GNUC_PREREQ (4,7)) > diff --git a/stdlib/Makefile b/stdlib/Makefile > index 808a8ceab7..b5e55b0a55 100644 > --- a/stdlib/Makefile > +++ b/stdlib/Makefile > @@ -26,7 +26,7 @@ headers := stdlib.h bits/stdlib.h bits/stdlib-ldbl.h bits/stdlib-float.h \ > monetary.h bits/monetary-ldbl.h \ > inttypes.h stdint.h bits/wordsize.h \ > errno.h sys/errno.h bits/errno.h bits/types/error_t.h \ > - ucontext.h sys/ucontext.h \ > + ucontext.h sys/ucontext.h bits/indirect-return.h \ OK. > alloca.h fmtmsg.h \ > bits/stdlib-bsearch.h sys/random.h bits/stdint-intn.h \ > bits/stdint-uintn.h > diff --git a/stdlib/ucontext.h b/stdlib/ucontext.h > index eec7611631..ec630038f6 100644 > --- a/stdlib/ucontext.h > +++ b/stdlib/ucontext.h > @@ -22,6 +22,9 @@ > > #include <features.h> > > +/* Get definition of __INDIRECT_RETURN. */ > +#include <bits/indirect-return.h> OK. > + > /* Get machine dependent definition of data structures. */ > #include <sys/ucontext.h> > > @@ -36,7 +39,8 @@ extern int setcontext (const ucontext_t *__ucp) __THROWNL; > /* Save current context in context variable pointed to by OUCP and set > context from variable pointed to by UCP. */ > extern int swapcontext (ucontext_t *__restrict __oucp, > - const ucontext_t *__restrict __ucp) __THROWNL; > + const ucontext_t *__restrict __ucp) > + __THROWNL __INDIRECT_RETURN; OK. > > /* Manipulate user context UCP to continue with calling functions FUNC > and the ARGC-1 parameters following ARGC when the context is used > diff --git a/string/tst-xbzero-opt.c b/string/tst-xbzero-opt.c > index cf7041f37a..aab4a7f715 100644 > --- a/string/tst-xbzero-opt.c > +++ b/string/tst-xbzero-opt.c > @@ -100,7 +100,15 @@ static ucontext_t uc_main, uc_co; > /* Always check the test buffer immediately after filling it; this > makes externally visible side effects depend on the buffer existing > and having been filled in. */ > -static inline __attribute__ ((always_inline)) void > +#if defined __CET__ && !__glibc_has_attribute (__indirect_return__) > +/* Note: swapcontext returns via indirect branch when SHSTK is enabled. > + Without indirect_return attribute, swapcontext is marked with > + returns_twice attribute, which prevents always_inline to work. */ > +# define ALWAYS_INLINE > +#else > +# define ALWAYS_INLINE __attribute__ ((always_inline)) > +#endif > +static inline ALWAYS_INLINE void OK. I don't think this will impact the semantics of the test, the compiler should still be able to see the local call and carry out any optimizations it needs to do. > prepare_test_buffer (unsigned char *buf) > { > for (unsigned int i = 0; i < PATTERN_REPS; i++) > diff --git a/sysdeps/x86/bits/indirect-return.h b/sysdeps/x86/bits/indirect-return.h > new file mode 100644 > index 0000000000..0587e687ac > --- /dev/null > +++ b/sysdeps/x86/bits/indirect-return.h > @@ -0,0 +1,35 @@ > +/* Definition of __INDIRECT_RETURN. x86 version. > + Copyright (C) 2018 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 > + <http://www.gnu.org/licenses/>. */ > + > +#ifndef _UCONTEXT_H > +# error "Never include <bits/indirect-return.h> directly; use <ucontext.h> instead." > +#endif > + > +/* On x86, swapcontext returns via indirect branch when the shadow stack > + is enabled. Define __INDIRECT_RETURN to indicate whether swapcontext > + returns via indirect branch. */ > +#if defined __CET__ && (__CET__ & 2) != 0 > +# if __glibc_has_attribute (__indirect_return__) > +# define __INDIRECT_RETURN __attribute__ ((__indirect_return__)) > +# else > +/* Without indirect_return attribute, use returns_twice attribute. */ Suggest: /* Newer compilers provide the indirect_return attribute, but without it we can use returns_twice to affect the optimizer in the same way and avoid unsafe optimizations. */ > +# define __INDIRECT_RETURN __attribute__ ((__returns_twice__)) > +# endif > +#else > +# define __INDIRECT_RETURN > +#endif >
diff --git a/bits/indirect-return.h b/bits/indirect-return.h new file mode 100644 index 0000000000..47f6f15a6e --- /dev/null +++ b/bits/indirect-return.h @@ -0,0 +1,25 @@ +/* Definition of __INDIRECT_RETURN. Generic version. + Copyright (C) 2018 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 + <http://www.gnu.org/licenses/>. */ + +#ifndef _UCONTEXT_H +# error "Never include <bits/indirect-return.h> directly; use <ucontext.h> instead." +#endif + +/* __INDIRECT_RETURN is used on swapcontext to indicate if it requires + special compiler treatment. */ +#define __INDIRECT_RETURN diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h index e80a45ca68..3f6fe3cc85 100644 --- a/misc/sys/cdefs.h +++ b/misc/sys/cdefs.h @@ -406,6 +406,12 @@ # define __glibc_likely(cond) (cond) #endif +#ifdef __has_attribute +# define __glibc_has_attribute(attr) __has_attribute (attr) +#else +# define __glibc_has_attribute(attr) 0 +#endif + #if (!defined _Noreturn \ && (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) < 201112 \ && !__GNUC_PREREQ (4,7)) diff --git a/stdlib/Makefile b/stdlib/Makefile index 808a8ceab7..b5e55b0a55 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -26,7 +26,7 @@ headers := stdlib.h bits/stdlib.h bits/stdlib-ldbl.h bits/stdlib-float.h \ monetary.h bits/monetary-ldbl.h \ inttypes.h stdint.h bits/wordsize.h \ errno.h sys/errno.h bits/errno.h bits/types/error_t.h \ - ucontext.h sys/ucontext.h \ + ucontext.h sys/ucontext.h bits/indirect-return.h \ alloca.h fmtmsg.h \ bits/stdlib-bsearch.h sys/random.h bits/stdint-intn.h \ bits/stdint-uintn.h diff --git a/stdlib/ucontext.h b/stdlib/ucontext.h index eec7611631..ec630038f6 100644 --- a/stdlib/ucontext.h +++ b/stdlib/ucontext.h @@ -22,6 +22,9 @@ #include <features.h> +/* Get definition of __INDIRECT_RETURN. */ +#include <bits/indirect-return.h> + /* Get machine dependent definition of data structures. */ #include <sys/ucontext.h> @@ -36,7 +39,8 @@ extern int setcontext (const ucontext_t *__ucp) __THROWNL; /* Save current context in context variable pointed to by OUCP and set context from variable pointed to by UCP. */ extern int swapcontext (ucontext_t *__restrict __oucp, - const ucontext_t *__restrict __ucp) __THROWNL; + const ucontext_t *__restrict __ucp) + __THROWNL __INDIRECT_RETURN; /* Manipulate user context UCP to continue with calling functions FUNC and the ARGC-1 parameters following ARGC when the context is used diff --git a/string/tst-xbzero-opt.c b/string/tst-xbzero-opt.c index cf7041f37a..aab4a7f715 100644 --- a/string/tst-xbzero-opt.c +++ b/string/tst-xbzero-opt.c @@ -100,7 +100,15 @@ static ucontext_t uc_main, uc_co; /* Always check the test buffer immediately after filling it; this makes externally visible side effects depend on the buffer existing and having been filled in. */ -static inline __attribute__ ((always_inline)) void +#if defined __CET__ && !__glibc_has_attribute (__indirect_return__) +/* Note: swapcontext returns via indirect branch when SHSTK is enabled. + Without indirect_return attribute, swapcontext is marked with + returns_twice attribute, which prevents always_inline to work. */ +# define ALWAYS_INLINE +#else +# define ALWAYS_INLINE __attribute__ ((always_inline)) +#endif +static inline ALWAYS_INLINE void prepare_test_buffer (unsigned char *buf) { for (unsigned int i = 0; i < PATTERN_REPS; i++) diff --git a/sysdeps/x86/bits/indirect-return.h b/sysdeps/x86/bits/indirect-return.h new file mode 100644 index 0000000000..0587e687ac --- /dev/null +++ b/sysdeps/x86/bits/indirect-return.h @@ -0,0 +1,35 @@ +/* Definition of __INDIRECT_RETURN. x86 version. + Copyright (C) 2018 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 + <http://www.gnu.org/licenses/>. */ + +#ifndef _UCONTEXT_H +# error "Never include <bits/indirect-return.h> directly; use <ucontext.h> instead." +#endif + +/* On x86, swapcontext returns via indirect branch when the shadow stack + is enabled. Define __INDIRECT_RETURN to indicate whether swapcontext + returns via indirect branch. */ +#if defined __CET__ && (__CET__ & 2) != 0 +# if __glibc_has_attribute (__indirect_return__) +# define __INDIRECT_RETURN __attribute__ ((__indirect_return__)) +# else +/* Without indirect_return attribute, use returns_twice attribute. */ +# define __INDIRECT_RETURN __attribute__ ((__returns_twice__)) +# endif +#else +# define __INDIRECT_RETURN +#endif