Skip to content

Commit d9e6ae9

Browse files
author
Ariel Ben-Yehuda
committed
Stabilize -Zstack-protector as -Cstack-protector
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. [Debian]: https://wiki.debian.org/Hardening#DEB_BUILD_HARDENING_STACKPROTECTOR_.28gcc.2Fg.2B-.2B-_-fstack-protector-strong.29 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
1 parent 8387095 commit d9e6ae9

25 files changed

Lines changed: 323 additions & 67 deletions

bootstrap.example.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -755,7 +755,7 @@
755755
#rust.frame-pointers = false
756756

757757
# Indicates whether stack protectors should be used
758-
# via the unstable option `-Zstack-protector`.
758+
# via `-Cstack-protector`.
759759
#
760760
# Valid options are : `none`(default),`basic`,`strong`, or `all`.
761761
# `strong` and `basic` options may be buggy and are not recommended, see rust-lang/rust#114903.

compiler/rustc_codegen_llvm/src/lib.rs

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -314,19 +314,32 @@ impl CodegenBackend for LlvmCodegenBackend {
314314
Generate stack canaries in all functions.
315315
316316
strong
317-
Generate stack canaries in a function if it either:
318-
- has a local variable of `[T; N]` type, regardless of `T` and `N`
319-
- takes the address of a local variable.
317+
Generate stack canaries for all functions, unless the compiler
318+
can prove these functions can't be the source of a stack
319+
buffer overflow (even in the presence of undefined behavior).
320320
321-
(Note that a local variable being borrowed is not equivalent to its
322-
address being taken: e.g. some borrows may be removed by optimization,
323-
while by-value argument passing may be implemented with reference to a
324-
local stack variable in the ABI.)
321+
This provides the same security guarantees as Clang's
322+
`-fstack-protector=strong`.
323+
324+
The exact rules are unstable and subject to change, but
325+
currently, it generates stack protectors for functions that,
326+
*post-optimization*, contain either arrays (of any size
327+
or type) or address-taken locals.
325328
326329
basic
327-
Generate stack canaries in functions with local variables of `[T; N]`
330+
Generate stack canaries in functions that are heuristically
331+
suspected to contain buffer overflows.
332+
333+
The heuristic is subject to change, but currently it
334+
includes functions with local variables of `[T; N]`
328335
type, where `T` is byte-sized and `N` >= 8.
329336
337+
This heuristic originated from C, where it detects
338+
functions that allocate a `char buf[N];` buffer on the
339+
stack, and are therefore likely to have a stack buffer overflow
340+
in the case of a length-calculation error. It is *not* a good
341+
heuristic for Rust code.
342+
330343
none
331344
Do not generate stack canaries.
332345
"#

compiler/rustc_interface/src/tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,7 @@ fn test_codegen_options_tracking_hash() {
640640
tracked!(relro_level, Some(RelroLevel::Full));
641641
tracked!(soft_float, true);
642642
tracked!(split_debuginfo, Some(SplitDebuginfo::Packed));
643+
tracked!(stack_protector, StackProtector::All);
643644
tracked!(symbol_mangling_version, Some(SymbolManglingVersion::V0));
644645
tracked!(target_cpu, Some(String::from("abc")));
645646
tracked!(target_feature, String::from("all the features, all of them"));
@@ -870,7 +871,6 @@ fn test_unstable_options_tracking_hash() {
870871
tracked!(small_data_threshold, Some(16));
871872
tracked!(split_lto_unit, Some(true));
872873
tracked!(src_hash_algorithm, Some(SourceFileHashAlgorithm::Sha1));
873-
tracked!(stack_protector, StackProtector::All);
874874
tracked!(teach, true);
875875
tracked!(thinlto, Some(true));
876876
tracked!(tiny_const_eval_limit, true);
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
session_apple_deployment_target_invalid =
2+
failed to parse deployment target specified in {$env_var}: {$error}
3+
4+
session_apple_deployment_target_too_low =
5+
deployment target in {$env_var} was set to {$version}, but the minimum supported by `rustc` is {$os_min}
6+
7+
session_binary_float_literal_not_supported = binary float literal is not supported
8+
session_branch_protection_requires_aarch64 = `-Zbranch-protection` is only supported on aarch64
9+
10+
session_cannot_enable_crt_static_linux = sanitizer is incompatible with statically linked libc, disable it using `-C target-feature=-crt-static`
11+
12+
session_cannot_mix_and_match_sanitizers = `-Zsanitizer={$first}` is incompatible with `-Zsanitizer={$second}`
13+
14+
session_cli_feature_diagnostic_help =
15+
add `-Zcrate-attr="feature({$feature})"` to the command-line options to enable
16+
17+
session_crate_name_empty = crate name must not be empty
18+
19+
session_embed_source_insufficient_dwarf_version = `-Zembed-source=y` requires at least `-Z dwarf-version=5` but DWARF version is {$dwarf_version}
20+
21+
session_embed_source_requires_debug_info = `-Zembed-source=y` requires debug information to be enabled
22+
23+
session_expr_parentheses_needed = parentheses are required to parse this as an expression
24+
25+
session_failed_to_create_profiler = failed to create profiler: {$err}
26+
27+
session_feature_diagnostic_for_issue =
28+
see issue #{$n} <https://github.com/rust-lang/rust/issues/{$n}> for more information
29+
30+
session_feature_diagnostic_help =
31+
add `#![feature({$feature})]` to the crate attributes to enable
32+
33+
session_feature_diagnostic_suggestion =
34+
add `#![feature({$feature})]` to the crate attributes to enable
35+
36+
session_feature_suggest_upgrade_compiler =
37+
this compiler was built on {$date}; consider upgrading it if it is out of date
38+
39+
session_file_is_not_writeable = output file {$file} is not writeable -- check its permissions
40+
41+
session_file_write_fail = failed to write `{$path}` due to error `{$err}`
42+
43+
session_function_return_requires_x86_or_x86_64 = `-Zfunction-return` (except `keep`) is only supported on x86 and x86_64
44+
45+
session_function_return_thunk_extern_requires_non_large_code_model = `-Zfunction-return=thunk-extern` is only supported on non-large code models
46+
47+
session_hexadecimal_float_literal_not_supported = hexadecimal float literal is not supported
48+
49+
session_incompatible_linker_flavor = linker flavor `{$flavor}` is incompatible with the current target
50+
.note = compatible flavors are: {$compatible_list}
51+
52+
session_indirect_branch_cs_prefix_requires_x86_or_x86_64 = `-Zindirect-branch-cs-prefix` is only supported on x86 and x86_64
53+
54+
session_instrumentation_not_supported = {$us} instrumentation is not supported for this target
55+
56+
session_int_literal_too_large = integer literal is too large
57+
.note = value exceeds limit of `{$limit}`
58+
59+
session_invalid_character_in_crate_name = invalid character {$character} in crate name: `{$crate_name}`
60+
61+
session_invalid_float_literal_suffix = invalid suffix `{$suffix}` for float literal
62+
.label = invalid suffix `{$suffix}`
63+
.help = valid suffixes are `f32` and `f64`
64+
65+
session_invalid_float_literal_width = invalid width `{$width}` for float literal
66+
.help = valid widths are 32 and 64
67+
68+
session_invalid_int_literal_width = invalid width `{$width}` for integer literal
69+
.help = valid widths are 8, 16, 32, 64 and 128
70+
71+
session_invalid_literal_suffix = suffixes on {$kind} literals are invalid
72+
.label = invalid suffix `{$suffix}`
73+
74+
session_invalid_num_literal_base_prefix = invalid base prefix for number literal
75+
.note = base prefixes (`0xff`, `0b1010`, `0o755`) are lowercase
76+
.suggestion = try making the prefix lowercase
77+
78+
session_invalid_num_literal_suffix = invalid suffix `{$suffix}` for number literal
79+
.label = invalid suffix `{$suffix}`
80+
.help = the suffix must be one of the numeric types (`u32`, `isize`, `f32`, etc.)
81+
82+
session_linker_plugin_lto_windows_not_supported = linker plugin based LTO is not supported together with `-C prefer-dynamic` when targeting Windows-like targets
83+
84+
session_must_be_name_of_associated_function = must be a name of an associated function
85+
86+
session_not_circumvent_feature = `-Zunleash-the-miri-inside-of-you` may not be used to circumvent feature gates, except when testing error paths in the CTFE engine
87+
88+
session_not_supported = not supported
89+
90+
session_octal_float_literal_not_supported = octal float literal is not supported
91+
92+
session_profile_sample_use_file_does_not_exist = file `{$path}` passed to `-C profile-sample-use` does not exist
93+
94+
session_profile_use_file_does_not_exist = file `{$path}` passed to `-C profile-use` does not exist
95+
96+
session_sanitizer_cfi_canonical_jump_tables_requires_cfi = `-Zsanitizer-cfi-canonical-jump-tables` requires `-Zsanitizer=cfi`
97+
98+
session_sanitizer_cfi_generalize_pointers_requires_cfi = `-Zsanitizer-cfi-generalize-pointers` requires `-Zsanitizer=cfi` or `-Zsanitizer=kcfi`
99+
100+
session_sanitizer_cfi_normalize_integers_requires_cfi = `-Zsanitizer-cfi-normalize-integers` requires `-Zsanitizer=cfi` or `-Zsanitizer=kcfi`
101+
102+
session_sanitizer_cfi_requires_lto = `-Zsanitizer=cfi` requires `-Clto` or `-Clinker-plugin-lto`
103+
104+
session_sanitizer_cfi_requires_single_codegen_unit = `-Zsanitizer=cfi` with `-Clto` requires `-Ccodegen-units=1`
105+
106+
session_sanitizer_kcfi_arity_requires_kcfi = `-Zsanitizer-kcfi-arity` requires `-Zsanitizer=kcfi`
107+
108+
session_sanitizer_kcfi_requires_panic_abort = `-Z sanitizer=kcfi` requires `-C panic=abort`
109+
110+
session_sanitizer_not_supported = {$us} sanitizer is not supported for this target
111+
112+
session_sanitizers_not_supported = {$us} sanitizers are not supported for this target
113+
114+
session_skipping_const_checks = skipping const checks
115+
116+
session_soft_float_deprecated =
117+
`-Csoft-float` is unsound and deprecated; use a corresponding *eabi target instead
118+
.note = it will be removed or ignored in a future version of Rust
119+
session_soft_float_deprecated_issue = see issue #129893 <https://github.com/rust-lang/rust/issues/129893> for more information
120+
121+
session_soft_float_ignored =
122+
`-Csoft-float` is ignored on this target; it only has an effect on *eabihf targets
123+
.note = this may become a hard error in a future version of Rust
124+
125+
session_split_debuginfo_unstable_platform = `-Csplit-debuginfo={$debuginfo}` is unstable on this platform
126+
127+
session_split_lto_unit_requires_lto = `-Zsplit-lto-unit` requires `-Clto`, `-Clto=thin`, or `-Clinker-plugin-lto`
128+
129+
session_target_requires_unwind_tables = target requires unwind tables, they cannot be disabled with `-C force-unwind-tables=no`
130+
131+
session_target_small_data_threshold_not_supported = `-Z small-data-threshold` is not supported for target {$target_triple} and will be ignored
132+
133+
session_target_stack_protector_not_supported = `-C stack-protector={$stack_protector}` is not supported for target {$target_triple}
134+
135+
session_unexpected_builtin_cfg = unexpected `--cfg {$cfg}` flag
136+
.controlled_by = config `{$cfg_name}` is only supposed to be controlled by `{$controlled_by}`
137+
.incoherent = manually setting a built-in cfg can and does create incoherent behaviors
138+
139+
session_unleashed_feature_help_named = skipping check for `{$gate}` feature
140+
session_unleashed_feature_help_unnamed = skipping check that does not even have a feature gate
141+
142+
session_unstable_virtual_function_elimination = `-Zvirtual-function-elimination` requires `-Clto`
143+
144+
session_unsupported_crate_type_for_codegen_backend =
145+
dropping unsupported crate type `{$crate_type}` for codegen backend `{$codegen_backend}`
146+
147+
session_unsupported_crate_type_for_target =
148+
dropping unsupported crate type `{$crate_type}` for target `{$target_triple}`
149+
150+
session_unsupported_dwarf_version = requested DWARF version {$dwarf_version} is not supported
151+
session_unsupported_dwarf_version_help = supported DWARF versions are 2, 3, 4 and 5
152+
153+
session_unsupported_reg_struct_return_arch = `-Zreg-struct-return` is only supported on x86
154+
session_unsupported_regparm = `-Zregparm={$regparm}` is unsupported (valid values 0-3)
155+
session_unsupported_regparm_arch = `-Zregparm=N` is only supported on x86

compiler/rustc_session/src/errors.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,9 @@ pub(crate) struct EmbedSourceRequiresDebugInfo;
206206

207207
#[derive(Diagnostic)]
208208
#[diag(
209-
"`-Z stack-protector={$stack_protector}` is not supported for target {$target_triple} and will be ignored"
209+
"`-C stack-protector={$stack_protector}` is not supported for target {$target_triple} and will be ignored"
210210
)]
211-
pub(crate) struct StackProtectorNotSupportedForTarget<'a> {
212-
pub(crate) stack_protector: StackProtector,
211+
pub(crate) struct StackProtectorNotSupportedForTarget<'a> { pub(crate) stack_protector: StackProtector,
213212
pub(crate) target_triple: &'a TargetTuple,
214213
}
215214

