Tracking issue for -Z control_flow_guard

[ajpaverd]

This is a tracking issue for the flag to enable Windows Control Flow Guard, added in #68180.

[ajpaverd]

The config.toml option to build libstd with Control Flow Guard checks was added in #68824.

[nikomatsakis]

So @rylev reached out to me to ask about stabilization as well as the possibility of enabling this option by default for the stdlib when built on Windows (at least with msvc, I imagine).

I would like to see this move forward, as it seems like a useful option with important safety implications.

What I think we want is the following

• A PR adding user-facing documentation to the unstable book
  • What does the feature do?
  • Benefits, drawbacks of enabling it
  • Links to other tooling and related explanations (LLVM, etc)
• A write-up describing what the background and motivation for this feature; this overlaps somewhat with the user-facing documentation but is probably in more depth:
  • Some links and background on what the flag does
  • Benefits, drawbacks of using it
  • Precedent for how other compilers behave (is this option on by default with msvc? clang? etc? is it used in other major projects?)
  • If we want to argue for making it the default, I would include some arguments for why this makes sense and what the drawbacks might be, and any relevant precedent
• Once we have that, we can nominate the issue and potentially move to FCP (as a user-facing change, I believe this requires an FCP at least; I'd not be opposed to an RFC, but it may not be necessary).

In terms of the decision makers:

• Stabilizing the flag seems like a @rust-lang/compiler decision
• But I'm not sure which team(s) make the decisions about "what options to use when building the standard library". @rust-lang/compiler, @rust-lang/libs, perhaps? But it's not really the "core competency" for either team as I think of it. I'll cc @Mark-Simulacrum (release team) to get their take on that question too. =)

[Mark-Simulacrum]

Yeah, I think that's about the right set of people. In this particular case I imagine we'd treat this similarly to stack guards and so forth on other platforms (at a high level, my understanding is that this is similar, just more pervasive -- related to control flow vs. stack overflows).

With regards to whether or not to stabilize/add this to std's flags I would definitely need to read a write up as you've suggested. :)

[nikomatsakis]

@ajpaverd do you think you'd be up for producing the write-up that I referenced?

[ajpaverd]

@ajpaverd do you think you'd be up for producing the write-up that I referenced?

Yes certainly! I've started working on this already. Thanks for the helpful guidance.

[ajpaverd]

Here are the items requested above:

• Updated documentation for Control Flow Guard #72945 adds documentation about this flag to the unstable book.

• Control Flow Guard for Rust.pdf is a write-up about how CFG is implemented in Rust, benefits and drawbacks of enabling it, and a discussion about CFG in the standard library.

• Control Flow Guard for LLVM.pdf is a detailed explanation of how CFG is implemented in LLVM.

Apologies for the delay on this. Let me know if any further clarification is needed.

[nikomatsakis]

@ajpaverd thanks! For those who are curious, here is a link to the chapter in the unstable book.

Ideally @ajpaverd the implementation documentation would make its way into the rustc-dev-guide, I have to say you went above and beyond the call of duty there, it seems very thorough. =)

[nikomatsakis]

A few minor questions for you @ajpaverd:

• when this is stabilized, would it be -Ccontrol-flow-guard?
• also, do we want control_flow_guard or control-flow-guard?
• also, what happens if you use this on some non-windows target?

I'm basically ready to move to stabilize, just wanted to clarify the naming in particular.

[ajpaverd]

Thanks @nikomatsakis! In answer to your questions:

• Yes, I think this should be a codegen option.
• I think control-flow-guard (with hyphens) would be more similar to existing codegen options right? What do you think of the following syntax (modelled on existing options)?
  -C control-flow-guard=[val] where [val] can be:
  • y, yes, on, checks, or no value: enable Control Flow Guard.
  • nochecks: emit Control Flow Guard metadata without runtime enforcement checks (this should only be used for testing purposes as it does not provide security enforcement).
  • n, no, off: do not enable Control Flow Guard (the default).
• This would be silently ignored on non-Windows targets. The rustc part actually behaves the same on all targets as it is just passing module flags to LLVM. LLVM ignores these flags for non-Windows targets. Does this sound ok or should we have an error/warning?

[nikomatsakis]

I think that all sounds fine.

[nikomatsakis]

Can you prepare a stabilization PR that also makes those changes, @ajpaverd?

Or, alternatively, prepare a PR making those proposed changes but leaving the option unstable (perhaps with -Zunstable-options or whatever) and we can separate out the stability.

I think I'm good either way.

[withoutboats]

Not really a blocker on stabilizing the flag, but I'm curious about the longer term intention. Do we imagine this would be on by default on Windows? Or do we imagine users would opt into it? Do we have a better idea of how to enable users to opt into it than the RUSTFLAGS (i.e. a cargo interface for turning this on)?

I notice that there's an important difference from other runtime protection like stack guards: unlike stack overflows, these bugs should be prevented at compile time by Rust's type system. That doesn't mean defense in depth isn't good, of course, but its a category difference.

[nikomatsakis]

I don't have a strong opinion on those questions, but I do imagine it's the sort of thing we would probably eventually want to expose in Cargo.toml as part of a profile, and I could imagine enabling it by default as part of the "debug build profile" at minimum (I guess that would depend on what the common practices are? I'd probably expect us to match the typical defaults most folks are using).

[retep998]

Rust is not completely free of bugs like this. Unsafe code does exist and people do get it wrong. Also Rust does not exist in a void and links to C code which continues to remain very vulnerable. Therefore my stance is that we absolutely should have all these mitigations enabled by default.

[oribs1]

When building a binary with the stabilized flag, would it still be required to compile the standard library from source, or cargo will know to choose the right version?

[ajpaverd]

When building a binary with the stabilized flag, would it still be required to compile the standard library from source, or cargo will know to choose the right version?

At the moment, getting full protection stills requires compiling the standard library from source (but it will still work normally otherwise). Once stabilized, my suggestion would be that we build the standard library with this enabled by default for windows-msvc targets. Although the standard library would contain the necessary checks and metadata, these would only be used if the actual program is compiled with Control Flow Guard enabled.

[roblabla]

-C control-flow-guard flag was stabilized in 1.47.0, in the form of a flag that can be passed to the compiler. I guess in that sense, this issue should be closed.

However, using this feature is still a bit rough. The libstd distributed via rustup doesn't have the control-flow-guard enabled by default. This means the CFG coverage isn't complete. What are the next steps towards this? Are there any blockers?

Another piece that isn't covered by CFG today is C code built in build.rs through cc-rs. In theory, cc-rs could enable the appropriate flag to make sure the generated code is also protected by CFG, but there is no clear way to detect that control-flow-guard was requested from a build script. See rust-lang/cc-rs#557. Is there any plan to make this kind of integration possible?

[nagisa]

Closing as per the comment above.