Proposal for Adapt Stack Protector for Rust

[SparrowLii]

trafficstars

Proposal for Adapt Stack Protector for Rust

Stack smash protection is a requirement for many products in actual production environments.

Although Rust is known for its memory safety, Rust's unsafe code may still cause stack smash risks. In the current industry, many products use Rust/C/C++ interop, which leads to the frequent use of unsafe code. This is the main reason why products have a great demand for Rust's stack smash protection.

There are three modes of stack protection: basic, strong and all. (Tracking issue here). The current status is that the all mode will significantly increase the binary size ( >7% in average) (which maybe affect performance too, needing more test), which is hard to accept for products. So they prefer to use basic/strong mode. But as this issue discussed, this two modes are for C++ and cannot be adapted to Rust.

Therefore, Rust needs to implement its own stack protection mode. Our goal is to define similar checking rules for Rust, referring to the implementation of basic/strong in gcc/clang, and enable the compiler to identify functions that need to be protected.

The following are the initially proposed function check rules for rusty mode that require stack protection in Rust(Reference for clang here):

• calls to stack memory allocation (Although there is no direct function in the Rust standard library that provides this functionality)
• obtaining reference/pointer of local variables

Arrays and references/pointers in Rust are of different types. If you want to use an array to manipulate stack space to cause a buffer overflow, you must first obtain a reference/pointer to it. Therefore, there is no need to specify stack protection rules for arrays in Rust.

By checking each function in the mir layer (because it is convenient to traverse rvalues), we can effectively identify functions that need to perform stack protection and add the corresponding flag in codegen.

Problems that require further discussion：

• How to identify inline scenarios and make corresponding strategy adjustments
• Figure out how the Rust compiler passes large structures (whether it will be optimized to pass by reference)
• How to identify stack memory allocation functions more effectively

prototype PR: https://github.com/rust-lang/rust/pull/137418

Mentors or Reviewers

@rcvalle

Thanks for your help!

Process

The main points of the Major Change Process are as follows:

• [x] File an issue describing the proposal.
• [ ] A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
  • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
  • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
• [ ] Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

Concerns

• lose-debuginfo-data by wesleywiser
• impl-at-mir-level by wesleywiser
• inhibit-opts by wesleywiser

Managed by @rustbot—see help for details.

[rustbot]

[!IMPORTANT] This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

Concerns or objections to the proposal should be discussed on Zulip and formally registered here by adding a comment with the following syntax:

@rustbot concern reason-for-concern 
<description of the concern> 

Concerns can be lifted with:

@rustbot resolve reason-for-concern 

See documentation at https://forge.rust-lang.org

cc @rust-lang/compiler

[rcvalle]

@rustbot second

[wesleywiser]

Recording some objections that should be resolved prior to accepting this MCP:

@rustbot concern impl-at-mir-level

Zulip: doing this on MIR [...] will end up up protecting a lot more functions than necessary, because it happens pre-(LLVM-)inlining, so there will be a lot of addresses taken that later get inlined out.

@rustbot concern inhibit-opts

Zulip: By inserting the stack protectors prior to optimization, we're most likely going to inhibit optimizations

@rustbot concern lose-debuginfo-data

Zulip: Some targets (eg, msvc) record in debuginfo whether stack protectors were enabled or disabled on a per-function basis and this data is used in automated compliance tooling to ensure standard deployment practices are being followed. By inserting stack protectors earlier than LLVM is aware, we lose out on this data being captured correctly.

EDIT: reformatted comment for machine parsing

[SparrowLii]

@wesleywiser Based on the previous discussion, I think these problems have solutions.

• The heuristics need to be reduced. Instead of adding stack protection based on whether a variable reference is obtained, the following behaviors are detected: （1）Converts a local variable reference to a pointer type （2）Passing reference types(ot types contains reference) to other functions: recursively check whether the function has the behavior of converting the reference to a pointer

• This does not affect llvm's optimizations, because we only add a LLVM attribute flag to the function, which is the same as the flag added in current stack-protection=all mode.

• If the current stack-protector=all mode does not have this problem, then this MCP does not have either. If it have, then we can add another flag to inform the debuginfo-data tool.