Fix linux build

Author: MikailBag

src/linux.rs

@@ -24,8 +24,10 @@ use std::{
     fs,
     os::unix::io::IntoRawFd,
     path::PathBuf,
-    sync::atomic::{AtomicI64, Ordering},
-    sync::Arc,
+    sync::{
+        atomic::{AtomicI64, Ordering},
+        Arc,
+    },
     time::Duration,
 };
 
@@ -36,7 +38,7 @@ pub struct LinuxChildProcess {
     stdin: Option<LinuxWritePipe>,
     stdout: Option<LinuxReadPipe>,
     stderr: Option<LinuxReadPipe>,
-    sandbox_ref: LinuxSandbox,
+    sandbox_ref: Arc<LinuxSandbox>,
 
     pid: Pid,
 }

src/linux/sandbox.rs

@@ -74,12 +74,12 @@ struct LinuxSandboxDebugHelper<'a> {
 impl Debug for LinuxSandbox {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         let h = LinuxSandboxDebugHelper {
-            id: &self.0.id,
-            options: &self.0.options,
-            zygote_sock: self.0.zygote_sock.lock().unwrap().as_raw_fd(),
-            zygote_pid: self.0.zygote_pid,
-            watchdog_chan: self.0.watchdog_chan,
-            state: self.0.state.snapshot(),
+            id: &self.id,
+            options: &self.options,
+            zygote_sock: self.zygote_sock.lock().unwrap().as_raw_fd(),
+            zygote_pid: self.zygote_pid,
+            watchdog_chan: self.watchdog_chan,
+            state: self.state.snapshot(),
         };
 
         h.fmt(f)
@@ -89,29 +89,29 @@ impl Debug for LinuxSandbox {
 impl Sandbox for LinuxSandbox {
     type Error = Error;
 
-    fn id(&self) -> String {
-        self.0.id
+    fn id(&self) -> &str {
+        &self.id
     }
 
     fn check_cpu_tle(&self) -> Result<bool, Error> {
         self.poll_state()?;
-        Ok(self.0.state.was_cpu_tle.load(SeqCst))
+        Ok(self.state.was_cpu_tle.load(SeqCst))
     }
 
     fn check_real_tle(&self) -> Result<bool, Error> {
         self.poll_state()?;
-        Ok(self.0.state.was_wall_tle.load(SeqCst))
+        Ok(self.state.was_wall_tle.load(SeqCst))
     }
 
     fn kill(&self) -> Result<(), Error> {
-        jail_common::kill_sandbox(self.0.zygote_pid, &self.0.id, &self.0.cgroup_driver)
+        jail_common::kill_sandbox(self.zygote_pid, &self.id, &self.cgroup_driver)
             .map_err(|err| Error::Io { source: err })?;
         Ok(())
     }
 
     fn resource_usage(&self) -> Result<crate::ResourceUsageData, Error> {
-        let cpu_usage = self.0.cgroup_driver.get_cpu_usage(&self.0.id);
-        let memory_usage = self.0.cgroup_driver.get_memory_usage(&self.0.id);
+        let cpu_usage = self.cgroup_driver.get_cpu_usage(&self.id);
+        let memory_usage = self.cgroup_driver.get_memory_usage(&self.id);
         Ok(crate::ResourceUsageData {
             memory: memory_usage,
             time: Some(cpu_usage),
@@ -142,7 +142,7 @@ impl LinuxSandbox {
                 break;
             }
             for ch in &buf[..num_read] {
-                self.0.state.process_flag(*ch)?;
+                self.state.process_flag(*ch)?;
             }
         }
 
@@ -154,7 +154,7 @@ impl LinuxSandbox {
         settings: &crate::linux::Settings,
         cgroup_driver: Arc<crate::linux::cgroup::Driver>,
     ) -> Result<LinuxSandbox, Error> {
-        let jail_id = jail_common::gen_jail_id();
+        let jail_id = crate::util::gen_jail_id();
         let mut read_end = 0;
         let mut write_end = 0;
         setup_pipe(&mut read_end, &mut write_end)?;
@@ -176,7 +176,7 @@ impl LinuxSandbox {
         };
         let startup_info = zygote::start_zygote(jail_options, &cgroup_driver)?;
 
-        let inner = LinuxSandboxInner {
+        let sandbox = LinuxSandbox {
             id: jail_id,
             options,
             zygote_sock: Mutex::new(startup_info.socket),
@@ -189,7 +189,7 @@ impl LinuxSandbox {
             cgroup_driver,
         };
 
-        Ok(LinuxSandbox(Arc::new(inner)))
+        Ok(sandbox)
     }
 
     pub(crate) unsafe fn spawn_job(
@@ -198,7 +198,7 @@ impl LinuxSandbox {
     ) -> Option<jail_common::JobStartupInfo> {
         let q = jail_common::Query::Spawn(query.job_query.clone());
 
-        let mut sock = self.0.zygote_sock.lock().unwrap();
+        let mut sock = self.zygote_sock.lock().unwrap();
 
         // note that we ignore errors, because zygote can be already killed for some reason
         sock.send(&q).ok();
@@ -211,7 +211,7 @@ impl LinuxSandbox {
 
     pub(crate) unsafe fn poll_job(&self, pid: Pid, timeout: Option<Duration>) -> Option<ExitCode> {
         let q = jail_common::Query::Poll(jail_common::PollQuery { pid, timeout });
-        let mut sock = self.0.zygote_sock.lock().unwrap();
+        let mut sock = self.zygote_sock.lock().unwrap();
         sock.send(&q).ok();
         match sock.recv::<Option<i32>>() {
             Ok(x) => x.map(Into::into),
@@ -222,24 +222,18 @@ impl LinuxSandbox {
 
 impl Drop for LinuxSandbox {
     fn drop(&mut self) {
-        match Arc::get_mut(&mut self.0) {
-            // we are last Sandbox handle, so we can drop it
-            Some(_) => (),
-            // there are other handles, so we must not do anyhing
-            None => return,
-        };
         // Kill all processes.
         if let Err(err) = self.kill() {
             panic!("unable to kill sandbox: {}", err);
         }
         // Remove cgroups.
         if std::env::var("MINION_DEBUG_KEEP_CGROUPS").is_err() {
-            self.0
+            self
                 .cgroup_driver
-                .drop_cgroup(&self.0.id, &["pids", "memory", "cpuacct"]);
+                .drop_cgroup(&self.id, &["pids", "memory", "cpuacct"]);
         }
 
         // Close handles
-        nix::unistd::close(self.0.watchdog_chan).ok();
+        nix::unistd::close(self.watchdog_chan).ok();
     }
 }

src/windows.rs

@@ -38,6 +38,6 @@ impl crate::Backend for WindowsBackend {
         &self,
         options: crate::ChildProcessOptions<Self::Sandbox>,
     ) -> Result<Self::ChildProcess, Self::Error> {
-        todo!()
+        child::WindowsChildProcess::create_process()
     }
 }

src/windows/child.rs

@@ -7,7 +7,10 @@ use std::{
     sync::atomic::{AtomicU32, Ordering::Relaxed},
     time::Duration,
 };
-use winapi::um::{minwinbase::STILL_ACTIVE, winnt::HANDLE};
+use winapi::um::{
+    handleapi::CloseHandle, minwinbase::STILL_ACTIVE, processthreadsapi::PROCESS_INFORMATION,
+    winnt::HANDLE,
+};
 
 #[derive(Debug)]
 pub struct WindowsChildProcess {
@@ -28,7 +31,20 @@ pub struct WindowsChildProcess {
     stderr: Option<ReadPipe>,
 }
 
-impl WindowsChildProcess {}
+impl Drop for WindowsChildProcess {
+    fn drop(&mut self) {
+        unsafe {
+            CloseHandle(self.main_thread);
+            CloseHandle(self.child);
+        }
+    }
+}
+
+impl WindowsChildProcess {
+    pub(in crate::windows) fn create_process() -> Result<Self, Error> {
+        todo!()
+    }
+}
 
 impl crate::ChildProcess for WindowsChildProcess {
     type Error = Error;

src/windows/spawn.rs

@@ -3,19 +3,26 @@ use crate::windows::{
     pipe::{self, ReadPipe, WritePipe},
     Cvt, Error, WindowsSandbox,
 };
-use std::mem::size_of;
-use winapi::um::{
-    minwinbase::SECURITY_ATTRIBUTES,
-    processthreadsapi::{
-        CreateProcessW, DeleteProcThreadAttributeList, InitializeProcThreadAttributeList,
-        UpdateProcThreadAttribute, PROC_THREAD_ATTRIBUTE_LIST,
-    },
-    userenv::{CreateAppContainerProfile, DeleteAppContainerProfile},
-    winbase::{
-        CREATE_UNICODE_ENVIRONMENT, EXTENDED_STARTUPINFO_PRESENT, STARTF_USESTDHANDLES,
-        STARTUPINFOEXW,
+use std::{
+    ffi::{OsStr, OsString},
+    mem::size_of,
+    os::windows::ffi::OsStrExt,
+};
+use winapi::{
+    shared::minwindef::TRUE,
+    um::{
+        minwinbase::SECURITY_ATTRIBUTES,
+        processthreadsapi::{
+            CreateProcessW, DeleteProcThreadAttributeList, InitializeProcThreadAttributeList,
+            UpdateProcThreadAttribute, PROCESS_INFORMATION, PROC_THREAD_ATTRIBUTE_LIST,
+        },
+        userenv::{CreateAppContainerProfile, DeleteAppContainerProfile},
+        winbase::{
+            CREATE_UNICODE_ENVIRONMENT, EXTENDED_STARTUPINFO_PRESENT, STARTF_USESTDHANDLES,
+            STARTUPINFOEXW,
+        },
+        winnt::{HANDLE, SECURITY_CAPABILITIES},
     },
-    winnt::{SECURITY_CAPABILITIES, HANDLE},
 };
 
 pub(in crate::windows) struct Stdio {
@@ -24,16 +31,25 @@ pub(in crate::windows) struct Stdio {
     pub stderr: HANDLE,
 }
 
+pub(in crate::windows) struct ChildParams {
+    pub exe: OsString,
+    /// Does not contain argv[0] - will be prepended automatically.
+    pub argv: Vec<OsString>,
+    pub env: Vec<OsString>,
+    pub cwd: OsString,
+}
+
 // TODO: upstream to winapi: https://github.com/retep998/winapi-rs/pull/933/
 const MAGIC_PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES: usize = 131081;
 
 pub(in crate::windows) fn spawn(
     sandbox: &WindowsSandbox,
     stdio: Stdio,
-) -> Result<WindowsChildProcess, Error> {
+    params: ChildParams,
+) -> Result<PROCESS_INFORMATION, Error> {
     let mut proc_thread_attr_list_storage: Vec<u64>;
     let mut security_capabilities;
-    let startup_info = unsafe {
+    let mut startup_info = unsafe {
         let mut startup_info: STARTUPINFOEXW = std::mem::zeroed();
         let mut proc_thread_attr_list_len = 0;
         Cvt::nonzero(InitializeProcThreadAttributeList(
@@ -72,6 +88,84 @@ pub(in crate::windows) fn spawn(
         startup_info
     };
     let creation_flags = CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT;
+    let mut info: PROCESS_INFORMATION = unsafe { std::mem::zeroed() };
+    unsafe {
+        let application_name: Vec<u16> = params.exe.encode_wide().collect();
+        let mut cmd_line = application_name.clone();
+        for arg in params.argv {
+            quote_arg(&mut cmd_line, &arg);
+        }
+        let (mut env, env_status) = encode_env(&params.env);
+        if let EncodeEnvResult::Partial = env_status {
+            tracing::warn!("skipped zero chars in provided environment");
+        }
+        let cwd: Vec<u16> = params.cwd.encode_wide().collect();
+        Cvt::nonzero(CreateProcessW(
+            application_name.as_ptr(),
+            cmd_line.as_mut_ptr(),
+            // pass null as process attributes to disallow inheritance
+            std::ptr::null_mut(),
+            // same for thread
+            std::ptr::null_mut(),
+            // inherit handles
+            TRUE,
+            creation_flags,
+            env.as_mut_ptr().cast(),
+            cwd.as_ptr(),
+            (&mut startup_info as *mut STARTUPINFOEXW).cast(),
+            &mut info,
+        ))?;
+    }
+    Ok(info)
+}
+
+fn ascii_to_u16(ch: u8) -> u16 {
+    let ch = ch as char;
+    let mut out: u16 = 0;
+    ch.encode_utf16(std::slice::from_mut(&mut out));
+    out
+}
+
+fn quote_arg(out: &mut Vec<u16>, data: &OsStr) {
+    // FIXME incorrectly handles quotes.
+    out.push(ascii_to_u16(b' '));
+    out.push(ascii_to_u16(b'"'));
+    for ch in data.encode_wide() {
+        assert_ne!(ch, ascii_to_u16(b'"'));
+        out.push(ch);
+    }
+
+    out.push(ascii_to_u16(b'"'));
+}
+
+enum EncodeEnvResult {
+    /// Success
+    Ok,
+    /// Partial success: zero chars were skipped
+    Partial,
+}
 
-    todo!()
+/// Returns None if data contains zero char.
+fn encode_env(data: &[OsString]) -> (Vec<u16>, EncodeEnvResult) {
+    let mut res = EncodeEnvResult::Ok;
+    let mut capacity = 1;
+    for item in data {
+        capacity += item.encode_wide().count() + 1;
+    }
+    let mut out = Vec::with_capacity(capacity);
+    for item in data {
+        for char in item.encode_wide() {
+            if char == 0 {
+                res = EncodeEnvResult::Partial;
+                continue;
+            }
+            out.push(char);
+        }
+        out.push(0);
+    }
+    out.push(0);
+    // let's verify capacity was correct
+    debug_assert_eq!(out.capacity(), capacity);
+    debug_assert_eq!(out.len(), capacity);
+    (out, res)
 }