| Message ID | 20210128215219.6kct3h2eiustncws@treble (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | x86: Disable CET instrumentation in the kernel | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
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 <nborisov@suse.com> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Tested-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
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 <nborisov@suse.com> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > --- > 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?
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: <stable@vger.kernel.org> ? 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?
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: <stable@vger.kernel.org> > > ? > > 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.
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.
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
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.
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 <seth.forshee@canonical.com>
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
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 <nborisov@suse.com> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- Makefile | 6 ------ arch/x86/Makefile | 3 +++ 2 files changed, 3 insertions(+), 6 deletions(-)