sess: stabilize -C stack-protector=all

[davidtwco]

cc #114903
r? @nikic

[davidtwco]

Stabilization report

Stack smashing protection is a codegen option (just enabling this support in LLVM). It inserts stack canaries into functions to detect when stack smashing occurs. Stack canaries are random guard values, which are checked for changes when returning.

Stack smashing has been implemented since 2021 and hasn't seen any significant changes since then. Only the all option for this flag is stabilized as the basic and strong options rely on LLVM heuristics which are unsuitable for Rust. There are a handful of examples of users requesting this stabilisation (#114903 (comment), #84197 (comment)).

Tests

• tests/assembly/stack-protector
• tests/codegen/stack-protector.rs
• tests/ui/abi/stack-protector.rs
• tests/ui/stack-protector

History

• add codegen option for using LLVM stack smash protection #84197 (initial implementation)
• Document stack smashing protection #93052 (documentation)
• LLVM 17 regression: tests/ui/abi/stack-protector.rs no longer actually attempts stack-smashing #109671 (regression fix)
• Document stack-protector option #111722 (more documentation)
• Add -Zstack-protector test for Windows targets #116037 (windows test)

Unresolved questions

• This pull request only stabilizes -Zstack-protector=all as -Cstack-protector=all, and does not stabilize -Zstack-protector=basic or -Zstack-protector=strong as these options rely on LLVM heuristics which are not suitable for Rust - we should check that -Zstack-protector=all is definitely okay.
  • cc @nikic

[nikic]

Stabilizing -C stack-protector=all should be fine. It still uses some potentially dubious heuristics for the frame layout, but we don't make specific guarantees about that.

That said, I find this stabilization somewhat dangerous due to the potential "security theater" it can enable. The value of stack protectors in Rust is much more tenuous than in C, and there is a danger that people will follow the logic "I use -fstack-protector for my C code, so I should enable stack protectors in Rust as well -- but it only supports -C stack-protector=all, so I guess I'll just have to use that!" and arrive at a very unfortunate end result.

[davidtwco]

That said, I find this stabilization somewhat dangerous due to the potential "security theater" it can enable. The value of stack protectors in Rust is much more tenuous than in C, and there is a danger that people will follow the logic "I use -fstack-protector for my C code, so I should enable stack protectors in Rust as well -- but it only supports -C stack-protector=all, so I guess I'll just have to use that!" and arrive at a very unfortunate end result.

I agree that this is a risk. I don't think that we should keep this unstable just because of a risk like this though, if this option is useful and appropriate for some users, then we allow it to be used on stable (absent other concerns). If this option is never useful or appropriate, then we should just remove it altogether, even on nightly.

If there is additional context I could add to the documentation for this option about when it is appropriate and when it is not, then I'd be happy to add that.

[nikic]

That said, I find this stabilization somewhat dangerous due to the potential "security theater" it can enable. The value of stack protectors in Rust is much more tenuous than in C, and there is a danger that people will follow the logic "I use -fstack-protector for my C code, so I should enable stack protectors in Rust as well -- but it only supports -C stack-protector=all, so I guess I'll just have to use that!" and arrive at a very unfortunate end result.

I agree that this is a risk. I don't think that we should keep this unstable just because of a risk like this though, if this option is useful and appropriate for some users, then we allow it to be used on stable (absent other concerns). If this option is never useful or appropriate, then we should just remove it altogether, even on nightly.

If there is additional context I could add to the documentation for this option about when it is appropriate and when it is not, then I'd be happy to add that.

Generally agree. Would love to hear from some people who are interested in using -C stack-protector=all to get a better idea on what the actual use case here is.

[rustbot]

Some changes occurred in src/doc/rustc/src/exploit-mitigations.md

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

Some changes occurred in tests/codegen/stack-protector.rs

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

Some changes occurred in tests/ui/stack-protector

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

[davidtwco]

Generally agree. Would love to hear from some people who are interested in using -C stack-protector=all to get a better idea on what the actual use case here is.

See #114903 (comment)

[apiraino]

hey 👋 what are the next steps for this PR? do you need feedback from @nikic ? I saw a mention for @rcvalle either.

thanks

[davidtwco]

I think this is just waiting until we've decided we've gotten enough feedback from users on whether this would be worth it (like this comment).