compiler/rustc_session/src/options.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1895,9 +1895,12 @@ pub mod parse {
18951895
true
18961896
}
18971897

1898-
pub(crate) fn parse_stack_protector(slot: &mut StackProtector, v: Option<&str>) -> bool {
1898+
pub(crate) fn parse_stack_protector(
1899+
slot: &mut Option<StackProtector>,
1900+
v: Option<&str>,
1901+
) -> bool {
18991902
match v.and_then(|s| StackProtector::from_str(s).ok()) {
1900-
Some(ssp) => *slot = ssp,
1903+
Some(ssp) => *slot = Some(ssp),
19011904
_ => return false,
19021905
}
19031906
true
@@ -2190,6 +2193,9 @@ options! {
21902193
#[rustc_lint_opt_deny_field_access("use `Session::split_debuginfo` instead of this field")]
21912194
split_debuginfo: Option<SplitDebuginfo> = (None, parse_split_debuginfo, [TRACKED],
21922195
"how to handle split-debuginfo, a platform-specific option"),
2196+
#[rustc_lint_opt_deny_field_access("use `Session::stack_protector` instead of this field")]
2197+
stack_protector: Option<StackProtector> = (None, parse_stack_protector, [TRACKED],
2198+
"control stack smash protection strategy (`rustc --print stack-protector-strategies` for details)"),
21932199
strip: Strip = (Strip::None, parse_strip, [UNTRACKED],
21942200
"tell the linker which information to strip (`none` (default), `debuginfo` or `symbols`)"),
21952201
symbol_mangling_version: Option<SymbolManglingVersion> = (None,
@@ -2670,7 +2676,7 @@ written to standard error output)"),
26702676
src_hash_algorithm: Option<SourceFileHashAlgorithm> = (None, parse_src_file_hash, [TRACKED],
26712677
"hash algorithm of source files in debug info (`md5`, `sha1`, or `sha256`)"),
26722678
#[rustc_lint_opt_deny_field_access("use `Session::stack_protector` instead of this field")]
2673-
stack_protector: StackProtector = (StackProtector::None, parse_stack_protector, [TRACKED],
2679+
stack_protector: Option<StackProtector> = (None, parse_stack_protector, [TRACKED],
26742680
"control stack smash protection strategy (`rustc --print stack-protector-strategies` for details)"),
26752681
staticlib_allow_rdylib_deps: bool = (false, parse_bool, [TRACKED],
26762682
"allow staticlibs to have rust dylib dependencies"),

compiler/rustc_session/src/session.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -729,11 +729,12 @@ impl Session {
729729
}
730730

731731
pub fn stack_protector(&self) -> StackProtector {
732-
if self.target.options.supports_stack_protector {
733-
self.opts.unstable_opts.stack_protector
734-
} else {
735-
StackProtector::None
736-
}
732+
// -C stack-protector overwrites -Z stack-protector, default to StackProtector::None
733+
self.opts
734+
.cg
735+
.stack_protector
736+
.or(self.opts.unstable_opts.stack_protector)
737+
.unwrap_or(StackProtector::None)
737738
}
738739

739740
pub fn must_emit_unwind_tables(&self) -> bool {
@@ -1250,10 +1251,10 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
12501251
}
12511252
}
12521253

1253-
if sess.opts.unstable_opts.stack_protector != StackProtector::None {
1254+
if sess.stack_protector() != StackProtector::None {
12541255
if !sess.target.options.supports_stack_protector {
1255-
sess.dcx().emit_warn(errors::StackProtectorNotSupportedForTarget {
1256-
stack_protector: sess.opts.unstable_opts.stack_protector,
1256+
sess.dcx().emit_err(errors::StackProtectorNotSupportedForTarget {
1257+
stack_protector: sess.stack_protector(),
12571258
target_triple: &sess.opts.target_triple,
12581259
});
12591260
}

src/bootstrap/src/core/builder/cargo.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -958,7 +958,7 @@ impl Builder<'_> {
958958
cargo.env(profile_var("STRIP"), self.config.rust_strip.to_string());
959959

960960
if let Some(stack_protector) = &self.config.rust_stack_protector {
961-
rustflags.arg(&format!("-Zstack-protector={stack_protector}"));
961+
rustflags.arg(&format!("-Cstack-protector={stack_protector}"));
962962
}
963963

964964
let debuginfo_level = match mode {

src/doc/rustc/src/codegen-options/index.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,34 @@ Note that all three options are supported on Linux and Apple platforms,
695695
Attempting to use an unsupported option requires using the nightly channel
696696
with the `-Z unstable-options` flag.
697697

698+
## stack-protector
699+
700+
The option `-C stack-protector` (currently also supported in the
701+
old style `-Z stack-protector`) controls the generation of
702+
stack-protector canaries.
703+
704+
This flag controls stack smashing protection strategy.
705+
706+
Supported values for this option are:
707+
- `none` (default): Disable stack canary generation
708+
- `basic`: Generate stack canaries in functions that are suspected
709+
to have a high chance of containing stack buffer overflows (deprecated).
710+
- `strong`: Generate stack canaries in all functions, unless the compiler
711+
can prove these functions can't be the source of a stack
712+
buffer overflow (even in the presence of undefined behavior).
713+
714+
This provides the same security guarantees as Clang's
715+
`-fstack-protector=strong`.
716+
717+
The exact rules are unstable and subject to change, but
718+
currently, it generates stack protectors for functions that,
719+
*post-optimization*, contain either arrays (of any size
720+
or type) or address-taken locals.
721+
- `all`: Generate stack canaries in all functions
722+
723+
Stack protectors are not supported on many GPU targets, use of stack
724+
protectors on these targets is an error.
725+
698726
## strip
699727

700728
The option `-C strip=val` controls stripping of debuginfo and similar auxiliary

src/doc/rustc/src/exploit-mitigations.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ equivalent.
6262
| Stack clashing protection | Yes | Yes | 1.20.0 (2017-08-31) |
6363
| Read-only relocations and immediate binding | Yes | Yes | 1.21.0 (2017-10-12) |
6464
| Heap corruption protection | Yes | Yes | 1.32.0 (2019-01-17) (via operating system default or specified allocator) |
65-
| Stack smashing protection | Yes | No, `-Z stack-protector` | Nightly |
65+
| Stack smashing protection | Yes | No, `-C stack-protector` | ??? |
6666
| Forward-edge control flow protection | Yes | No, `-Z sanitizer=cfi` | Nightly |
6767
| Backward-edge control flow protection (e.g., shadow and safe stack) | Yes | No, `-Z sanitizer=shadow-call-stack,safestack` | Nightly |
6868

0 commit comments

Comments
 (0)