Rollup merge of #145150 - ChrisDenton:inherit, r=Mark-Simulacrum

Replace unsafe `security_attributes` function with safe `inherit_handle` alternative

The `security_attributes` function is marked as safe despite taking a raw pointer which will later be used. Fortunately this function is only used internally and only in one place that has been basically the same for a decade now. However, we only ever set one bool so it's easy enough to replace with something that's actually safe.

In the future we might want to expose the ability for users to set security attributes. But that should be properly designed (and safe!).

2 files changed, 11 insertions, 12 deletions

diff --git a/library/std/src/sys/fs/windows.rs b/library/std/src/sys/fs/windows.rs
index 9b674a25165..09feddd0be9 100644
--- a/library/std/src/sys/fs/windows.rs
+++ b/library/std/src/sys/fs/windows.rs
@@ -80,7 +80,7 @@ pub struct OpenOptions {
     attributes: u32,
     share_mode: u32,
     security_qos_flags: u32,
-    security_attributes: *mut c::SECURITY_ATTRIBUTES,
+    inherit_handle: bool,
 }
 
 #[derive(Clone, PartialEq, Eq, Debug)]
@@ -203,7 +203,7 @@ impl OpenOptions {
             share_mode: c::FILE_SHARE_READ | c::FILE_SHARE_WRITE | c::FILE_SHARE_DELETE,
             attributes: 0,
             security_qos_flags: 0,
-            security_attributes: ptr::null_mut(),
+            inherit_handle: false,
         }
     }
 
@@ -243,8 +243,8 @@ impl OpenOptions {
         // receive is `SECURITY_ANONYMOUS = 0x0`, which we can't check for later on.
         self.security_qos_flags = flags | c::SECURITY_SQOS_PRESENT;
     }
-    pub fn security_attributes(&mut self, attrs: *mut c::SECURITY_ATTRIBUTES) {
-        self.security_attributes = attrs;
+    pub fn inherit_handle(&mut self, inherit: bool) {
+        self.inherit_handle = inherit;
     }
 
     fn get_access_mode(&self) -> io::Result<u32> {
@@ -307,12 +307,17 @@ impl File {
 
     fn open_native(path: &WCStr, opts: &OpenOptions) -> io::Result<File> {
         let creation = opts.get_creation_mode()?;
+        let sa = c::SECURITY_ATTRIBUTES {
+            nLength: size_of::<c::SECURITY_ATTRIBUTES>() as u32,
+            lpSecurityDescriptor: ptr::null_mut(),
+            bInheritHandle: opts.inherit_handle as c::BOOL,
+        };
         let handle = unsafe {
             c::CreateFileW(
                 path.as_ptr(),
                 opts.get_access_mode()?,
                 opts.share_mode,
-                opts.security_attributes,
+                if opts.inherit_handle { &sa } else { ptr::null() },
                 creation,
                 opts.get_flags_and_attributes(),
                 ptr::null_mut(),
diff --git a/library/std/src/sys/process/windows.rs b/library/std/src/sys/process/windows.rs
index 1ee3fbd285f..f9e15b82475 100644
--- a/library/std/src/sys/process/windows.rs
+++ b/library/std/src/sys/process/windows.rs
@@ -623,16 +623,10 @@ impl Stdio {
             // permissions as well as the ability to be inherited to child
             // processes (as this is about to be inherited).
             Stdio::Null => {
-                let size = size_of::<c::SECURITY_ATTRIBUTES>();
-                let mut sa = c::SECURITY_ATTRIBUTES {
-                    nLength: size as u32,
-                    lpSecurityDescriptor: ptr::null_mut(),
-                    bInheritHandle: 1,
-                };
                 let mut opts = OpenOptions::new();
                 opts.read(stdio_id == c::STD_INPUT_HANDLE);
                 opts.write(stdio_id != c::STD_INPUT_HANDLE);
-                opts.security_attributes(&mut sa);
+                opts.inherit_handle(true);
                 File::open(Path::new(r"\\.\NUL"), &opts).map(|file| file.into_inner())
             }
         }