Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rusty stack protector #137418

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

SparrowLii
Copy link
Member

@SparrowLii SparrowLii commented Feb 22, 2025

MCP: rust-lang/compiler-team#841

This draft PR is a prototype for Rust's stack protection option.

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.

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

And changes to GlobalContext need to be avoided. I'm trying to think of a more convenient way than adding a new query.

r? @ghost

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 22, 2025
@@ -1392,6 +1392,8 @@ pub struct GlobalCtxt<'tcx> {
/// Stores memory for globals (statics/consts).
pub(crate) alloc_map: interpret::AllocMap<'tcx>,

pub stack_protector: Lock<FxHashSet<DefId>>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not correct for incremental (it's good enough to collect some data but would have to be fixed before merging). One implementation strategy I would try is to use the same mechanism as the (I forgot how exactly it's called) inferred function attrs thing pcwalton implemented a while ago.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I think so. I'm trying to think of a more convenient way than adding a new query

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I will learn the strategy you mentioned.

@rust-log-analyzer

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants