Re: [PATCH] x86: Disable CET instrumentation in the kernel
On Fri, Jan 29, 2021 at 06:07:55PM +0100, Borislav Petkov wrote:
> On Fri, Jan 29, 2021 at 11:03:31AM -0600, Josh Poimboeuf wrote:
> > On Fri, Jan 29, 2021 at 06:54:08PM +0200, Nikolay Borisov wrote:
> > >
> > >
> > > On 29.01.21 г. 18:49 ч., Josh Poimboeuf wrote:
> > > > Agreed, stable is a good idea. I think Nikolay saw it with GCC 9.
> > >
> > >
> > > Yes I did, with the default Ubuntu compiler as well as the default gcc-10
> > > compiler:
> > >
> > > # gcc -v -Q -O2 --help=target | grep protection
> > >
> > > gcc version 9.3.0 (Ubuntu 9.3.0-17ubuntu1~20.04)
> > > COLLECT_GCC_OPTIONS='-v' '-Q' '-O2' '--help=target' '-mtune=generic'
> > > '-march=x86-64'
> > > /usr/lib/gcc/x86_64-linux-gnu/9/cc1 -v -imultiarch x86_64-linux-gnu
> > > help-dummy -dumpbase help-dummy -mtune=generic -march=x86-64 -auxbase
> > > help-dummy -O2 -version --help=target -fasynchronous-unwind-tables
> > > -fstack-protector-strong -Wformat -Wformat-security
> > > -fstack-clash-protection -fcf-protection -o /tmp/ccSecttk.s
> > > GNU C17 (Ubuntu 9.3.0-17ubuntu1~20.04) version 9.3.0 (x86_64-linux-gnu)
> > > compiled by GNU C version 9.3.0, GMP version 6.2.0, MPFR version 4.0.2,
> > > MPC version 1.1.0, isl version isl-0.22.1-GMP
> > >
> > >
> > > It has -fcf-protection turned on by default it seems.
> >
> > Yup, explains why I didn't see it:
> >
> > gcc version 10.2.1 20201125 (Red Hat 10.2.1-9) (GCC)
> > COLLECT_GCC_OPTIONS='-v' '-Q' '-O2' '--help=target' '-mtune=generic'
> > '-march=x86-64'
> > /usr/libexec/gcc/x86_64-redhat-linux/10/cc1 -v help-dummy -dumpbase
> > help-dummy -mtune=generic -march=x86-64 -auxbase help-dummy -O2 -version
> > --help=target -o /tmp/cclBz55H.s
>
> The fact that you triggered it with an Ubuntu gcc explains why the
> original patch adding that switch:
>
> 29be86d7f9cb ("kbuild: add -fcf-protection=none when using retpoline flags")
>
> came from a Canonical.
>
> Adding the author to Cc for FYI.
>
> Seth, you can find this thread starting here:
>
> https://lkml.kernel.org/r/20210128215219.6kct3h2eiustncws@treble
Thanks for the heads up. This still works fine for our needs.
Acked-by: Seth Forshee
Re: [PATCH] x86: Disable CET instrumentation in the kernel
On Fri, Jan 29, 2021 at 11:03:31AM -0600, Josh Poimboeuf wrote:
> On Fri, Jan 29, 2021 at 06:54:08PM +0200, Nikolay Borisov wrote:
> >
> >
> > On 29.01.21 г. 18:49 ч., Josh Poimboeuf wrote:
> > > Agreed, stable is a good idea. I think Nikolay saw it with GCC 9.
> >
> >
> > Yes I did, with the default Ubuntu compiler as well as the default gcc-10
> > compiler:
> >
> > # gcc -v -Q -O2 --help=target | grep protection
> >
> > gcc version 9.3.0 (Ubuntu 9.3.0-17ubuntu1~20.04)
> > COLLECT_GCC_OPTIONS='-v' '-Q' '-O2' '--help=target' '-mtune=generic'
> > '-march=x86-64'
> > /usr/lib/gcc/x86_64-linux-gnu/9/cc1 -v -imultiarch x86_64-linux-gnu
> > help-dummy -dumpbase help-dummy -mtune=generic -march=x86-64 -auxbase
> > help-dummy -O2 -version --help=target -fasynchronous-unwind-tables
> > -fstack-protector-strong -Wformat -Wformat-security
> > -fstack-clash-protection -fcf-protection -o /tmp/ccSecttk.s
> > GNU C17 (Ubuntu 9.3.0-17ubuntu1~20.04) version 9.3.0 (x86_64-linux-gnu)
> > compiled by GNU C version 9.3.0, GMP version 6.2.0, MPFR version 4.0.2,
> > MPC version 1.1.0, isl version isl-0.22.1-GMP
> >
> >
> > It has -fcf-protection turned on by default it seems.
>
> Yup, explains why I didn't see it:
>
> gcc version 10.2.1 20201125 (Red Hat 10.2.1-9) (GCC)
> COLLECT_GCC_OPTIONS='-v' '-Q' '-O2' '--help=target' '-mtune=generic'
> '-march=x86-64'
> /usr/libexec/gcc/x86_64-redhat-linux/10/cc1 -v help-dummy -dumpbase
> help-dummy -mtune=generic -march=x86-64 -auxbase help-dummy -O2 -version
> --help=target -o /tmp/cclBz55H.s
The fact that you triggered it with an Ubuntu gcc explains why the
original patch adding that switch:
29be86d7f9cb ("kbuild: add -fcf-protection=none when using retpoline flags")
came from a Canonical.
Adding the author to Cc for FYI.
Seth, you can find this thread starting here:
https://lkml.kernel.org/r/20210128215219.6kct3h2eiustncws@treble
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86: Disable CET instrumentation in the kernel
On Fri, Jan 29, 2021 at 06:54:08PM +0200, Nikolay Borisov wrote: > > > On 29.01.21 г. 18:49 ч., Josh Poimboeuf wrote: > > Agreed, stable is a good idea. I think Nikolay saw it with GCC 9. > > > Yes I did, with the default Ubuntu compiler as well as the default gcc-10 > compiler: > > # gcc -v -Q -O2 --help=target | grep protection > > gcc version 9.3.0 (Ubuntu 9.3.0-17ubuntu1~20.04) > COLLECT_GCC_OPTIONS='-v' '-Q' '-O2' '--help=target' '-mtune=generic' > '-march=x86-64' > /usr/lib/gcc/x86_64-linux-gnu/9/cc1 -v -imultiarch x86_64-linux-gnu > help-dummy -dumpbase help-dummy -mtune=generic -march=x86-64 -auxbase > help-dummy -O2 -version --help=target -fasynchronous-unwind-tables > -fstack-protector-strong -Wformat -Wformat-security -fstack-clash-protection > -fcf-protection -o /tmp/ccSecttk.s > GNU C17 (Ubuntu 9.3.0-17ubuntu1~20.04) version 9.3.0 (x86_64-linux-gnu) > compiled by GNU C version 9.3.0, GMP version 6.2.0, MPFR version 4.0.2, > MPC version 1.1.0, isl version isl-0.22.1-GMP > > > It has -fcf-protection turned on by default it seems. Yup, explains why I didn't see it: gcc version 10.2.1 20201125 (Red Hat 10.2.1-9) (GCC) COLLECT_GCC_OPTIONS='-v' '-Q' '-O2' '--help=target' '-mtune=generic' '-march=x86-64' /usr/libexec/gcc/x86_64-redhat-linux/10/cc1 -v help-dummy -dumpbase help-dummy -mtune=generic -march=x86-64 -auxbase help-dummy -O2 -version --help=target -o /tmp/cclBz55H.s -- Josh
Re: [PATCH] x86: Disable CET instrumentation in the kernel
On 29.01.21 г. 18:49 ч., Josh Poimboeuf wrote: > Agreed, stable is a good idea. I think Nikolay saw it with GCC 9. Yes I did, with the default Ubuntu compiler as well as the default gcc-10 compiler: # gcc -v -Q -O2 --help=target | grep protection gcc version 9.3.0 (Ubuntu 9.3.0-17ubuntu1~20.04) COLLECT_GCC_OPTIONS='-v' '-Q' '-O2' '--help=target' '-mtune=generic' '-march=x86-64' /usr/lib/gcc/x86_64-linux-gnu/9/cc1 -v -imultiarch x86_64-linux-gnu help-dummy -dumpbase help-dummy -mtune=generic -march=x86-64 -auxbase help-dummy -O2 -version --help=target -fasynchronous-unwind-tables -fstack-protector-strong -Wformat -Wformat-security -fstack-clash-protection -fcf-protection -o /tmp/ccSecttk.s GNU C17 (Ubuntu 9.3.0-17ubuntu1~20.04) version 9.3.0 (x86_64-linux-gnu) compiled by GNU C version 9.3.0, GMP version 6.2.0, MPFR version 4.0.2, MPC version 1.1.0, isl version isl-0.22.1-GMP It has -fcf-protection turned on by default it seems.
Re: [PATCH] x86: Disable CET instrumentation in the kernel
On Fri, Jan 29, 2021 at 05:30:48PM +0100, Borislav Petkov wrote: > On Fri, Jan 29, 2021 at 09:10:34AM -0600, Josh Poimboeuf wrote: > > Maybe eventually. But the enablement (actually enabling CET/CFI/etc) > > happens in the arch code anyway, right? So it could be a per-arch > > decision. > > Right. > > Ok, for this one, what about > > Cc: > > ? > > What are "some configurations of GCC"? If it can be reproduced with > what's released out there, maybe that should go in now, even for 5.11? > > Hmm? Agreed, stable is a good idea. I think Nikolay saw it with GCC 9. -- Josh
Re: [PATCH] x86: Disable CET instrumentation in the kernel
On Fri, Jan 29, 2021 at 09:10:34AM -0600, Josh Poimboeuf wrote: > Maybe eventually. But the enablement (actually enabling CET/CFI/etc) > happens in the arch code anyway, right? So it could be a per-arch > decision. Right. Ok, for this one, what about Cc: ? What are "some configurations of GCC"? If it can be reproduced with what's released out there, maybe that should go in now, even for 5.11? Hmm? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86: Disable CET instrumentation in the kernel
On Thu, Jan 28, 2021 at 03:52:19PM -0600, Josh Poimboeuf wrote: > > With retpolines disabled, some configurations of GCC will add Intel CET > instrumentation to the kernel by default. That breaks certain tracing > scenarios by adding a superfluous ENDBR64 instruction before the fentry > call, for functions which can be called indirectly. > > CET instrumentation isn't currently necessary in the kernel, as CET is > only supported in user space. Disable it unconditionally. > > Reported-by: Nikolay Borisov > Signed-off-by: Josh Poimboeuf > --- > Makefile | 6 -- > arch/x86/Makefile | 3 +++ > 2 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/Makefile b/Makefile > index e0af7a4a5598..51c2bf34142d 100644 > --- a/Makefile > +++ b/Makefile > @@ -948,12 +948,6 @@ KBUILD_CFLAGS += $(call > cc-option,-Werror=designated-init) > # change __FILE__ to the relative path from the srctree > KBUILD_CPPFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=) > > -# ensure -fcf-protection is disabled when using retpoline as it is > -# incompatible with -mindirect-branch=thunk-extern > -ifdef CONFIG_RETPOLINE > -KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none) > -endif > - Why is that even here, in the main Makefile if this cf-protection thing is x86-specific? Are we going to move it back there when some other arch gets CET or CET-like support? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86: Disable CET instrumentation in the kernel
On 28.01.21 г. 23:52 ч., Josh Poimboeuf wrote: > > With retpolines disabled, some configurations of GCC will add Intel CET > instrumentation to the kernel by default. That breaks certain tracing > scenarios by adding a superfluous ENDBR64 instruction before the fentry > call, for functions which can be called indirectly. > > CET instrumentation isn't currently necessary in the kernel, as CET is > only supported in user space. Disable it unconditionally. > > Reported-by: Nikolay Borisov > Signed-off-by: Josh Poimboeuf Tested-by: Nikolay Borisov Reviewed-by: Nikolay Borisov
[PATCH] x86: Disable CET instrumentation in the kernel
With retpolines disabled, some configurations of GCC will add Intel CET instrumentation to the kernel by default. That breaks certain tracing scenarios by adding a superfluous ENDBR64 instruction before the fentry call, for functions which can be called indirectly. CET instrumentation isn't currently necessary in the kernel, as CET is only supported in user space. Disable it unconditionally. Reported-by: Nikolay Borisov Signed-off-by: Josh Poimboeuf --- Makefile | 6 -- arch/x86/Makefile | 3 +++ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index e0af7a4a5598..51c2bf34142d 100644 --- a/Makefile +++ b/Makefile @@ -948,12 +948,6 @@ KBUILD_CFLAGS += $(call cc-option,-Werror=designated-init) # change __FILE__ to the relative path from the srctree KBUILD_CPPFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=) -# ensure -fcf-protection is disabled when using retpoline as it is -# incompatible with -mindirect-branch=thunk-extern -ifdef CONFIG_RETPOLINE -KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none) -endif - # include additional Makefiles when needed include-y := scripts/Makefile.extrawarn include-$(CONFIG_KASAN)+= scripts/Makefile.kasan diff --git a/arch/x86/Makefile b/arch/x86/Makefile index 32dcdddc1089..109c7f86483c 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -120,6 +120,9 @@ else KBUILD_CFLAGS += -mno-red-zone KBUILD_CFLAGS += -mcmodel=kernel + + # Intel CET isn't enabled in the kernel + KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none) endif ifdef CONFIG_X86_X32 -- 2.29.2
