add codegen option for using LLVM stack smash protection by bbjornse · Pull Request #84197 · rust-lang/rust (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
LLVM has built-in heuristics for adding stack canaries to functions. These
heuristics can be selected with LLVM function attributes. This patch adds a
rustc option -Z stack-protector={none,basic,strong,all} which controls the use
of these attributes. This gives rustc the same stack smash protection support as
clang offers through options -fno-stack-protector, -fstack-protector,
-fstack-protector-strong, and -fstack-protector-all. The protection this can
offer is demonstrated in test/ui/abi/stack-protector.rs. This fills a gap in the
current list of rustc exploit
mitigations (https://doc.rust-lang.org/rustc/exploit-mitigations.html),
originally discussed in rust-lang#15179.
Stack smash protection adds runtime overhead and is therefore still off by default, but now users have the option to trade performance for security as they see fit. An example use case is adding Rust code in an existing C/C++ code base compiled with stack smash protection. Without the ability to add stack smash protection to the Rust code, the code base artifacts could be exploitable in ways not possible if the code base remained pure C/C++.
Stack smash protection support is present in LLVM for almost all the current tier 1/tier 2 targets: see test/assembly/stack-protector/stack-protector-target-support.rs. The one exception is nvptx64-nvidia-cuda. This patch follows clang's example, and adds a warning message printed if stack smash protection is used with this target (see test/ui/stack-protector/warn-stack-protector-unsupported.rs). Support for tier 3 targets has not been checked.
Since the heuristics are applied at the LLVM level, the heuristics are expected
to add stack smash protection to a fraction of functions comparable to C/C++.
Some experiments demonstrating how Rust code is affected by the different
heuristics can be found in
test/assembly/stack-protector/stack-protector-heuristics-effect.rs. There is
potential for better heuristics using Rust-specific safety information. For
example it might be reasonable to skip stack smash protection in functions which
transitively only use safe Rust code, or which uses only a subset of functions
the user declares safe (such as anything under std.*). Such alternative
heuristics could be added at a later point.
LLVM also offers a "safestack" sanitizer as an alternative way to guard against
stack smashing (see rust-lang#26612). This could possibly also be included as a
stack-protection heuristic. An alternative is to add it as a sanitizer (rust-lang#39699).
This is what clang does: safestack is exposed with option
-fsanitize=safe-stack.
The options are only supported by the LLVM backend, but as with other codegen options it is visible in the main codegen option help menu. The heuristic names "basic", "strong", and "all" are hopefully sufficiently generic to be usable in other backends as well.
Reviewed-by: Nikita Popov nikic@php.net
Extra commits during review:
[address-review] make the stack-protector option unstable
[address-review] reduce detail level of stack-protector option help text
[address-review] correct grammar in comment
[address-review] use compiler flag to avoid merging functions in test
[address-review] specify min LLVM version in fortanix stack-protector test
Only for Fortanix test, since this target specifically requests the
--x86-experimental-lvi-inline-asm-hardeningflag.[address-review] specify required LLVM components in stack-protector tests
move stack protector option enum closer to other similar option enums
rustc_interface/tests: sort debug option list in tracking hash test
add an explicit
nonestack-protector option
Revert "set LLVM requirements for all stack protector support test revisions"
This reverts commit a49b74f92a4e7d701d6f6cf63d207a8aff2e0f68.
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
arielb1 pushed a commit to arielb1/rust that referenced this pull request
I propose stabilizing -Cstack-protector as -Zstack-protector. This PR adds a new -Cstack-protector flag, leaving the unstable -Z flag as is to ease the transition period. The -Z flag will be removed in the future.
No RFC/MCP, this flag was added in rust-lang#84197 and was not deemed large enough to require additional process.
The tracking issue for this feature is rust-lang#114903.
The -Cstack-protector=strong mode uses the same underlying heuristics as Clang's -fstack-protector-strong.
These heuristics weren't designed for Rust, and may be over-conservative in some cases - for example, if
Rust stores a field's data in an alloca using an LLVM array type, LLVM regard the alloca as meaning
that the function has a C array, and enable stack overflow canaries even if the function accesses
the alloca in a safe way. Some people thought we should wait on stabilization until there are better
heuristics, but I didn't hear about any concrete case where this unduly harms performance, and I think
that when a need comes, we can improve the heuristics in LLVM after stabilization.
The heuristics do seem to not be under-conservative, so this should not be a security risk.
The -Cstack-protector=basic mode (-fstack-protector) uses heuristics that are specifically designed
to catch old-C-style string manipulation. This is not a good fit to Rust, which does not perform much
unsafe C-style string manipulation. As far as I can tell, nobody has been asking for it,
and few people are using it even in today's C - modern distros (e.g. Debian) tend to use
-fstack-protector-strong.
Therefore, -Cstack-protector=basic has been removed. If anyone is interested in it, they
are welcome to add it back as an unstable option.
Most implementation was done in <rust-lang#84197>. The command-line attribute enables the relevant LLVM attribute on all functions in <https://github.com/rust-lang/rust/blob/68baa87ba6f03f8b6af2a368690161f1601e4040/compiler/rustc_codegen_llvm/src/attributes.rs#L267-L276>.
Each target can indicate that it does not support stack canaries - currently,
the GPU platforms nvptx64-nvidia-cuda and amdgcn-amd-amdhsa. On these
platforms, use of -Cstack-protector causes an error.
The feature has tests that make sure that the LLVM heuristic gives reasonable
results for several functions, by checking for __security_check_cookie (on Windows)
or __stack_chk_fail (on Linux). See
<https://github.com/rust-lang/rust/tree/68baa87ba6f03f8b6af2a368690161f1601e4040/tests/assembly-llvm/stack-protector>
No call-for-testing has been conducted, but the feature seems to be in use.
No reported bugs seem to exist.
- @bbjornse was the original implementor at rust-lang#84197
- @mrcnski documented it at rust-lang#111722
- @wesleywiser added tests for Windows at rust-lang#116037
- @davidtwco worked on the feature at rust-lang#121742
- @nikic provided support from the LLVM side (on Zulip on <https://rust-lang.zulipchat.com/#narrow/channel/233931-t-compiler.2Fmajor-changes/topic/Proposal.20for.20Adapt.20Stack.20Protector.20for.20Ru.E2.80.A6.20compiler-team.23841> and elsewhere), thanks @nikic!
No FIXMEs related to this feature.
This feature cannot cause undefined behavior.
No changes to reference/spec, docs added to the codegen docs as part of the stabilization PR.
No.
None.
No support needed for rustdoc, clippy, rust-analyzer, rustfmt or rustup.
Cargo could expose this as an option in build profiles but I would expect the decision as to what version should be used would be made for the entire crate graph at build time rather than by individual package authors.
-C stack-protector is propagated to C compilers using cc-rs via rust-lang/cc-rs#1550
arielb1 pushed a commit to arielb1/rust that referenced this pull request
I propose stabilizing -Cstack-protector as -Zstack-protector. This PR adds a new -Cstack-protector flag, leaving the unstable -Z flag as is to ease the transition period. The -Z flag will be removed in the future.
No RFC/MCP, this flag was added in 84197 and was not deemed large enough to require additional process.
The tracking issue for this feature is 114903.
The -Cstack-protector=strong mode uses the same underlying heuristics as Clang's -fstack-protector-strong.
These heuristics weren't designed for Rust, and may be over-conservative in some cases - for example, if
Rust stores a field's data in an alloca using an LLVM array type, LLVM regard the alloca as meaning
that the function has a C array, and enable stack overflow canaries even if the function accesses
the alloca in a safe way. Some people thought we should wait on stabilization until there are better
heuristics, but I didn't hear about any concrete case where this unduly harms performance, and I think
that when a need comes, we can improve the heuristics in LLVM after stabilization.
The heuristics do seem to not be under-conservative, so this should not be a security risk.
The -Cstack-protector=basic mode (-fstack-protector) uses heuristics that are specifically designed
to catch old-C-style string manipulation. This is not a good fit to Rust, which does not perform much
unsafe C-style string manipulation. As far as I can tell, nobody has been asking for it,
and few people are using it even in today's C - modern distros (e.g. Debian) tend to use
-fstack-protector-strong.
Therefore, -Cstack-protector=basic has been removed. If anyone is interested in it, they
are welcome to add it back as an unstable option.
Most implementation was done in <rust-lang#84197>. The command-line attribute enables the relevant LLVM attribute on all functions in <https://github.com/rust-lang/rust/blob/68baa87ba6f03f8b6af2a368690161f1601e4040/compiler/rustc_codegen_llvm/src/attributes.rs#L267-L276>.
Each target can indicate that it does not support stack canaries - currently,
the GPU platforms nvptx64-nvidia-cuda and amdgcn-amd-amdhsa. On these
platforms, use of -Cstack-protector causes an error.
The feature has tests that make sure that the LLVM heuristic gives reasonable
results for several functions, by checking for __security_check_cookie (on Windows)
or __stack_chk_fail (on Linux). See
<https://github.com/rust-lang/rust/tree/68baa87ba6f03f8b6af2a368690161f1601e4040/tests/assembly-llvm/stack-protector>
No call-for-testing has been conducted, but the feature seems to be in use.
No reported bugs seem to exist.
- bbjornse was the original implementor at 84197
- mrcnski documented it at 111722
- wesleywiser added tests for Windows at 116037
- davidtwco worked on the feature at 121742
- nikic provided support from the LLVM side (on Zulip on <https://rust-lang.zulipchat.com/#narrow/channel/233931-t-compiler.2Fmajor-changes/topic/Proposal.20for.20Adapt.20Stack.20Protector.20for.20Ru.E2.80.A6.20compiler-team.23841> and elsewhere), thanks nikic!
No FIXMEs related to this feature.
This feature cannot cause undefined behavior.
No changes to reference/spec, docs added to the codegen docs as part of the stabilization PR.
No.
None.
No support needed for rustdoc, clippy, rust-analyzer, rustfmt or rustup.
Cargo could expose this as an option in build profiles but I would expect the decision as to what version should be used would be made for the entire crate graph at build time rather than by individual package authors.
-C stack-protector is propagated to C compilers using cc-rs via rust-lang/cc-rs issue 1550
arielb1 pushed a commit to arielb1/rust that referenced this pull request
I propose stabilizing -Cstack-protector as -Zstack-protector. This PR adds a new -Cstack-protector flag, leaving the unstable -Z flag as is to ease the transition period. The -Z flag will be removed in the future.
No RFC/MCP, this flag was added in 84197 and was not deemed large enough to require additional process.
The tracking issue for this feature is 114903.
The -Cstack-protector=strong mode uses the same underlying heuristics as Clang's -fstack-protector-strong.
These heuristics weren't designed for Rust, and may be over-conservative in some cases - for example, if
Rust stores a field's data in an alloca using an LLVM array type, LLVM regard the alloca as meaning
that the function has a C array, and enable stack overflow canaries even if the function accesses
the alloca in a safe way. Some people thought we should wait on stabilization until there are better
heuristics, but I didn't hear about any concrete case where this unduly harms performance, and I think
that when a need comes, we can improve the heuristics in LLVM after stabilization.
The heuristics do seem to not be under-conservative, so this should not be a security risk.
The -Cstack-protector=basic mode (-fstack-protector) uses heuristics that are specifically designed
to catch old-C-style string manipulation. This is not a good fit to Rust, which does not perform much
unsafe C-style string manipulation. As far as I can tell, nobody has been asking for it,
and few people are using it even in today's C - modern distros (e.g. Debian) tend to use
-fstack-protector-strong.
Therefore, -Cstack-protector=basic has been removed. If anyone is interested in it, they
are welcome to add it back as an unstable option.
Most implementation was done in <rust-lang#84197>. The command-line attribute enables the relevant LLVM attribute on all functions in <https://github.com/rust-lang/rust/blob/68baa87ba6f03f8b6af2a368690161f1601e4040/compiler/rustc_codegen_llvm/src/attributes.rs#L267-L276>.
Each target can indicate that it does not support stack canaries - currently,
the GPU platforms nvptx64-nvidia-cuda and amdgcn-amd-amdhsa. On these
platforms, use of -Cstack-protector causes an error.
The feature has tests that make sure that the LLVM heuristic gives reasonable
results for several functions, by checking for __security_check_cookie (on Windows)
or __stack_chk_fail (on Linux). See
<https://github.com/rust-lang/rust/tree/68baa87ba6f03f8b6af2a368690161f1601e4040/tests/assembly-llvm/stack-protector>
No call-for-testing has been conducted, but the feature seems to be in use.
No reported bugs seem to exist.
- bbjornse was the original implementor at 84197
- mrcnski documented it at 111722
- wesleywiser added tests for Windows at 116037
- davidtwco worked on the feature at 121742
- nikic provided support from the LLVM side (on Zulip on <https://rust-lang.zulipchat.com/#narrow/channel/233931-t-compiler.2Fmajor-changes/topic/Proposal.20for.20Adapt.20Stack.20Protector.20for.20Ru.E2.80.A6.20compiler-team.23841> and elsewhere), thanks nikic!
No FIXMEs related to this feature.
This feature cannot cause undefined behavior.
No changes to reference/spec, docs added to the codegen docs as part of the stabilization PR.
No.
None.
No support needed for rustdoc, clippy, rust-analyzer, rustfmt or rustup.
Cargo could expose this as an option in build profiles but I would expect the decision as to what version should be used would be made for the entire crate graph at build time rather than by individual package authors.
-C stack-protector is propagated to C compilers using cc-rs via rust-lang/cc-rs issue 1550
arielb1 pushed a commit to arielb1/rust that referenced this pull request
I propose stabilizing -Cstack-protector as -Zstack-protector. This PR adds a new -Cstack-protector flag, leaving the unstable -Z flag as is to ease the transition period. The -Z flag will be removed in the future.
No RFC/MCP, this flag was added in 84197 and was not deemed large enough to require additional process.
The tracking issue for this feature is 114903.
The -Cstack-protector=strong mode uses the same underlying heuristics as Clang's -fstack-protector-strong.
These heuristics weren't designed for Rust, and may be over-conservative in some cases - for example, if
Rust stores a field's data in an alloca using an LLVM array type, LLVM regard the alloca as meaning
that the function has a C array, and enable stack overflow canaries even if the function accesses
the alloca in a safe way. Some people thought we should wait on stabilization until there are better
heuristics, but I didn't hear about any concrete case where this unduly harms performance, and I think
that when a need comes, we can improve the heuristics in LLVM after stabilization.
The heuristics do seem to not be under-conservative, so this should not be a security risk.
The -Cstack-protector=basic mode (-fstack-protector) uses heuristics that are specifically designed
to catch old-C-style string manipulation. This is not a good fit to Rust, which does not perform much
unsafe C-style string manipulation. As far as I can tell, nobody has been asking for it,
and few people are using it even in today's C - modern distros (e.g. Debian) tend to use
-fstack-protector-strong.
Therefore, -Cstack-protector=basic has been removed. If anyone is interested in it, they
are welcome to add it back as an unstable option.
Most implementation was done in <rust-lang#84197>. The command-line attribute enables the relevant LLVM attribute on all functions in <https://github.com/rust-lang/rust/blob/68baa87ba6f03f8b6af2a368690161f1601e4040/compiler/rustc_codegen_llvm/src/attributes.rs#L267-L276>.
Each target can indicate that it does not support stack canaries - currently,
the GPU platforms nvptx64-nvidia-cuda and amdgcn-amd-amdhsa. On these
platforms, use of -Cstack-protector causes an error.
The feature has tests that make sure that the LLVM heuristic gives reasonable
results for several functions, by checking for __security_check_cookie (on Windows)
or __stack_chk_fail (on Linux). See
<https://github.com/rust-lang/rust/tree/68baa87ba6f03f8b6af2a368690161f1601e4040/tests/assembly-llvm/stack-protector>
No call-for-testing has been conducted, but the feature seems to be in use.
No reported bugs seem to exist.
- bbjornse was the original implementor at 84197
- mrcnski documented it at 111722
- wesleywiser added tests for Windows at 116037
- davidtwco worked on the feature at 121742
- nikic provided support from the LLVM side (on Zulip on <https://rust-lang.zulipchat.com/#narrow/channel/233931-t-compiler.2Fmajor-changes/topic/Proposal.20for.20Adapt.20Stack.20Protector.20for.20Ru.E2.80.A6.20compiler-team.23841> and elsewhere), thanks nikic!
No FIXMEs related to this feature.
This feature cannot cause undefined behavior.
No changes to reference/spec, docs added to the codegen docs as part of the stabilization PR.
No.
None.
No support needed for rustdoc, clippy, rust-analyzer, rustfmt or rustup.
Cargo could expose this as an option in build profiles but I would expect the decision as to what version should be used would be made for the entire crate graph at build time rather than by individual package authors.
-C stack-protector is propagated to C compilers using cc-rs via rust-lang/cc-rs issue 1550
arielb1 pushed a commit to arielb1/rust that referenced this pull request
I propose stabilizing -Cstack-protector as -Zstack-protector. This PR adds a new -Cstack-protector flag, leaving the unstable -Z flag as is to ease the transition period. The -Z flag will be removed in the future.
No RFC/MCP, this flag was added in 84197 and was not deemed large enough to require additional process.
The tracking issue for this feature is 114903.
The -Cstack-protector=strong mode uses the same underlying heuristics as Clang's -fstack-protector-strong.
These heuristics weren't designed for Rust, and may be over-conservative in some cases - for example, if
Rust stores a field's data in an alloca using an LLVM array type, LLVM regard the alloca as meaning
that the function has a C array, and enable stack overflow canaries even if the function accesses
the alloca in a safe way. Some people thought we should wait on stabilization until there are better
heuristics, but I didn't hear about any concrete case where this unduly harms performance, and I think
that when a need comes, we can improve the heuristics in LLVM after stabilization.
The heuristics do seem to not be under-conservative, so this should not be a security risk.
The -Cstack-protector=basic mode (-fstack-protector) uses heuristics that are specifically designed
to catch old-C-style string manipulation. This is not a good fit to Rust, which does not perform much
unsafe C-style string manipulation. As far as I can tell, nobody has been asking for it,
and few people are using it even in today's C - modern distros (e.g. Debian) tend to use
-fstack-protector-strong.
Therefore, -Cstack-protector=basic has been removed. If anyone is interested in it, they
are welcome to add it back as an unstable option.
Most implementation was done in <rust-lang#84197>. The command-line attribute enables the relevant LLVM attribute on all functions in <https://github.com/rust-lang/rust/blob/68baa87ba6f03f8b6af2a368690161f1601e4040/compiler/rustc_codegen_llvm/src/attributes.rs#L267-L276>.
Each target can indicate that it does not support stack canaries - currently,
the GPU platforms nvptx64-nvidia-cuda and amdgcn-amd-amdhsa. On these
platforms, use of -Cstack-protector causes an error.
The feature has tests that make sure that the LLVM heuristic gives reasonable
results for several functions, by checking for __security_check_cookie (on Windows)
or __stack_chk_fail (on Linux). See
<https://github.com/rust-lang/rust/tree/68baa87ba6f03f8b6af2a368690161f1601e4040/tests/assembly-llvm/stack-protector>
No call-for-testing has been conducted, but the feature seems to be in use.
No reported bugs seem to exist.
- bbjornse was the original implementor at 84197
- mrcnski documented it at 111722
- wesleywiser added tests for Windows at 116037
- davidtwco worked on the feature at 121742
- nikic provided support from the LLVM side (on Zulip on <https://rust-lang.zulipchat.com/#narrow/channel/233931-t-compiler.2Fmajor-changes/topic/Proposal.20for.20Adapt.20Stack.20Protector.20for.20Ru.E2.80.A6.20compiler-team.23841> and elsewhere), thanks nikic!
No FIXMEs related to this feature.
This feature cannot cause undefined behavior.
No changes to reference/spec, docs added to the codegen docs as part of the stabilization PR.
No.
None.
No support needed for rustdoc, clippy, rust-analyzer, rustfmt or rustup.
Cargo could expose this as an option in build profiles but I would expect the decision as to what version should be used would be made for the entire crate graph at build time rather than by individual package authors.
-C stack-protector is propagated to C compilers using cc-rs via rust-lang/cc-rs issue 1550
arielb1 pushed a commit to arielb1/rust that referenced this pull request
I propose stabilizing -Cstack-protector as -Zstack-protector. This PR adds a new -Cstack-protector flag, leaving the unstable -Z flag as is to ease the transition period. The -Z flag will be removed in the future.
No RFC/MCP, this flag was added in 84197 and was not deemed large enough to require additional process.
The tracking issue for this feature is 114903.
The -Cstack-protector=strong mode uses the same underlying heuristics as Clang's -fstack-protector-strong.
These heuristics weren't designed for Rust, and may be over-conservative in some cases - for example, if
Rust stores a field's data in an alloca using an LLVM array type, LLVM regard the alloca as meaning
that the function has a C array, and enable stack overflow canaries even if the function accesses
the alloca in a safe way. Some people thought we should wait on stabilization until there are better
heuristics, but I didn't hear about any concrete case where this unduly harms performance, and I think
that when a need comes, we can improve the heuristics in LLVM after stabilization.
The heuristics do seem to not be under-conservative, so this should not be a security risk.
The -Cstack-protector=basic mode (-fstack-protector) uses heuristics that are specifically designed
to catch old-C-style string manipulation. This is not a good fit to Rust, which does not perform much
unsafe C-style string manipulation. As far as I can tell, nobody has been asking for it,
and few people are using it even in today's C - modern distros (e.g. Debian) tend to use
-fstack-protector-strong.
Therefore, -Cstack-protector=basic has been removed. If anyone is interested in it, they
are welcome to add it back as an unstable option.
Most implementation was done in <rust-lang#84197>. The command-line attribute enables the relevant LLVM attribute on all functions in <https://github.com/rust-lang/rust/blob/68baa87ba6f03f8b6af2a368690161f1601e4040/compiler/rustc_codegen_llvm/src/attributes.rs#L267-L276>.
Each target can indicate that it does not support stack canaries - currently,
the GPU platforms nvptx64-nvidia-cuda and amdgcn-amd-amdhsa. On these
platforms, use of -Cstack-protector causes an error.
The feature has tests that make sure that the LLVM heuristic gives reasonable
results for several functions, by checking for __security_check_cookie (on Windows)
or __stack_chk_fail (on Linux). See
<https://github.com/rust-lang/rust/tree/68baa87ba6f03f8b6af2a368690161f1601e4040/tests/assembly-llvm/stack-protector>
No call-for-testing has been conducted, but the feature seems to be in use.
No reported bugs seem to exist.
- bbjornse was the original implementor at 84197
- mrcnski documented it at 111722
- wesleywiser added tests for Windows at 116037
- davidtwco worked on the feature at 121742
- nikic provided support from the LLVM side (on Zulip on <https://rust-lang.zulipchat.com/#narrow/channel/233931-t-compiler.2Fmajor-changes/topic/Proposal.20for.20Adapt.20Stack.20Protector.20for.20Ru.E2.80.A6.20compiler-team.23841> and elsewhere), thanks nikic!
No FIXMEs related to this feature.
This feature cannot cause undefined behavior.
No changes to reference/spec, docs added to the codegen docs as part of the stabilization PR.
No.
None.
No support needed for rustdoc, clippy, rust-analyzer, rustfmt or rustup.
Cargo could expose this as an option in build profiles but I would expect the decision as to what version should be used would be made for the entire crate graph at build time rather than by individual package authors.
-C stack-protector is propagated to C compilers using cc-rs via rust-lang/cc-rs issue 1550