Disable CFI for weakly linked syscalls by 1c3t3a · Pull Request #138002 · rust-lang/rust

1c3t3a · 2025-03-04T14:40:32Z

Currently, when enabling CFI via -Zsanitizer=cfi and executing e.g. std::sys::random::getrandom, we can observe a CFI violation. This is the case for all consumers of the std::sys::pal::weak::syscall macro, as it is defining weak functions which don't show up in LLVM IR metadata. CFI fails for all these functions.

Similar to other such cases in
#115199, this change stops emitting the CFI typecheck for consumers of the macro via the #[no_sanitize(cfi)] attribute.

r? @rcvalle

bjorn3 · 2025-03-04T16:19:52Z

There are a couple other places where the weak!() macro is used. Should those also get #[no_sanitize(cfi)]? And is there a way to do this automatically for every call of a weak function instead?

rcvalle · 2025-03-04T17:49:20Z

is there a way to do this automatically for every call of a weak function instead?

That would be the best approach for now if we could do this only for functions (i.e., call sites) that indirectly call weakly linked functions.

Noratrieb · 2025-03-04T18:20:42Z

Is there a reason weak symbols don't get LLVM metadata?

1c3t3a · 2025-03-05T15:46:41Z

There are a couple other places where the weak!() macro is used. Should those also get #[no_sanitize(cfi)]?

Yes! I updated the PR to all callsites.

And is there a way to do this automatically for every call of a weak function instead?

There is no easy way of marking this as no_sanitize as part of the weak! macro, as it's currently just creating a member that is callable and has #[linkage = "extern_weak")]. The callsite of the resulting function pointer is not controlled by the macro. I think there are two alternatives to this if we don't want to manually go to the callsites (like this PR does it):

Don't emit the CFI type check at all for functions that are "extern weak". CFI would always be expected to fail in these cases.
Update the no_sanitize attribute to work well with #[linkage = "extern_weak")] statics.

But I am not very familiar with this, so I am happy to learn about a good alternative!

rcvalle · 2025-03-07T18:46:09Z

Is there a reason weak symbols don't get LLVM metadata?

I don't know. I think this is likely a bug and that the compiler should emit metadata for those. If there are no issues with it, I'll land this workaround (similarly to #115200) while we investigate and work on emitting metadata for those if possible.

rcvalle · 2025-03-07T18:46:35Z

@bors r+

bors · 2025-03-07T18:46:37Z

📌 Commit ddfa9d3 has been approved by rcvalle

It is now in the queue for this repository.

Noratrieb · 2025-03-07T19:53:08Z

@bors r-

please add a comment with an explanation to every use of #[no_sanitize(cfi)]

rcvalle · 2025-03-08T00:05:10Z

Maybe also point to #115199 for reference.

bors · 2025-03-09T22:34:43Z

☔ The latest upstream changes (presumably #138279) made this pull request unmergeable. Please resolve the merge conflicts.

Currently, when enabling CFI via -Zsanitizer=cfi and executing e.g. std::sys::random::getrandom, we can observe a CFI violation. This is the case for all consumers of the std::sys::pal::weak::weak macro, as it is defining weak functions which don't show up in LLVM IR metadata. CFI fails for all these functions. Similar to other such cases in rust-lang#115199, this change stops emitting the CFI typecheck for consumers of the macro via the \#[no_sanitize(cfi)] attribute.

1c3t3a · 2025-03-10T09:51:26Z

Fair point! I added a comment to every use of #[no_sanitize(cfi)].

rcvalle · 2025-03-10T18:47:33Z

Thank you, Bastian!

rcvalle · 2025-03-10T18:47:41Z

@bors r+

bors · 2025-03-10T18:47:43Z

📌 Commit e5dc1e3 has been approved by rcvalle

It is now in the queue for this repository.

…valle Disable CFI for weakly linked syscalls Currently, when enabling CFI via -Zsanitizer=cfi and executing e.g. std::sys::random::getrandom, we can observe a CFI violation. This is the case for all consumers of the std::sys::pal::weak::syscall macro, as it is defining weak functions which don't show up in LLVM IR metadata. CFI fails for all these functions. Similar to other such cases in rust-lang#115199, this change stops emitting the CFI typecheck for consumers of the macro via the `#[no_sanitize(cfi)]` attribute. r? `@rcvalle`

…valle Disable CFI for weakly linked syscalls Currently, when enabling CFI via -Zsanitizer=cfi and executing e.g. std::sys::random::getrandom, we can observe a CFI violation. This is the case for all consumers of the std::sys::pal::weak::syscall macro, as it is defining weak functions which don't show up in LLVM IR metadata. CFI fails for all these functions. Similar to other such cases in rust-lang#115199, this change stops emitting the CFI typecheck for consumers of the macro via the `#[no_sanitize(cfi)]` attribute. r? ``@rcvalle``

…valle Disable CFI for weakly linked syscalls Currently, when enabling CFI via -Zsanitizer=cfi and executing e.g. std::sys::random::getrandom, we can observe a CFI violation. This is the case for all consumers of the std::sys::pal::weak::syscall macro, as it is defining weak functions which don't show up in LLVM IR metadata. CFI fails for all these functions. Similar to other such cases in rust-lang#115199, this change stops emitting the CFI typecheck for consumers of the macro via the `#[no_sanitize(cfi)]` attribute. r? ```@rcvalle```

Rollup of 16 pull requests Successful merges: - rust-lang#126856 (remove deprecated tool `rls`) - rust-lang#136932 (Reduce formatting `width` and `precision` to 16 bits) - rust-lang#137314 (change definitely unproductive cycles to error) - rust-lang#137612 (Update bootstrap to edition 2024) - rust-lang#138002 (Disable CFI for weakly linked syscalls) - rust-lang#138052 (strip `-Wlinker-messages` wrappers from `rust-lld` rmake test) - rust-lang#138063 (Improve `-Zunpretty=hir` for parsed attrs) - rust-lang#138109 (make precise capturing args in rustdoc Json typed) - rust-lang#138147 (Add maintainers for powerpc64le-unknown-linux-gnu) - rust-lang#138245 (stabilize `ci_rustc_if_unchanged_logic` test for local environments) - rust-lang#138296 (Remove `AdtFlags::IS_ANONYMOUS` and `Copy`/`Clone` condition for anonymous ADT) - rust-lang#138300 (add tracking issue for unqualified_local_imports) - rust-lang#138307 (Allow specifying glob patterns for try jobs) - rust-lang#138313 (Update books) - rust-lang#138315 (use next_back() instead of last() on DoubleEndedIterator) - rust-lang#138318 (Rustdoc: remove a bunch of `@ts-expect-error` from main.js) r? `@ghost` `@rustbot` modify labels: rollup

…valle Disable CFI for weakly linked syscalls Currently, when enabling CFI via -Zsanitizer=cfi and executing e.g. std::sys::random::getrandom, we can observe a CFI violation. This is the case for all consumers of the std::sys::pal::weak::syscall macro, as it is defining weak functions which don't show up in LLVM IR metadata. CFI fails for all these functions. Similar to other such cases in rust-lang#115199, this change stops emitting the CFI typecheck for consumers of the macro via the `#[no_sanitize(cfi)]` attribute. r? ````@rcvalle````

Rollup of 18 pull requests Successful merges: - rust-lang#126856 (remove deprecated tool `rls`) - rust-lang#137314 (change definitely unproductive cycles to error) - rust-lang#137504 (Move methods from Map to TyCtxt, part 4.) - rust-lang#137701 (Convert `ShardedHashMap` to use `hashbrown::HashTable`) - rust-lang#137967 ([AIX] Fix hangs during testing) - rust-lang#138002 (Disable CFI for weakly linked syscalls) - rust-lang#138052 (strip `-Wlinker-messages` wrappers from `rust-lld` rmake test) - rust-lang#138063 (Improve `-Zunpretty=hir` for parsed attrs) - rust-lang#138109 (make precise capturing args in rustdoc Json typed) - rust-lang#138147 (Add maintainers for powerpc64le-unknown-linux-gnu) - rust-lang#138245 (stabilize `ci_rustc_if_unchanged_logic` test for local environments) - rust-lang#138296 (Remove `AdtFlags::IS_ANONYMOUS` and `Copy`/`Clone` condition for anonymous ADT) - rust-lang#138300 (add tracking issue for unqualified_local_imports) - rust-lang#138307 (Allow specifying glob patterns for try jobs) - rust-lang#138313 (Update books) - rust-lang#138315 (use next_back() instead of last() on DoubleEndedIterator) - rust-lang#138318 (Rustdoc: remove a bunch of `@ts-expect-error` from main.js) - rust-lang#138330 (Remove unnecessary `[lints.rust]` sections.) Failed merges: - rust-lang#137147 (Add exclude to config.toml) r? `@ghost` `@rustbot` modify labels: rollup

…valle Disable CFI for weakly linked syscalls Currently, when enabling CFI via -Zsanitizer=cfi and executing e.g. std::sys::random::getrandom, we can observe a CFI violation. This is the case for all consumers of the std::sys::pal::weak::syscall macro, as it is defining weak functions which don't show up in LLVM IR metadata. CFI fails for all these functions. Similar to other such cases in rust-lang#115199, this change stops emitting the CFI typecheck for consumers of the macro via the `#[no_sanitize(cfi)]` attribute. r? `````@rcvalle`````

…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#137715 (Allow int literals for pattern types with int base types) - rust-lang#138002 (Disable CFI for weakly linked syscalls) - rust-lang#138051 (Add support for downloading GCC from CI) - rust-lang#138231 (Prevent ICE in autodiff validation by emitting user-friendly errors) - rust-lang#138245 (stabilize `ci_rustc_if_unchanged_logic` test for local environments) - rust-lang#138256 (Do not feed anon const a type that references generics that it does not have) - rust-lang#138284 (Do not write user type annotation for const param value path) - rust-lang#138296 (Remove `AdtFlags::IS_ANONYMOUS` and `Copy`/`Clone` condition for anonymous ADT) - rust-lang#138352 (miri native_calls: ensure we actually expose *mutable* provenance to the memory FFI can access) - rust-lang#138354 (remove redundant `body` arguments) r? `@ghost` `@rustbot` modify labels: rollup

Previously (rust-lang#115200, rust-lang#138002), we added `#[no_sanitize(cfi)]` to all code paths that call to a weakly linked function. In rust-lang#138349 we fixed the root cause for this issue, which means we can now remove the corresponding attributes.

cfi: Remove #[no_sanitize(cfi)] for extern weak functions Previously (rust-lang#115200, rust-lang#138002), we added `#[no_sanitize(cfi)]` to all code paths that call to a weakly linked function. In rust-lang#138349 we fixed the root cause for this issue, which means we can now remove the corresponding attributes. r? `@rcvalle`

Rollup merge of rust-lang#139667 - 1c3t3a:remove-no-sanitize, r=m-ou-se cfi: Remove #[no_sanitize(cfi)] for extern weak functions Previously (rust-lang#115200, rust-lang#138002), we added `#[no_sanitize(cfi)]` to all code paths that call to a weakly linked function. In rust-lang#138349 we fixed the root cause for this issue, which means we can now remove the corresponding attributes. r? `@rcvalle`

rustbot assigned rcvalle Mar 4, 2025

rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 4, 2025

1c3t3a force-pushed the fix-std-cfi-violation branch from 3ef2ae6 to ddfa9d3 Compare March 5, 2025 14:46

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 Mar 7, 2025

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 7, 2025

1c3t3a added 2 commits March 10, 2025 08:51

Add comments for #[no_sanitize(cfi)] in stdlib

e5dc1e3

1c3t3a force-pushed the fix-std-cfi-violation branch from ddfa9d3 to e5dc1e3 Compare March 10, 2025 09:49

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 10, 2025

rcvalle added PG-exploit-mitigations Project group: Exploit mitigations A-sanitizers Area: Sanitizers for correctness and code quality labels Mar 10, 2025

rcvalle added the A-control-flow-integrity Area: Control Flow Integrity (CFI) security mitigation label Mar 10, 2025

jieyouxu mentioned this pull request Mar 10, 2025

Rollup of 25 pull requests #138324

Closed

jieyouxu mentioned this pull request Mar 10, 2025

Rollup of 17 pull requests #138325

Closed

jieyouxu mentioned this pull request Mar 11, 2025

Rollup of 16 pull requests #138326

Closed

jieyouxu mentioned this pull request Mar 11, 2025

Rollup of 18 pull requests #138337

Closed

This was referenced Mar 11, 2025

Rollup of 11 pull requests #138365

Closed

Rollup of 10 pull requests #138366

Merged

bors merged commit 9746ac5 into rust-lang:master Mar 12, 2025

rustbot added this to the 1.87.0 milestone Mar 12, 2025

rcvalle mentioned this pull request Mar 14, 2025

Emit function declarations for functions with #[linkage="extern_weak"] #138349

Merged

1c3t3a mentioned this pull request Apr 11, 2025

cfi: Remove #[no_sanitize(cfi)] for extern weak functions #139667

Merged

Uh oh!

Conversation

1c3t3a commented Mar 4, 2025

Uh oh!

bjorn3 commented Mar 4, 2025

Uh oh!

rcvalle commented Mar 4, 2025

Uh oh!

Noratrieb commented Mar 4, 2025

Uh oh!

1c3t3a commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rcvalle commented Mar 7, 2025

Uh oh!

rcvalle commented Mar 7, 2025

Uh oh!

bors commented Mar 7, 2025

Uh oh!

Noratrieb commented Mar 7, 2025

Uh oh!

rcvalle commented Mar 8, 2025

Uh oh!

bors commented Mar 9, 2025

Uh oh!

1c3t3a commented Mar 10, 2025

Uh oh!

rcvalle commented Mar 10, 2025

Uh oh!

rcvalle commented Mar 10, 2025

Uh oh!

bors commented Mar 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

1c3t3a commented Mar 5, 2025 •

edited

Loading