Add missing module flags for CFI and KCFI sanitizers by samitolvanen · Pull Request #129373 · rust-lang/rust

samitolvanen · 2024-08-21T20:27:57Z

Set the cfi-normalize-integers and kcfi-offset module flags when Control-Flow Integrity sanitizers are used, so functions generated by the LLVM backend use the same CFI/KCFI options as rustc.

cfi-normalize-integers tells LLVM to also use integer normalization for generated functions when -Zsanitizer-cfi-normalize-integers is used.

kcfi-offset specifies the number of prefix nops between the KCFI type hash and the function entry when -Z patchable-function-entry is used. Note that LLVM assumes all indirectly callable functions use the same number of prefix NOPs with -Zsanitizer=kcfi.

Set the cfi-normalize-integers and kcfi-offset module flags when Control-Flow Integrity sanitizers are used, so functions generated by the LLVM backend use the same CFI/KCFI options as rustc. cfi-normalize-integers tells LLVM to also use integer normalization for generated functions when -Zsanitizer-cfi-normalize-integers is used. kcfi-offset specifies the number of prefix nops between the KCFI type hash and the function entry when -Z patchable-function-entry is used. Note that LLVM assumes all indirectly callable functions use the same number of prefix NOPs with -Zsanitizer=kcfi.

rustbot · 2024-08-21T20:28:05Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jieyouxu (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

rustbot · 2024-08-21T20:28:07Z

Some changes occurred in tests/codegen/sanitizer

cc @rust-lang/project-exploit-mitigations, @rcvalle

rcvalle · 2024-08-21T21:09:13Z

r? @compiler-errors

rcvalle · 2024-08-21T21:10:01Z

LGTM (@compiler-errors FYI). Thank you for your time and for working on this, @samitolvanen! Much appreciated.

rcvalle · 2024-08-21T21:59:51Z

@compiler-errors for context: llvm/llvm-project#104826

compiler-errors · 2024-08-22T02:23:14Z

@bors r+

bors · 2024-08-22T02:23:17Z

📌 Commit 40f1d9d has been approved by compiler-errors

It is now in the queue for this repository.

…ompiler-errors Add missing module flags for CFI and KCFI sanitizers Set the cfi-normalize-integers and kcfi-offset module flags when Control-Flow Integrity sanitizers are used, so functions generated by the LLVM backend use the same CFI/KCFI options as rustc. cfi-normalize-integers tells LLVM to also use integer normalization for generated functions when -Zsanitizer-cfi-normalize-integers is used. kcfi-offset specifies the number of prefix nops between the KCFI type hash and the function entry when -Z patchable-function-entry is used. Note that LLVM assumes all indirectly callable functions use the same number of prefix NOPs with -Zsanitizer=kcfi.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#128432 (WASI: forbid `unsafe_op_in_unsafe_fn` for `std::{os, sys}`) - rust-lang#129373 (Add missing module flags for CFI and KCFI sanitizers) - rust-lang#129374 (Use `assert_unsafe_precondition!` in `AsciiChar::digit_unchecked`) - rust-lang#129376 (Change `assert_unsafe_precondition` docs to refer to `check_language_ub`) - rust-lang#129382 (Add `const_cell_into_inner` to `OnceCell`) - rust-lang#129387 (Advise against removing the remaining Python scripts from `tests/run-make`) - rust-lang#129388 (Do not rely on names to find lifetimes.) - rust-lang#129395 (Pretty-print own args of existential projections (dyn-Trait w/ GAT constraints)) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#129373 - samitolvanen:cfi-module-flags, r=compiler-errors Add missing module flags for CFI and KCFI sanitizers Set the cfi-normalize-integers and kcfi-offset module flags when Control-Flow Integrity sanitizers are used, so functions generated by the LLVM backend use the same CFI/KCFI options as rustc. cfi-normalize-integers tells LLVM to also use integer normalization for generated functions when -Zsanitizer-cfi-normalize-integers is used. kcfi-offset specifies the number of prefix nops between the KCFI type hash and the function entry when -Z patchable-function-entry is used. Note that LLVM assumes all indirectly callable functions use the same number of prefix NOPs with -Zsanitizer=kcfi.

There is a bug in the LLVM implementation of KASAN and GCOV that makes these options incompatible with the CFI_ICALL_NORMALIZE_INTEGERS option. The bug has already been fixed in llvm/clang [1] and rustc [2]. However, Kconfig currently has no way to gate features on the LLVM version inside rustc, so we cannot write down a precise `depends on` clause in this case. Instead, a `def_bool` option is defined for whether CFI_ICALL_NORMALIZE_INTEGERS is available, and its default value is set to false when GCOV or KASAN are turned on. End users using a patched clang/rustc can turn on the HAVE_CFI_ICALL_NORMALIZE_INTEGERS option directly to override this. An alternative solution is to inspect a binary created by clang or rustc to see whether the faulty CFI tags are in the binary. This would be a precise check, but it would involve hard-coding the *hashed* version of the CFI tag. This is because there's no way to get clang or rustc to output the unhased version of the CFI tag. Relying on the precise hashing algorithm using by CFI seems too fragile, so I have not pursued this option. Besides, this kind of hack is exactly what lead to the LLVM bug in the first place. If the CFI_ICALL_NORMALIZE_INTEGERS option is used without CONFIG_RUST, then we actually can perform a precise check today: just compare the clang version number. This works since clang and llvm are always updated in lockstep. However, encoding this in Kconfig would give the HAVE_CFI_ICALL_NORMALIZE_INTEGERS option a dependency on CONFIG_RUST, which is not possible as the reverse dependency already exists. HAVE_CFI_ICALL_NORMALIZE_INTEGERS is defined to be a `def_bool` instead of `bool` to avoid asking end users whether they want to turn on the option. Turning it on explicitly is something only experts should do, so making it hard to do so is not an issue. I added a `depends on CFI_CLANG` clause to the new Kconfig option. I'm not sure whether that makes sense or not, but it doesn't seem to make a big difference. In a future kernel release, I would like to add a Kconfig option similar to CLANG_VERSION/RUSTC_VERSION for inspecting the version of the LLVM inside rustc. Once that feature lands, this logic will be replaced with a precise version check. This check is not being introduced here to avoid introducing a new _VERSION constant in a fix. Link: llvm/llvm-project#104826 [1] Link: rust-lang/rust#129373 [2] Fixes: ce4a262 ("cfi: add CONFIG_CFI_ICALL_NORMALIZE_INTEGERS") Reported-by: kernel test robot <[email protected]> Closes: https://lore.kernel.org/oe-lkp/[email protected] Signed-off-by: Alice Ryhl <[email protected]>

There is a bug in the LLVM implementation of KASAN and GCOV that makes these options incompatible with the CFI_ICALL_NORMALIZE_INTEGERS option. The bug has already been fixed in llvm/clang [1] and rustc [2]. However, Kconfig currently has no way to gate features on the LLVM version inside rustc, so we cannot write down a precise `depends on` clause in this case. Instead, a `def_bool` option is defined for whether CFI_ICALL_NORMALIZE_INTEGERS is available, and its default value is set to false when GCOV or KASAN are turned on. End users using a patched clang/rustc can turn on the HAVE_CFI_ICALL_NORMALIZE_INTEGERS option directly to override this. An alternative solution is to inspect a binary created by clang or rustc to see whether the faulty CFI tags are in the binary. This would be a precise check, but it would involve hard-coding the *hashed* version of the CFI tag. This is because there's no way to get clang or rustc to output the unhased version of the CFI tag. Relying on the precise hashing algorithm using by CFI seems too fragile, so I have not pursued this option. Besides, this kind of hack is exactly what lead to the LLVM bug in the first place. If the CFI_ICALL_NORMALIZE_INTEGERS option is used without CONFIG_RUST, then we actually can perform a precise check today: just compare the clang version number. This works since clang and llvm are always updated in lockstep. However, encoding this in Kconfig would give the HAVE_CFI_ICALL_NORMALIZE_INTEGERS option a dependency on CONFIG_RUST, which is not possible as the reverse dependency already exists. HAVE_CFI_ICALL_NORMALIZE_INTEGERS is defined to be a `def_bool` instead of `bool` to avoid asking end users whether they want to turn on the option. Turning it on explicitly is something only experts should do, so making it hard to do so is not an issue. I added a `depends on CFI_CLANG` clause to the new Kconfig option. I'm not sure whether that makes sense or not, but it doesn't seem to make a big difference. In a future kernel release, I would like to add a Kconfig option similar to CLANG_VERSION/RUSTC_VERSION for inspecting the version of the LLVM inside rustc. Once that feature lands, this logic will be replaced with a precise version check. This check is not being introduced here to avoid introducing a new _VERSION constant in a fix. Link: llvm/llvm-project#104826 [1] Link: rust-lang/rust#129373 [2] Fixes: ce4a262 ("cfi: add CONFIG_CFI_ICALL_NORMALIZE_INTEGERS") Reported-by: kernel test robot <[email protected]> Closes: https://lore.kernel.org/oe-lkp/[email protected] Signed-off-by: Alice Ryhl <[email protected]> Reviewed-by: Sami Tolvanen <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>

…sleywiser Add missing module flags for `-Zfunction-return=thunk-extern` This fixes a bug in the `-Zfunction-return=thunk-extern` flag. The flag needs to be passed onto LLVM to ensure that functions such as `asan.module_ctor` and `asan.module_dtor` that are created internally in LLVM have the mitigation applied to them. This was originally discovered [in the Linux kernel](https://lore.kernel.org/all/CANiq72myZL4_poCMuNFevtpYYc0V0embjSuKb7y=C+m3vVA_8g@mail.gmail.com/). Original flag PR: rust-lang#116892 PR for similar issue: rust-lang#129373 Tracking issue: rust-lang#116853 cc `@ojeda` r? `@wesleywiser`

…sleywiser Add missing module flags for `-Zfunction-return=thunk-extern` This fixes a bug in the `-Zfunction-return=thunk-extern` flag. The flag needs to be passed onto LLVM to ensure that functions such as `asan.module_ctor` and `asan.module_dtor` that are created internally in LLVM have the mitigation applied to them. This was originally discovered [in the Linux kernel](https://lore.kernel.org/all/CANiq72myZL4_poCMuNFevtpYYc0V0embjSuKb7y=C+m3vVA_8g@mail.gmail.com/). Original flag PR: rust-lang#116892 PR for similar issue: rust-lang#129373 Tracking issue: rust-lang#116853 cc ``@ojeda`` r? ``@wesleywiser``

Rollup merge of rust-lang#130824 - Darksonn:fix-function-return, r=wesleywiser Add missing module flags for `-Zfunction-return=thunk-extern` This fixes a bug in the `-Zfunction-return=thunk-extern` flag. The flag needs to be passed onto LLVM to ensure that functions such as `asan.module_ctor` and `asan.module_dtor` that are created internally in LLVM have the mitigation applied to them. This was originally discovered [in the Linux kernel](https://lore.kernel.org/all/CANiq72myZL4_poCMuNFevtpYYc0V0embjSuKb7y=C+m3vVA_8g@mail.gmail.com/). Original flag PR: rust-lang#116892 PR for similar issue: rust-lang#129373 Tracking issue: rust-lang#116853 cc ``@ojeda`` r? ``@wesleywiser``

The HAVE_CFI_ICALL_NORMALIZE_INTEGERS option has some tricky conditions when KASAN or GCOV are turned on, as in that case we need some clang and rustc fixes [1][2] to avoid boot failures. The intent with the current setup is that you should be able to override the check and turn on the option if your clang/rustc has the fix. However, this override does not work in practice. Thus, use the new RUSTC_LLVM_VERSION to correctly implement the check for whether the fix is available. Additionally, remove KASAN_HW_TAGS from the list of incompatible options. The CFI_ICALL_NORMALIZE_INTEGERS option is incompatible with KASAN because LLVM will emit some constructors when using KASAN that are assigned incorrect CFI tags. These constructors are emitted due to use of -fsanitize=kernel-address or -fsanitize=kernel-hwaddress that are respectively passed when KASAN_GENERIC or KASAN_SW_TAGS are enabled. However, the KASAN_HW_TAGS option relies on hardware support for MTE instead and does not pass either flag. (Note also that KASAN_HW_TAGS does not `select CONSTRUCTORS`.) Link: llvm/llvm-project#104826 [1] Link: rust-lang/rust#129373 [2] Fixes: 4c66f83 ("cfi: encode cfi normalized integers + kasan/gcov bug in Kconfig") Signed-off-by: Alice Ryhl <[email protected]>

The HAVE_CFI_ICALL_NORMALIZE_INTEGERS option has some tricky conditions when KASAN or GCOV are turned on, as in that case we need some clang and rustc fixes [1][2] to avoid boot failures. The intent with the current setup is that you should be able to override the check and turn on the option if your clang/rustc has the fix. However, this override does not work in practice. Thus, use the new RUSTC_LLVM_VERSION to correctly implement the check for whether the fix is available. Additionally, remove KASAN_HW_TAGS from the list of incompatible options. The CFI_ICALL_NORMALIZE_INTEGERS option is incompatible with KASAN because LLVM will emit some constructors when using KASAN that are assigned incorrect CFI tags. These constructors are emitted due to use of -fsanitize=kernel-address or -fsanitize=kernel-hwaddress that are respectively passed when KASAN_GENERIC or KASAN_SW_TAGS are enabled. However, the KASAN_HW_TAGS option relies on hardware support for MTE instead and does not pass either flag. (Note also that KASAN_HW_TAGS does not `select CONSTRUCTORS`.) Link: llvm/llvm-project#104826 [1] Link: rust-lang/rust#129373 [2] Fixes: 4c66f83 ("cfi: encode cfi normalized integers + kasan/gcov bug in Kconfig") Signed-off-by: Alice Ryhl <[email protected]> Reviewed-by: Sami Tolvanen <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>

The HAVE_CFI_ICALL_NORMALIZE_INTEGERS option has some tricky conditions when KASAN or GCOV are turned on, as in that case we need some clang and rustc fixes [1][2] to avoid boot failures. The intent with the current setup is that you should be able to override the check and turn on the option if your clang/rustc has the fix. However, this override does not work in practice. Thus, use the new RUSTC_LLVM_VERSION to correctly implement the check for whether the fix is available. Additionally, remove KASAN_HW_TAGS from the list of incompatible options. The CFI_ICALL_NORMALIZE_INTEGERS option is incompatible with KASAN because LLVM will emit some constructors when using KASAN that are assigned incorrect CFI tags. These constructors are emitted due to use of -fsanitize=kernel-address or -fsanitize=kernel-hwaddress that are respectively passed when KASAN_GENERIC or KASAN_SW_TAGS are enabled. However, the KASAN_HW_TAGS option relies on hardware support for MTE instead and does not pass either flag. (Note also that KASAN_HW_TAGS does not `select CONSTRUCTORS`.) Link: llvm/llvm-project#104826 [1] Link: rust-lang/rust#129373 [2] Fixes: 4c66f83 ("cfi: encode cfi normalized integers + kasan/gcov bug in Kconfig") Signed-off-by: Alice Ryhl <[email protected]> Reviewed-by: Sami Tolvanen <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]> Bug: 359429865 (cherry picked from commit 8b8ca9c https://github.com/Rust-for-Linux/linux.git rust-fixes) Change-Id: Iceed2dd26295ae90a797481481abbb6efcb1e8ea Signed-off-by: Alice Ryhl <[email protected]>

There is a bug in the LLVM implementation of KASAN and GCOV that makes these options incompatible with the CFI_ICALL_NORMALIZE_INTEGERS option. The bug has already been fixed in llvm/clang [1] and rustc [2]. However, Kconfig currently has no way to gate features on the LLVM version inside rustc, so we cannot write down a precise `depends on` clause in this case. Instead, a `def_bool` option is defined for whether CFI_ICALL_NORMALIZE_INTEGERS is available, and its default value is set to false when GCOV or KASAN are turned on. End users using a patched clang/rustc can turn on the HAVE_CFI_ICALL_NORMALIZE_INTEGERS option directly to override this. An alternative solution is to inspect a binary created by clang or rustc to see whether the faulty CFI tags are in the binary. This would be a precise check, but it would involve hard-coding the *hashed* version of the CFI tag. This is because there's no way to get clang or rustc to output the unhased version of the CFI tag. Relying on the precise hashing algorithm using by CFI seems too fragile, so I have not pursued this option. Besides, this kind of hack is exactly what lead to the LLVM bug in the first place. If the CFI_ICALL_NORMALIZE_INTEGERS option is used without CONFIG_RUST, then we actually can perform a precise check today: just compare the clang version number. This works since clang and llvm are always updated in lockstep. However, encoding this in Kconfig would give the HAVE_CFI_ICALL_NORMALIZE_INTEGERS option a dependency on CONFIG_RUST, which is not possible as the reverse dependency already exists. HAVE_CFI_ICALL_NORMALIZE_INTEGERS is defined to be a `def_bool` instead of `bool` to avoid asking end users whether they want to turn on the option. Turning it on explicitly is something only experts should do, so making it hard to do so is not an issue. I added a `depends on CFI_CLANG` clause to the new Kconfig option. I'm not sure whether that makes sense or not, but it doesn't seem to make a big difference. In a future kernel release, I would like to add a Kconfig option similar to CLANG_VERSION/RUSTC_VERSION for inspecting the version of the LLVM inside rustc. Once that feature lands, this logic will be replaced with a precise version check. This check is not being introduced here to avoid introducing a new _VERSION constant in a fix. Link: llvm/llvm-project#104826 [1] Link: rust-lang/rust#129373 [2] Fixes: ce4a262 ("cfi: add CONFIG_CFI_ICALL_NORMALIZE_INTEGERS") Reported-by: kernel test robot <[email protected]> Closes: https://lore.kernel.org/oe-lkp/[email protected] Signed-off-by: Alice Ryhl <[email protected]> Reviewed-by: Sami Tolvanen <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>

The HAVE_CFI_ICALL_NORMALIZE_INTEGERS option has some tricky conditions when KASAN or GCOV are turned on, as in that case we need some clang and rustc fixes [1][2] to avoid boot failures. The intent with the current setup is that you should be able to override the check and turn on the option if your clang/rustc has the fix. However, this override does not work in practice. Thus, use the new RUSTC_LLVM_VERSION to correctly implement the check for whether the fix is available. Additionally, remove KASAN_HW_TAGS from the list of incompatible options. The CFI_ICALL_NORMALIZE_INTEGERS option is incompatible with KASAN because LLVM will emit some constructors when using KASAN that are assigned incorrect CFI tags. These constructors are emitted due to use of -fsanitize=kernel-address or -fsanitize=kernel-hwaddress that are respectively passed when KASAN_GENERIC or KASAN_SW_TAGS are enabled. However, the KASAN_HW_TAGS option relies on hardware support for MTE instead and does not pass either flag. (Note also that KASAN_HW_TAGS does not `select CONSTRUCTORS`.) Link: llvm/llvm-project#104826 [1] Link: rust-lang/rust#129373 [2] Fixes: 4c66f83 ("cfi: encode cfi normalized integers + kasan/gcov bug in Kconfig") Signed-off-by: Alice Ryhl <[email protected]> Reviewed-by: Sami Tolvanen <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>

rustbot assigned jieyouxu Aug 21, 2024

rustbot added PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 21, 2024

rustbot assigned compiler-errors and unassigned jieyouxu Aug 21, 2024

compiler-errors approved these changes Aug 22, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 22, 2024

jieyouxu mentioned this pull request Aug 22, 2024

Rollup of 8 pull requests #129393

Closed

matthiaskrgr mentioned this pull request Aug 22, 2024

Rollup of 8 pull requests #129398

Merged

bors merged commit 8f2c4d1 into rust-lang:master Aug 22, 2024

rustbot added this to the 1.82.0 milestone Aug 22, 2024

Darksonn mentioned this pull request Sep 25, 2024

Add missing module flags for -Zfunction-return=thunk-extern #130824

Merged

ojeda mentioned this pull request Sep 25, 2024

Rust unstable features needed for the kernel Rust-for-Linux/linux#2

Open

46 tasks

rcvalle mentioned this pull request Mar 13, 2025

KCFI: Add KCFI arity indicator support #138368

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

Add missing module flags for CFI and KCFI sanitizers#129373

Add missing module flags for CFI and KCFI sanitizers#129373
bors merged 1 commit intorust-lang:masterfrom
samitolvanen:cfi-module-flags

samitolvanen commented Aug 21, 2024

Uh oh!

rustbot commented Aug 21, 2024

Uh oh!

rustbot commented Aug 21, 2024

Uh oh!

rcvalle commented Aug 21, 2024

Uh oh!

rcvalle commented Aug 21, 2024

Uh oh!

rcvalle commented Aug 21, 2024

Uh oh!

compiler-errors commented Aug 22, 2024

Uh oh!

bors commented Aug 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Comments

Conversation

samitolvanen commented Aug 21, 2024

Uh oh!

rustbot commented Aug 21, 2024

Uh oh!

rustbot commented Aug 21, 2024

Uh oh!

rcvalle commented Aug 21, 2024

Uh oh!

rcvalle commented Aug 21, 2024

Uh oh!

rcvalle commented Aug 21, 2024

Uh oh!

compiler-errors commented Aug 22, 2024

Uh oh!

bors commented Aug 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants