-
Notifications
You must be signed in to change notification settings - Fork 897
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
Windows library loading #3493
Windows library loading #3493
Changes from all commits
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 |
---|---|---|
|
@@ -29,6 +29,9 @@ use rustup::is_proxyable_tools; | |
use rustup::utils::utils; | ||
|
||
fn main() { | ||
#[cfg(windows)] | ||
pre_rustup_main_init(); | ||
|
||
let process = OSProcess::default(); | ||
with(process.into(), || match maybe_trace_rustup() { | ||
Err(e) => { | ||
|
@@ -163,3 +166,21 @@ fn do_recursion_guard() -> Result<()> { | |
|
||
Ok(()) | ||
} | ||
|
||
/// Windows pre-main security mitigations. | ||
/// | ||
/// This attempts to defend against malicious DLLs that may sit alongside | ||
/// rustup-init in the user's download folder. | ||
#[cfg(windows)] | ||
pub fn pre_rustup_main_init() { | ||
use winapi::um::libloaderapi::{SetDefaultDllDirectories, LOAD_LIBRARY_SEARCH_SYSTEM32}; | ||
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. Maybe orthogonal, but should we start using windows-sys here? 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'm surprised there's not an issue for that already but I do think that's beyond the scope of this PR. |
||
// Default to loading delay loaded DLLs from the system directory. | ||
// For DLLs loaded at load time, this relies on the `delayload` linker flag. | ||
// This is only necessary prior to Windows 10 RS1. See build.rs for details. | ||
unsafe { | ||
let result = SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32); | ||
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. The docs for this function seem to say that it's not directly available on Win7:
So... how did this call work? Are we not setting up the build in a Win7 mode? (until we actually drop that support) 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. Hm, this works in my Win7 VM. Presumably the advice to use But yes, Windows 7 hasn't been tested in CI in a long time. The same goes for all rustlang repos I'm aware of. 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. So the Rust 1.72 blog post had this text:
How do you think those changes (5 months from now) should interact with this code? 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. It won't change anything until rust's minimum supported version is Windows 10 RS1 (aka 1607). I can add a similar comment to the one in the build script and note how they're related. |
||
// SetDefaultDllDirectories should never fail if given valid arguments. | ||
// But just to be safe and to catch mistakes, assert that it succeeded. | ||
assert_ne!(result, 0); | ||
} | ||
} |
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.
AFAICT
delayimp.lib
is not mentioned on the linked page. Do you have a reference for this/why this is necessary?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, see the documentation for the delayload linker option. I've added a comment that links to it.