Re: [PATCH] x86: Support IBT and SHSTK in Intel CET [BZ #21598]

[Joseph Myers <joseph at codesourcery dot com>]

On Tue, 19 Dec 2017, H.J. Lu wrote:

>   b. Otherwise lock SHSTK.
> 3. After SHSTK is enabled, it is an error to load a shared object
> without GNU_PROPERTY_X86_FEATURE_1_SHSTK.

This seems like something that would be unsafe to enable by default in 
glibc / the toolchain as a whole, because it would mean that rebuilding a 
program (and the shared libraries it uses at startup) stops it from 
dlopening existing shared objects.  That is, making it an error to load 
existing shared objects should only be enabled by an explicit non-default 
option (whether configure-time option for glibc, or link-time option for 
programs that do not use dlopen) that is only used when building for a 
system where it is known that there is no need to be able to dlopen any 
existing shared object.

> We will submit a separate patch for kernel interface when kernel API
> is settled.

What depends on the kernel interface?  Is this anything to do with all the 
FIXMEs in the code?

The presence of those FIXMEs and unknown kernel interface suggests to me 
that this is not ready for 2.27 and should be reconsidered at some point 
in the 2.28 cycle or later, once the kernel support is in Linus's git 
tree.

> 	(CFLAGS-tst-context1.c): Add -fcf-protection=branch -mno-shstk
> 	-mibt.
> 	(CFLAGS-bug-getcontext.c): Likewise.
> 	(CFLAGS-tst-makecontext.c): Likewise.
> 	(CFLAGS-tst-makecontext2.c): Likewise.
> 	(CFLAGS-tst-makecontext3.c): Likewise.
> 	(CFLAGS-tst-setcontext.c): Likewise.
> 	(CFLAGS-tst-setcontext2.c): Likewise.
> 	(CFLAGS-tst-setcontext3.c): Likewise.
> 	(CFLAGS-tst-xbzero-opt.c): Likewise.

If you need those options for certain tests, I'd expect them to be needed 
for building conform/ tests as well, but don't see anything that would add 
them for those tests.

>  sysdeps/unix/sysv/linux/x86/ucontext.h             |  56 +++++++++

Again, adding sysdeps versions of installed headers that are not bits/*, 
or to a lesser extent sys/*, is a bad idea because those headers are 
liable to get out of sync with the main versions; people don't expect such 
headers to have multiple versions.

If you need to do something differently in this header for x86, you should 
add a bits/ header which has an x86-specific version.

> diff --git a/sysdeps/unix/sysv/linux/x86/check-cet.awk b/sysdeps/unix/sysv/linux/x86/check-cet.awk
> new file mode 100644
> index 0000000000..e3723f4839
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/check-cet.awk
> @@ -0,0 +1,35 @@
> +# This awk script expects to get command-line files that are each

Should start with a one-line description of the file, then copyright and 
license notices.

-- 
Joseph S. Myers
joseph@codesourcery.com