These flags are not made conditional on compiler support because at the
moment exactly one version of rustc supported, and that one supports
these flags.
Building without these additional flags will manifest as objtool
printing a large number of errors about missing ENDBR and if CFI is
enabled (not currently possible) will result in incorrectly structured
function prefixes.
Signed-off-by: Matthew Maurer <[email protected]>
---
Split out the IBT additions as per
https://lkml.kernel.org/linux-fsdevel/CANiq72kK6ppBE7j=z7uua1cJMKaLoR5U3NUAZXT5MrNEs9ZhfQ@mail.gmail.com/
arch/x86/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 5bfe5caaa444..941f7abf6dbf 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -81,6 +81,7 @@ ifeq ($(CONFIG_X86_KERNEL_IBT),y)
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816
#
KBUILD_CFLAGS += $(call cc-option,-fcf-protection=branch -fno-jump-tables)
+KBUILD_RUSTFLAGS += -Zcf-protection=branch -Zno-jump-tables
else
KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none)
endif
--
2.42.0.609.gbb76f46606-goog
On Mon, Oct 09, 2023 at 10:42:54PM +0000, Matthew Maurer wrote:
> These flags are not made conditional on compiler support because at the
> moment exactly one version of rustc supported, and that one supports
> these flags.
>
> Building without these additional flags will manifest as objtool
> printing a large number of errors about missing ENDBR and if CFI is
> enabled (not currently possible) will result in incorrectly structured
> function prefixes.
Well, I would also imagine running it on actual IBT enabled hardware
will get you a non-booting kernel.
> Signed-off-by: Matthew Maurer <[email protected]>
> ---
>
> Split out the IBT additions as per
> https://lkml.kernel.org/linux-fsdevel/CANiq72kK6ppBE7j=z7uua1cJMKaLoR5U3NUAZXT5MrNEs9ZhfQ@mail.gmail.com/
>
> arch/x86/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 5bfe5caaa444..941f7abf6dbf 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -81,6 +81,7 @@ ifeq ($(CONFIG_X86_KERNEL_IBT),y)
> # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816
> #
> KBUILD_CFLAGS += $(call cc-option,-fcf-protection=branch -fno-jump-tables)
> +KBUILD_RUSTFLAGS += -Zcf-protection=branch -Zno-jump-tables
One question, -Zcf-protection=branch, will that ever emit NOTRACK
prefix? The kernel very explicitly does not support (enable) NOTRACK.
> else
> KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none)
> endif
> --
> 2.42.0.609.gbb76f46606-goog
>
On Tue, Oct 10, 2023 at 1:12 AM Peter Zijlstra <[email protected]> wrote:
>
> On Mon, Oct 09, 2023 at 10:42:54PM +0000, Matthew Maurer wrote:
> > These flags are not made conditional on compiler support because at the
> > moment exactly one version of rustc supported, and that one supports
> > these flags.
> >
> > Building without these additional flags will manifest as objtool
> > printing a large number of errors about missing ENDBR and if CFI is
> > enabled (not currently possible) will result in incorrectly structured
> > function prefixes.
>
> Well, I would also imagine running it on actual IBT enabled hardware
> will get you a non-booting kernel.
>
> > Signed-off-by: Matthew Maurer <[email protected]>
> > ---
> >
> > Split out the IBT additions as per
> > https://lkml.kernel.org/linux-fsdevel/CANiq72kK6ppBE7j=z7uua1cJMKaLoR5U3NUAZXT5MrNEs9ZhfQ@mail.gmail.com/
> >
> > arch/x86/Makefile | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > index 5bfe5caaa444..941f7abf6dbf 100644
> > --- a/arch/x86/Makefile
> > +++ b/arch/x86/Makefile
> > @@ -81,6 +81,7 @@ ifeq ($(CONFIG_X86_KERNEL_IBT),y)
> > # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816
> > #
> > KBUILD_CFLAGS += $(call cc-option,-fcf-protection=branch -fno-jump-tables)
> > +KBUILD_RUSTFLAGS += -Zcf-protection=branch -Zno-jump-tables
>
> One question, -Zcf-protection=branch, will that ever emit NOTRACK
> prefix? The kernel very explicitly does not support (enable) NOTRACK.
rustc does this via LLVM, so its code generation works very similarly to clang.
It does not create its own explicit NOTRACKs, but LLVM will by default
with just -Zcf-protection-branch.
I've linked a godbolt showing that at least for the basic case, your
no-jump-tables approach from clang ports over.
https://godbolt.org/z/bc4n6sq5q
Whether rust generates NOTRACK should end up being roughly equivalent
to whether clang generates it, and if LLVM gains a code generation
flag for NOTRACK being disallowed some day, we can pass that through
as well.
>
> > else
> > KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none)
> > endif
> > --
> > 2.42.0.609.gbb76f46606-goog
> >
On Tue, Oct 10, 2023 at 07:06:32AM -0700, Matthew Maurer wrote:
> > > +KBUILD_RUSTFLAGS += -Zcf-protection=branch -Zno-jump-tables
> >
> > One question, -Zcf-protection=branch, will that ever emit NOTRACK
> > prefix? The kernel very explicitly does not support (enable) NOTRACK.
> rustc does this via LLVM, so its code generation works very similarly to clang.
> It does not create its own explicit NOTRACKs, but LLVM will by default
> with just -Zcf-protection-branch.
> I've linked a godbolt showing that at least for the basic case, your
> no-jump-tables approach from clang ports over.
> https://godbolt.org/z/bc4n6sq5q
> Whether rust generates NOTRACK should end up being roughly equivalent
> to whether clang generates it, and if LLVM gains a code generation
> flag for NOTRACK being disallowed some day, we can pass that through
> as well.
IIRC C++ will also emit NOTRACK for things like catch/throw and other
stack/scope unwinds. Obviously C doesn't have that, but does Rust? (as
might be obvious, I *really* don't know the language).
ISTR HJL had a GCC patch to force-disable NOTRACK, but I've no idea what
happened to that.
On Tue, Oct 10, 2023 at 7:24 AM Peter Zijlstra <[email protected]> wrote:
>
> On Tue, Oct 10, 2023 at 07:06:32AM -0700, Matthew Maurer wrote:
>
> > > > +KBUILD_RUSTFLAGS += -Zcf-protection=branch -Zno-jump-tables
> > >
> > > One question, -Zcf-protection=branch, will that ever emit NOTRACK
> > > prefix? The kernel very explicitly does not support (enable) NOTRACK.
>
> > rustc does this via LLVM, so its code generation works very similarly to clang.
> > It does not create its own explicit NOTRACKs, but LLVM will by default
> > with just -Zcf-protection-branch.
> > I've linked a godbolt showing that at least for the basic case, your
> > no-jump-tables approach from clang ports over.
> > https://godbolt.org/z/bc4n6sq5q
> > Whether rust generates NOTRACK should end up being roughly equivalent
> > to whether clang generates it, and if LLVM gains a code generation
> > flag for NOTRACK being disallowed some day, we can pass that through
> > as well.
>
> IIRC C++ will also emit NOTRACK for things like catch/throw and other
> stack/scope unwinds. Obviously C doesn't have that, but does Rust? (as
> might be obvious, I *really* don't know the language).
>
That's fine - Rust does have stack/scope unwinds with the
`panic=unwind` strategy. In the kernel, we use `panic=abort` and are
unlikely to ever change this approach. There are a host of other
complications that come from unwinding without NOTRACK getting
involved :)
In case you find `catch_unwind` - this function only has an effect
with `panic=unwind`. When `panic=abort`, there's nothing analogous to
catch/throw anymore, and `catch_unwind` becomes a no-op.
Are there other features you expect might trigger NOTRACK?
> ISTR HJL had a GCC patch to force-disable NOTRACK, but I've no idea what
> happened to that.
>
On Tue, Oct 10, 2023 at 07:34:45AM -0700, Matthew Maurer wrote:
> On Tue, Oct 10, 2023 at 7:24 AM Peter Zijlstra <[email protected]> wrote:
> >
> > On Tue, Oct 10, 2023 at 07:06:32AM -0700, Matthew Maurer wrote:
> >
> > > > > +KBUILD_RUSTFLAGS += -Zcf-protection=branch -Zno-jump-tables
> > > >
> > > > One question, -Zcf-protection=branch, will that ever emit NOTRACK
> > > > prefix? The kernel very explicitly does not support (enable) NOTRACK.
> >
> > > rustc does this via LLVM, so its code generation works very similarly to clang.
> > > It does not create its own explicit NOTRACKs, but LLVM will by default
> > > with just -Zcf-protection-branch.
> > > I've linked a godbolt showing that at least for the basic case, your
> > > no-jump-tables approach from clang ports over.
> > > https://godbolt.org/z/bc4n6sq5q
> > > Whether rust generates NOTRACK should end up being roughly equivalent
> > > to whether clang generates it, and if LLVM gains a code generation
> > > flag for NOTRACK being disallowed some day, we can pass that through
> > > as well.
> >
> > IIRC C++ will also emit NOTRACK for things like catch/throw and other
> > stack/scope unwinds. Obviously C doesn't have that, but does Rust? (as
> > might be obvious, I *really* don't know the language).
> >
> That's fine - Rust does have stack/scope unwinds with the
> `panic=unwind` strategy. In the kernel, we use `panic=abort` and are
> unlikely to ever change this approach. There are a host of other
> complications that come from unwinding without NOTRACK getting
> involved :)
>
> In case you find `catch_unwind` - this function only has an effect
> with `panic=unwind`. When `panic=abort`, there's nothing analogous to
> catch/throw anymore, and `catch_unwind` becomes a no-op.
>
> Are there other features you expect might trigger NOTRACK?
I'm not sure -- if they happen, objtool should warn about them. So I
suppose we'll take it from there.
Acked-by: Peter Zijlstra (Intel) <[email protected]>
On Tue, Oct 10, 2023 at 1:13 AM Peter Zijlstra <[email protected]> wrote:
>
> On Mon, Oct 09, 2023 at 10:42:54PM +0000, Matthew Maurer wrote:
> > These flags are not made conditional on compiler support because at the
> > moment exactly one version of rustc supported, and that one supports
> > these flags.
> >
> > Building without these additional flags will manifest as objtool
> > printing a large number of errors about missing ENDBR and if CFI is
> > enabled (not currently possible) will result in incorrectly structured
> > function prefixes.
>
> Well, I would also imagine running it on actual IBT enabled hardware
> will get you a non-booting kernel.
Do you know what machine type in QEMU first supports IBT?
>
> > Signed-off-by: Matthew Maurer <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
> > https://godbolt.org/z/bc4n6sq5q
Intel asm syntax...my eyes!!!
> > ---
> >
> > Split out the IBT additions as per
> > https://lkml.kernel.org/linux-fsdevel/CANiq72kK6ppBE7j=z7uua1cJMKaLoR5U3NUAZXT5MrNEs9ZhfQ@mail.gmail.com/
> >
> > arch/x86/Makefile | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > index 5bfe5caaa444..941f7abf6dbf 100644
> > --- a/arch/x86/Makefile
> > +++ b/arch/x86/Makefile
> > @@ -81,6 +81,7 @@ ifeq ($(CONFIG_X86_KERNEL_IBT),y)
> > # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816
> > #
> > KBUILD_CFLAGS += $(call cc-option,-fcf-protection=branch -fno-jump-tables)
> > +KBUILD_RUSTFLAGS += -Zcf-protection=branch -Zno-jump-tables
>
> One question, -Zcf-protection=branch, will that ever emit NOTRACK
> prefix? The kernel very explicitly does not support (enable) NOTRACK.
>
> > else
> > KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none)
> > endif
> > --
> > 2.42.0.609.gbb76f46606-goog
> >
>
--
Thanks,
~Nick Desaulniers
On Tue, Oct 10, 2023 at 08:38:58AM -0700, Nick Desaulniers wrote:
> On Tue, Oct 10, 2023 at 1:13 AM Peter Zijlstra <[email protected]> wrote:
> >
> > On Mon, Oct 09, 2023 at 10:42:54PM +0000, Matthew Maurer wrote:
> > > These flags are not made conditional on compiler support because at the
> > > moment exactly one version of rustc supported, and that one supports
> > > these flags.
> > >
> > > Building without these additional flags will manifest as objtool
> > > printing a large number of errors about missing ENDBR and if CFI is
> > > enabled (not currently possible) will result in incorrectly structured
> > > function prefixes.
> >
> > Well, I would also imagine running it on actual IBT enabled hardware
> > will get you a non-booting kernel.
>
> Do you know what machine type in QEMU first supports IBT?
I'm not sure QEMU has IBT support at all atm -- the whole CET
virtualization stuff is a bit of a mess.
But hardware wise it's Tigerlake (11) and everything after that - so
Alderlake (12) and Saphire Rapids (4th gen scalable -- or somesuch
nonsense).
On Tue, Oct 10, 2023 at 11:39 AM Nick Desaulniers
<[email protected]> wrote:
>
> > > https://godbolt.org/z/bc4n6sq5q
>
> Intel asm syntax...my eyes!!!
:) If it helps, you can click on the "output" (gear) dropdown in any
of the asm panels and toggle between Intel and AT&T
On Tue, Oct 10, 2023 at 12:43 AM Matthew Maurer <[email protected]> wrote:
>
> These flags are not made conditional on compiler support because at the
> moment exactly one version of rustc supported, and that one supports
> these flags.
>
> Building without these additional flags will manifest as objtool
> printing a large number of errors about missing ENDBR and if CFI is
> enabled (not currently possible) will result in incorrectly structured
> function prefixes.
>
> Signed-off-by: Matthew Maurer <[email protected]>
> ---
>
> Split out the IBT additions as per
> https://lkml.kernel.org/linux-fsdevel/CANiq72kK6ppBE7j=z7uua1cJMKaLoR5U3NUAZXT5MrNEs9ZhfQ@mail.gmail.com/
Thanks a lot Matthew for this! It is great to see those warnings
finally go away.
I have added the `objtool` pass to the intermediate Rust object files
and, with that + this patch applied + IBT enabled (but not
rethunk/retpoline), the only thing I see is:
samples/rust/rust_print.o: warning: objtool: init_module(): not an
indirect call target
samples/rust/rust_print.o: warning: objtool: cleanup_module(): not
an indirect call target
But we can fix those independently of this (ideally we want to reuse
the C macros, rather than putting more complexity in `module!`), so:
Acked-by: Miguel Ojeda <[email protected]>
I will send the patch for adding `objtool`.
Cheers,
Miguel
On Mon, Oct 9, 2023 at 6:44 PM Matthew Maurer <[email protected]> wrote:
> +KBUILD_RUSTFLAGS += -Zcf-protection=branch -Zno-jump-tables
I have not tested this, but is it possible to enable these options via
`-Cllvm-args=...` instead of using the unstable flags?
If so, I think this would be preferred in case the exact flags change
before they become stable. It sounds like they are likely to change,
see [1].
If not, no big deal since it would just need an update at a rust version bump.
- Trevor
[1]: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.60-Zbranch-protection.60.20stability
On Thu, Oct 12, 2023 at 10:13 PM Trevor Gross <[email protected]> wrote:
>
> I have not tested this, but is it possible to enable these options via
> `-Cllvm-args=...` instead of using the unstable flags?
We probably want to use the "real" flag eventually instead of
`-Cllvm-args`, right? So we would need to change it anyhow. And using
the `-Z` one means we test the "real" flag already.
Well, unless `-Cllvm-args` becomes the "official" way to enable this,
like you suggest in the Zulip, but should that really happen? e.g.
should not there be a generic flag for all backends for things like
these?
> If so, I think this would be preferred in case the exact flags change
> before they become stable. It sounds like they are likely to change,
> see [1].
That is fine, they will change anyway from `-Z` to `-C`, so having to
update those is expected.
> If not, no big deal since it would just need an update at a rust version bump.
Yeah, I don't think it is a big deal, and the version bump looks like
the best commit to put the change, in fact.
It is true, though, that these ones in particular are conditionally
enabled, so there is a slightly higher risk of forgetting about them.
But that is why we should get more `Tested-by`s! :)
Cheers,
Miguel
On Tue, Oct 10, 2023 at 4:56 PM Peter Zijlstra <[email protected]> wrote:
>
> I'm not sure -- if they happen, objtool should warn about them. So I
> suppose we'll take it from there.
>
> Acked-by: Peter Zijlstra (Intel) <[email protected]>
Applied to `rust-next` -- thanks!
For subsequent patches, do you want that they go through the tip tree?
If so, please let me know, I think it would be ideal.
Thanks!
Cheers,
Miguel