-
Notifications
You must be signed in to change notification settings - Fork 13k
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 support for full RELRO #43170
Add support for full RELRO #43170
Changes from 3 commits
2306687
94b9cc9
ecf3f6d
6a8328c
2161fb2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,7 +50,7 @@ use std::default::Default; | |
use std::io::prelude::*; | ||
use syntax::abi::{Abi, lookup as lookup_abi}; | ||
|
||
use {LinkerFlavor, PanicStrategy}; | ||
use {LinkerFlavor, PanicStrategy, RelroLevel}; | ||
|
||
mod android_base; | ||
mod apple_base; | ||
|
@@ -367,6 +367,10 @@ pub struct TargetOptions { | |
/// the functions in the executable are not randomized and can be used | ||
/// during an exploit of a vulnerability in any code. | ||
pub position_independent_executables: bool, | ||
/// Either partial, full, or off. Full RELRO makes the dynamic linker | ||
/// resolve all symbols at startup and marks the GOT read-only before | ||
/// starting the program, preventing overwriting the GOT. | ||
pub relro_level: RelroLevel, | ||
/// Format that archives should be emitted in. This affects whether we use | ||
/// LLVM to assemble an archive or fall back to the system linker, and | ||
/// currently only "gnu" is used to fall into LLVM. Unknown strings cause | ||
|
@@ -454,6 +458,7 @@ impl Default for TargetOptions { | |
has_rpath: false, | ||
no_default_libraries: true, | ||
position_independent_executables: false, | ||
relro_level: RelroLevel::Off, | ||
pre_link_objects_exe: Vec::new(), | ||
pre_link_objects_dll: Vec::new(), | ||
post_link_objects: Vec::new(), | ||
|
@@ -580,6 +585,20 @@ impl Target { | |
Some(Ok(())) | ||
})).unwrap_or(Ok(())) | ||
} ); | ||
($key_name:ident, RelroLevel) => ( { | ||
let name = (stringify!($key_name)).replace("_", "-"); | ||
obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| { | ||
match s { | ||
"full" => base.options.$key_name = RelroLevel::Full, | ||
"partial" => base.options.$key_name = RelroLevel::Partial, | ||
"off" => base.options.$key_name = RelroLevel::Off, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this be deduplicated with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was just duplicating the way it was done for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh heh good point! We should deduplicate that one as well at some point... |
||
_ => return Some(Err(format!("'{}' is not a valid value for \ | ||
relro-level. Use 'full', 'partial, or 'off'.", | ||
s))), | ||
} | ||
Some(Ok(())) | ||
})).unwrap_or(Ok(())) | ||
} ); | ||
($key_name:ident, list) => ( { | ||
let name = (stringify!($key_name)).replace("_", "-"); | ||
obj.find(&name[..]).map(|o| o.as_array() | ||
|
@@ -683,6 +702,7 @@ impl Target { | |
key!(has_rpath, bool); | ||
key!(no_default_libraries, bool); | ||
key!(position_independent_executables, bool); | ||
try!(key!(relro_level, RelroLevel)); | ||
key!(archive_format); | ||
key!(allow_asm, bool); | ||
key!(custom_unwind_resume, bool); | ||
|
@@ -870,6 +890,7 @@ impl ToJson for Target { | |
target_option_val!(has_rpath); | ||
target_option_val!(no_default_libraries); | ||
target_option_val!(position_independent_executables); | ||
target_option_val!(relro_level); | ||
target_option_val!(archive_format); | ||
target_option_val!(allow_asm); | ||
target_option_val!(custom_unwind_resume); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this actually start out in the list of debugging options so it goes through the normal channels for stability?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be an issue that the change in default behavior is insta-stable, while the new knob to control it is still unstable? One could use
-Clink-arg=-Wl,-z,norelro
to cancel the first part, but I don't think-z now
has an off switch.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cuviper
-z lazy
is the inverse of-z now
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, here I was looking only for something like
-z nonow
,-z notnow
... :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cuviper yeah that'd be a consequence of changing the defaults. I'd also be fine not changing the defaults and adding this option to eventually get stabilized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind the new default now that I know
-Clink-arg
can still undo it all if needed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a strange place to put it, but sure, I'll move it down to
DebuggingOptions
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kyrias oh sure yeah it's still a "codegen option" but I'd just prefer to have it start out behind
-Z
which is our set of unstable flags, whereas-C
would be "instantly stable"