Add flags to support the shadow call stack sanitizer, both in the
dynamic and non-dynamic modes.
Right now, the compiler will emit the warning "unknown feature specified
for `-Ctarget-feature`: `reserve-x18`". However, the compiler still
passes it to the codegen backend, so the flag will work just fine. Once
rustc starts recognizing the flag (or provides another way to enable the
feature), it will stop emitting this warning. See [1] for the relevant
issue.
Currently, the compiler thinks that the aarch64-unknown-none target
doesn't support -Zsanitizer=shadow-call-stack, so the build will fail if
you enable shadow call stack in non-dynamic mode. However, I still think
it is reasonable to add the flag now, as it will at least fail the build
when using an invalid configuration, until the Rust compiler is fixed to
list -Zsanitizer=shadow-call-stack as supported for the target. See [2]
for the feature request to add this.
I have tested this change with Rust Binder on an Android device using
CONFIG_DYNAMIC_SCS. Without the -Ctarget-feature=+reserve-x18 flag, the
phone crashes immediately on boot, and with the flag, the phone appears
to work normally.
Link: https://github.com/rust-lang/rust/issues/121970 [1]
Link: https://github.com/rust-lang/rust/issues/121972 [2]
Signed-off-by: Alice Ryhl <[email protected]>
---
It's not 100% clear to me whether this patch is enough for full SCS
support in Rust. If there is some issue where this makes things compile
and work without actually applying SCS to the Rust code, please let me
know. Is there some way to verify that it is actually working?
This patch raises the question of whether we should change the Rust
aarch64 support to use a custom target.json specification. If we do
that, then we can fix both the warning for dynamic SCS and the
build-failure for non-dynamic SCS without waiting for a new version of
rustc with the mentioned issues fixed.
---
Makefile | 1 +
arch/arm64/Makefile | 1 +
2 files changed, 2 insertions(+)
diff --git a/Makefile b/Makefile
index 0e36eff14608..345066643a76 100644
--- a/Makefile
+++ b/Makefile
@@ -936,6 +936,7 @@ ifdef CONFIG_SHADOW_CALL_STACK
ifndef CONFIG_DYNAMIC_SCS
CC_FLAGS_SCS := -fsanitize=shadow-call-stack
KBUILD_CFLAGS += $(CC_FLAGS_SCS)
+KBUILD_RUSTFLAGS += -Zsanitizer=shadow-call-stack
endif
export CC_FLAGS_SCS
endif
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index a88cdf910687..df3f21370165 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -103,6 +103,7 @@ endif
ifeq ($(CONFIG_SHADOW_CALL_STACK), y)
KBUILD_CFLAGS += -ffixed-x18
+KBUILD_RUSTFLAGS += -Ctarget-feature=+reserve-x18
endif
ifeq ($(CONFIG_CPU_BIG_ENDIAN), y)
---
base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72
change-id: 20240304-shadow-call-stack-9c197a4361d9
Best regards,
--
Alice Ryhl <[email protected]>
On Mon, Mar 4, 2024 at 5:17 AM Alice Ryhl <[email protected]> wrote:
>
> Add flags to support the shadow call stack sanitizer, both in the
> dynamic and non-dynamic modes.
>
> Right now, the compiler will emit the warning "unknown feature specified
> for `-Ctarget-feature`: `reserve-x18`". However, the compiler still
> passes it to the codegen backend, so the flag will work just fine. Once
> rustc starts recognizing the flag (or provides another way to enable the
> feature), it will stop emitting this warning. See [1] for the relevant
> issue.
>
> Currently, the compiler thinks that the aarch64-unknown-none target
> doesn't support -Zsanitizer=shadow-call-stack, so the build will fail if
> you enable shadow call stack in non-dynamic mode. However, I still think
> it is reasonable to add the flag now, as it will at least fail the build
> when using an invalid configuration, until the Rust compiler is fixed to
> list -Zsanitizer=shadow-call-stack as supported for the target. See [2]
> for the feature request to add this.
>
> I have tested this change with Rust Binder on an Android device using
> CONFIG_DYNAMIC_SCS. Without the -Ctarget-feature=+reserve-x18 flag, the
> phone crashes immediately on boot, and with the flag, the phone appears
> to work normally.
>
> Link: https://github.com/rust-lang/rust/issues/121970 [1]
> Link: https://github.com/rust-lang/rust/issues/121972 [2]
> Signed-off-by: Alice Ryhl <[email protected]>
> ---
> It's not 100% clear to me whether this patch is enough for full SCS
> support in Rust. If there is some issue where this makes things compile
> and work without actually applying SCS to the Rust code, please let me
> know. Is there some way to verify that it is actually working?
Perhaps you could write a Rust version of the CFI_BACKWARD test in LKDTM?
Alternatively, the simplest way to verify this is to look at the
disassembly and verify that shadow stack instructions are emitted to
Rust functions too. In case of dynamic SCS, you might need to dump
function memory in a debugger to verify that PAC instructions were
patched correctly. If they're not, the code will just quietly continue
working without using shadow stacks.
> This patch raises the question of whether we should change the Rust
> aarch64 support to use a custom target.json specification. If we do
> that, then we can fix both the warning for dynamic SCS and the
> build-failure for non-dynamic SCS without waiting for a new version of
> rustc with the mentioned issues fixed.
Sure, having a custom target description for the kernel might be
useful for other purposes too. In the meantime:
Reviewed-by: Sami Tolvanen <[email protected]>
Sami
> >
> > Add flags to support the shadow call stack sanitizer, both in the
> > dynamic and non-dynamic modes.
> >
> > Right now, the compiler will emit the warning "unknown feature specified
> > for `-Ctarget-feature`: `reserve-x18`". However, the compiler still
> > passes it to the codegen backend, so the flag will work just fine. Once
> > rustc starts recognizing the flag (or provides another way to enable the
> > feature), it will stop emitting this warning. See [1] for the relevant
> > issue.
> >
> > Currently, the compiler thinks that the aarch64-unknown-none target
> > doesn't support -Zsanitizer=shadow-call-stack, so the build will fail if
> > you enable shadow call stack in non-dynamic mode. However, I still think
> > it is reasonable to add the flag now, as it will at least fail the build
> > when using an invalid configuration, until the Rust compiler is fixed to
> > list -Zsanitizer=shadow-call-stack as supported for the target. See [2]
> > for the feature request to add this.
> >
> > I have tested this change with Rust Binder on an Android device using
> > CONFIG_DYNAMIC_SCS. Without the -Ctarget-feature=+reserve-x18 flag, the
> > phone crashes immediately on boot, and with the flag, the phone appears
> > to work normally.
> >
> > Link: https://github.com/rust-lang/rust/issues/121970 [1]
> > Link: https://github.com/rust-lang/rust/issues/121972 [2]
> > Signed-off-by: Alice Ryhl <[email protected]>
> > ---
> > It's not 100% clear to me whether this patch is enough for full SCS
> > support in Rust. If there is some issue where this makes things compile
> > and work without actually applying SCS to the Rust code, please let me
> > know. Is there some way to verify that it is actually working?
>
> Perhaps you could write a Rust version of the CFI_BACKWARD test in LKDTM?
>
> Alternatively, the simplest way to verify this is to look at the
> disassembly and verify that shadow stack instructions are emitted to
> Rust functions too. In case of dynamic SCS, you might need to dump
> function memory in a debugger to verify that PAC instructions were
> patched correctly. If they're not, the code will just quietly continue
> working without using shadow stacks.
Was just in the process of doing that:
- `paciasp`/`autiasp` pairs are emitted for functions in Rust modules.
- Rust modules have no `.init.eh_frame` section, which implies that
`module_finalize` is _not_ rewriting the pac insns when SCS is dynamic.
- Confirmed that behavior in the debugger (C modules and the C part of the
kernel are correctly rewritten, Rust modules execute with
`paciasp`/`autiasp` still in place).
- Kernel boots just fine with Rust kunit tests, tested with and without dynamic
SCS, i.e., on a CPU that supports PAC/BTI and one that does not.
- Rust sample modules load and unload without problems as well.
- `x18` is indeed not used in the codegen.
I guess we might be able to get this working when we tweak the build system
to emit the missing section for Rust modules.
- Best Valentin
>
> > This patch raises the question of whether we should change the Rust
> > aarch64 support to use a custom target.json specification. If we do
> > that, then we can fix both the warning for dynamic SCS and the
> > build-failure for non-dynamic SCS without waiting for a new version of
> > rustc with the mentioned issues fixed.
>
> Sure, having a custom target description for the kernel might be
> useful for other purposes too. In the meantime:
>
> Reviewed-by: Sami Tolvanen <[email protected]>
>
> Sami
>
>
On Tue, Mar 5, 2024 at 12:32 AM Valentin Obst <[email protected]> wrote:
>
> > >
> > > Add flags to support the shadow call stack sanitizer, both in the
> > > dynamic and non-dynamic modes.
> > >
> > > Right now, the compiler will emit the warning "unknown feature specified
> > > for `-Ctarget-feature`: `reserve-x18`". However, the compiler still
> > > passes it to the codegen backend, so the flag will work just fine. Once
> > > rustc starts recognizing the flag (or provides another way to enable the
> > > feature), it will stop emitting this warning. See [1] for the relevant
> > > issue.
> > >
> > > Currently, the compiler thinks that the aarch64-unknown-none target
> > > doesn't support -Zsanitizer=shadow-call-stack, so the build will fail if
> > > you enable shadow call stack in non-dynamic mode. However, I still think
> > > it is reasonable to add the flag now, as it will at least fail the build
> > > when using an invalid configuration, until the Rust compiler is fixed to
> > > list -Zsanitizer=shadow-call-stack as supported for the target. See [2]
> > > for the feature request to add this.
> > >
> > > I have tested this change with Rust Binder on an Android device using
> > > CONFIG_DYNAMIC_SCS. Without the -Ctarget-feature=+reserve-x18 flag, the
> > > phone crashes immediately on boot, and with the flag, the phone appears
> > > to work normally.
> > >
> > > Link: https://github.com/rust-lang/rust/issues/121970 [1]
> > > Link: https://github.com/rust-lang/rust/issues/121972 [2]
> > > Signed-off-by: Alice Ryhl <[email protected]>
> > > ---
> > > It's not 100% clear to me whether this patch is enough for full SCS
> > > support in Rust. If there is some issue where this makes things compile
> > > and work without actually applying SCS to the Rust code, please let me
> > > know. Is there some way to verify that it is actually working?
> >
> > Perhaps you could write a Rust version of the CFI_BACKWARD test in LKDTM?
> >
> > Alternatively, the simplest way to verify this is to look at the
> > disassembly and verify that shadow stack instructions are emitted to
> > Rust functions too. In case of dynamic SCS, you might need to dump
> > function memory in a debugger to verify that PAC instructions were
> > patched correctly. If they're not, the code will just quietly continue
> > working without using shadow stacks.
>
> Was just in the process of doing that:
>
> - `paciasp`/`autiasp` pairs are emitted for functions in Rust modules.
> - Rust modules have no `.init.eh_frame` section, which implies that
> `module_finalize` is _not_ rewriting the pac insns when SCS is dynamic.
> - Confirmed that behavior in the debugger (C modules and the C part of the
> kernel are correctly rewritten, Rust modules execute with
> `paciasp`/`autiasp` still in place).
> - Kernel boots just fine with Rust kunit tests, tested with and without dynamic
> SCS, i.e., on a CPU that supports PAC/BTI and one that does not.
> - Rust sample modules load and unload without problems as well.
> - `x18` is indeed not used in the codegen.
>
> I guess we might be able to get this working when we tweak the build system
> to emit the missing section for Rust modules.
I suppose the -Cforce-unwind-tables=y flag will most likely do it.
There's also an use_sync_unwind option, but it defaults to no, so it
doesn't seem like we need to set it.
Alice
> >>> It's not 100% clear to me whether this patch is enough for full SCS
> >>> support in Rust. If there is some issue where this makes things compile
> >>> and work without actually applying SCS to the Rust code, please let me
> >>> know. Is there some way to verify that it is actually working?
> >>
> >> Perhaps you could write a Rust version of the CFI_BACKWARD test in LKDTM?
> >>
> >> Alternatively, the simplest way to verify this is to look at the
> >> disassembly and verify that shadow stack instructions are emitted to
> >> Rust functions too. In case of dynamic SCS, you might need to dump
> >> function memory in a debugger to verify that PAC instructions were
> >> patched correctly. If they're not, the code will just quietly continue
> >> working without using shadow stacks.
> >
> > Was just in the process of doing that:
> >
> > - `paciasp`/`autiasp` pairs are emitted for functions in Rust modules.
> > - Rust modules have no `.init.eh_frame` section, which implies that
> > `module_finalize` is _not_ rewriting the pac insns when SCS is dynamic.
> > - Confirmed that behavior in the debugger (C modules and the C part of the
> > kernel are correctly rewritten, Rust modules execute with
> > `paciasp`/`autiasp` still in place).
> > - Kernel boots just fine with Rust kunit tests, tested with and without dynamic
> > SCS, i.e., on a CPU that supports PAC/BTI and one that does not.
> > - Rust sample modules load and unload without problems as well.
> > - `x18` is indeed not used in the codegen.
> >
> > I guess we might be able to get this working when we tweak the build system
> > to emit the missing section for Rust modules.
>
> I suppose the -Cforce-unwind-tables=y flag will most likely do it.
Yes, enabling this means that `.eh_frame` sections, which are converted to
`.init.eh_frame` sections for loadable modules, are generated for Rust
objects.
Tested booting, kunit tests, sample modules (as builtin and loadable) for
both, dynamic SCS active and inactive. Backtraces on Rust panicks also look
normal.
Confirmed that in the debugger that builtin and external modules are
rewritten (or not rewritten if no dynamic SCS). Did not check that the
`eh_frame` sections are exhaustive, i.e., cover all `paciasp`/`autiasp`
pairs, only verified a few functions (in init text and normal text).
> There's also an use_sync_unwind option, but it defaults to no, so it
> doesn't seem like we need to set it.
Are those defaults stable or will we notice if they change? If not it might
make sense to set it explicitly anyways to avoid surprises in the future.
- Best Valentin
>
> Alice
>
>
On Tue, Mar 5, 2024 at 12:20 PM Valentin Obst <[email protected]> wrote:
>
> > >>> It's not 100% clear to me whether this patch is enough for full SCS
> > >>> support in Rust. If there is some issue where this makes things compile
> > >>> and work without actually applying SCS to the Rust code, please let me
> > >>> know. Is there some way to verify that it is actually working?
> > >>
> > >> Perhaps you could write a Rust version of the CFI_BACKWARD test in LKDTM?
> > >>
> > >> Alternatively, the simplest way to verify this is to look at the
> > >> disassembly and verify that shadow stack instructions are emitted to
> > >> Rust functions too. In case of dynamic SCS, you might need to dump
> > >> function memory in a debugger to verify that PAC instructions were
> > >> patched correctly. If they're not, the code will just quietly continue
> > >> working without using shadow stacks.
> > >
> > > Was just in the process of doing that:
> > >
> > > - `paciasp`/`autiasp` pairs are emitted for functions in Rust modules.
> > > - Rust modules have no `.init.eh_frame` section, which implies that
> > > `module_finalize` is _not_ rewriting the pac insns when SCS is dynamic.
> > > - Confirmed that behavior in the debugger (C modules and the C part of the
> > > kernel are correctly rewritten, Rust modules execute with
> > > `paciasp`/`autiasp` still in place).
> > > - Kernel boots just fine with Rust kunit tests, tested with and without dynamic
> > > SCS, i.e., on a CPU that supports PAC/BTI and one that does not.
> > > - Rust sample modules load and unload without problems as well.
> > > - `x18` is indeed not used in the codegen.
> > >
> > > I guess we might be able to get this working when we tweak the build system
> > > to emit the missing section for Rust modules.
> >
> > I suppose the -Cforce-unwind-tables=y flag will most likely do it.
>
> Yes, enabling this means that `.eh_frame` sections, which are converted to
> `.init.eh_frame` sections for loadable modules, are generated for Rust
> objects.
>
> Tested booting, kunit tests, sample modules (as builtin and loadable) for
> both, dynamic SCS active and inactive. Backtraces on Rust panicks also look
> normal.
>
> Confirmed that in the debugger that builtin and external modules are
> rewritten (or not rewritten if no dynamic SCS). Did not check that the
> `eh_frame` sections are exhaustive, i.e., cover all `paciasp`/`autiasp`
> pairs, only verified a few functions (in init text and normal text).
Thank you for checking that!
> > There's also an use_sync_unwind option, but it defaults to no, so it
> > doesn't seem like we need to set it.
>
> Are those defaults stable or will we notice if they change? If not it might
> make sense to set it explicitly anyways to avoid surprises in the future.
The flag itself is unstable, so I imagine that nothing is promised about it.
I tried it, but I wasn't actually able to find a way to set it. I can
see the flag in the rustc source code, but passing -Zuse-sync-unwind=n
results in "error: unknown unstable option: `use-sync-unwind`". Not
sure what the issue is.
Alice
On Tue, Mar 5, 2024 at 12:28 PM Alice Ryhl <[email protected]> wrote:
>
> On Tue, Mar 5, 2024 at 12:20 PM Valentin Obst <[email protected]> wrote:
> >
> > > >>> It's not 100% clear to me whether this patch is enough for full SCS
> > > >>> support in Rust. If there is some issue where this makes things compile
> > > >>> and work without actually applying SCS to the Rust code, please let me
> > > >>> know. Is there some way to verify that it is actually working?
> > > >>
> > > >> Perhaps you could write a Rust version of the CFI_BACKWARD test in LKDTM?
> > > >>
> > > >> Alternatively, the simplest way to verify this is to look at the
> > > >> disassembly and verify that shadow stack instructions are emitted to
> > > >> Rust functions too. In case of dynamic SCS, you might need to dump
> > > >> function memory in a debugger to verify that PAC instructions were
> > > >> patched correctly. If they're not, the code will just quietly continue
> > > >> working without using shadow stacks.
> > > >
> > > > Was just in the process of doing that:
> > > >
> > > > - `paciasp`/`autiasp` pairs are emitted for functions in Rust modules.
> > > > - Rust modules have no `.init.eh_frame` section, which implies that
> > > > `module_finalize` is _not_ rewriting the pac insns when SCS is dynamic.
> > > > - Confirmed that behavior in the debugger (C modules and the C part of the
> > > > kernel are correctly rewritten, Rust modules execute with
> > > > `paciasp`/`autiasp` still in place).
> > > > - Kernel boots just fine with Rust kunit tests, tested with and without dynamic
> > > > SCS, i.e., on a CPU that supports PAC/BTI and one that does not.
> > > > - Rust sample modules load and unload without problems as well.
> > > > - `x18` is indeed not used in the codegen.
> > > >
> > > > I guess we might be able to get this working when we tweak the build system
> > > > to emit the missing section for Rust modules.
> > >
> > > I suppose the -Cforce-unwind-tables=y flag will most likely do it.
> >
> > Yes, enabling this means that `.eh_frame` sections, which are converted to
> > `.init.eh_frame` sections for loadable modules, are generated for Rust
> > objects.
> >
> > Tested booting, kunit tests, sample modules (as builtin and loadable) for
> > both, dynamic SCS active and inactive. Backtraces on Rust panicks also look
> > normal.
> >
> > Confirmed that in the debugger that builtin and external modules are
> > rewritten (or not rewritten if no dynamic SCS). Did not check that the
> > `eh_frame` sections are exhaustive, i.e., cover all `paciasp`/`autiasp`
> > pairs, only verified a few functions (in init text and normal text).
>
> Thank you for checking that!
>
> > > There's also an use_sync_unwind option, but it defaults to no, so it
> > > doesn't seem like we need to set it.
> >
> > Are those defaults stable or will we notice if they change? If not it might
> > make sense to set it explicitly anyways to avoid surprises in the future.
>
> The flag itself is unstable, so I imagine that nothing is promised about it.
>
> I tried it, but I wasn't actually able to find a way to set it. I can
> see the flag in the rustc source code, but passing -Zuse-sync-unwind=n
> results in "error: unknown unstable option: `use-sync-unwind`". Not
> sure what the issue is.
Oh, I understand now. It's really recent and not in 1.73.0, which is
what I'm using in the Android build.
Alice
On Tue, Mar 5, 2024 at 12:31 PM Alice Ryhl <[email protected]> wrote:
>
> On Tue, Mar 5, 2024 at 12:28 PM Alice Ryhl <[email protected]> wrote:
> >
> > On Tue, Mar 5, 2024 at 12:20 PM Valentin Obst <[email protected]> wrote:
> > >
> > > > >>> It's not 100% clear to me whether this patch is enough for full SCS
> > > > >>> support in Rust. If there is some issue where this makes things compile
> > > > >>> and work without actually applying SCS to the Rust code, please let me
> > > > >>> know. Is there some way to verify that it is actually working?
> > > > >>
> > > > >> Perhaps you could write a Rust version of the CFI_BACKWARD test in LKDTM?
> > > > >>
> > > > >> Alternatively, the simplest way to verify this is to look at the
> > > > >> disassembly and verify that shadow stack instructions are emitted to
> > > > >> Rust functions too. In case of dynamic SCS, you might need to dump
> > > > >> function memory in a debugger to verify that PAC instructions were
> > > > >> patched correctly. If they're not, the code will just quietly continue
> > > > >> working without using shadow stacks.
> > > > >
> > > > > Was just in the process of doing that:
> > > > >
> > > > > - `paciasp`/`autiasp` pairs are emitted for functions in Rust modules.
> > > > > - Rust modules have no `.init.eh_frame` section, which implies that
> > > > > `module_finalize` is _not_ rewriting the pac insns when SCS is dynamic.
> > > > > - Confirmed that behavior in the debugger (C modules and the C part of the
> > > > > kernel are correctly rewritten, Rust modules execute with
> > > > > `paciasp`/`autiasp` still in place).
> > > > > - Kernel boots just fine with Rust kunit tests, tested with and without dynamic
> > > > > SCS, i.e., on a CPU that supports PAC/BTI and one that does not.
> > > > > - Rust sample modules load and unload without problems as well.
> > > > > - `x18` is indeed not used in the codegen.
> > > > >
> > > > > I guess we might be able to get this working when we tweak the build system
> > > > > to emit the missing section for Rust modules.
> > > >
> > > > I suppose the -Cforce-unwind-tables=y flag will most likely do it.
> > >
> > > Yes, enabling this means that `.eh_frame` sections, which are converted to
> > > `.init.eh_frame` sections for loadable modules, are generated for Rust
> > > objects.
> > >
> > > Tested booting, kunit tests, sample modules (as builtin and loadable) for
> > > both, dynamic SCS active and inactive. Backtraces on Rust panicks also look
> > > normal.
> > >
> > > Confirmed that in the debugger that builtin and external modules are
> > > rewritten (or not rewritten if no dynamic SCS). Did not check that the
> > > `eh_frame` sections are exhaustive, i.e., cover all `paciasp`/`autiasp`
> > > pairs, only verified a few functions (in init text and normal text).
> >
> > Thank you for checking that!
> >
> > > > There's also an use_sync_unwind option, but it defaults to no, so it
> > > > doesn't seem like we need to set it.
> > >
> > > Are those defaults stable or will we notice if they change? If not it might
> > > make sense to set it explicitly anyways to avoid surprises in the future.
> >
> > The flag itself is unstable, so I imagine that nothing is promised about it.
> >
> > I tried it, but I wasn't actually able to find a way to set it. I can
> > see the flag in the rustc source code, but passing -Zuse-sync-unwind=n
> > results in "error: unknown unstable option: `use-sync-unwind`". Not
> > sure what the issue is.
>
> Oh, I understand now. It's really recent and not in 1.73.0, which is
> what I'm using in the Android build.
Sent v2 with what we have learned:
https://lore.kernel.org/rust-for-linux/[email protected]/
Thanks!
On Mon, Mar 4, 2024 at 9:10 PM Sami Tolvanen <[email protected]> wrote:
>
> Sure, having a custom target description for the kernel might be
> useful for other purposes too. In the meantime:
Yeah, though to be clear: eventually the goal is to avoid using the
`target.json` since it will not be stable, i.e. the support we have
for `target.json` is good for having the maximum flexibility, but it
is only a workaround for the beginning.
So for each arch we need to get things stable in one way or another
that does not require a custom `target.json` (`-Ctarget-feature`s
where they make sense; otherwise "global target features"/target
options/flags or new builtin targets).
Cheers,
Miguel