[03/12] Add <bits/indirect-return.h>

Message ID 20180721142035.21059-4-hjl.tools@gmail.com
State Superseded
Headers show
Series
  • x86/CET: The last 12 patches to enable Intel CET
Related show

Commit Message

H.J. Lu July 21, 2018, 2:20 p.m.
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.

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)

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.
---
 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

-- 
2.17.1

Comments

Carlos O'Donell July 24, 2018, 12:57 p.m. | #1
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

>

Patch

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