From 7fd8d8b0b8cc27a5c590581feec956ab868cad66 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Sat, 1 Aug 2020 01:15:31 +0300 Subject: [PATCH 01/18] Implement windows support --- .github/workflows/rust.yml | 29 ++++ Cargo.lock | 83 ++++++++- Cargo.toml | 19 +++ ci/windows.ps1 | 25 +++ minion-ffi/src/lib.rs | 16 +- minion-tests/Cargo.toml | 8 +- minion-tests/src/tests/simple.rs | 24 ++- minion-tests/src/worker.rs | 1 + rustfmt.toml | 2 +- src/erased.rs | 10 +- src/lib.rs | 31 +++- src/linux.rs | 2 +- src/linux/cgroup/v1.rs | 1 - src/linux/jail_common.rs | 13 -- src/linux/sandbox.rs | 2 +- src/util.rs | 14 ++ src/windows.rs | 45 +++++ src/windows/child.rs | 76 +++++++++ src/windows/constrain.rs | 142 ++++++++++++++++ src/windows/error.rs | 53 ++++++ src/windows/isolate.rs | 80 +++++++++ src/windows/pipe.rs | 137 +++++++++++++++ src/windows/sandbox.rs | 45 +++++ src/windows/spawn.rs | 278 +++++++++++++++++++++++++++++++ src/windows/util.rs | 26 +++ src/windows/wait.rs | 137 +++++++++++++++ 26 files changed, 1267 insertions(+), 32 deletions(-) create mode 100644 ci/windows.ps1 create mode 100644 src/util.rs create mode 100644 src/windows.rs create mode 100644 src/windows/child.rs create mode 100644 src/windows/constrain.rs create mode 100644 src/windows/error.rs create mode 100644 src/windows/isolate.rs create mode 100644 src/windows/pipe.rs create mode 100644 src/windows/sandbox.rs create mode 100644 src/windows/spawn.rs create mode 100644 src/windows/util.rs create mode 100644 src/windows/wait.rs diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index b415f72d..19dcda99 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -101,6 +101,35 @@ jobs: path: /tmp/logs name: tests-trace-${{ matrix.cgroups }}-${{ matrix.os }} + tests-windows: + strategy: + matrix: + rust-target: + - 'x86_64-pc-windows-gnu' + - 'x86_64-pc-windows-msvc' + runs-on: windows-latest + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1.0.6 + with: + toolchain: stable + override: true + - name: Test + run: powershell ci/windows.ps1 + timeout-minutes: 5 + env: + CI_TARGET: ${{ matrix.rust-target }} + - name: Collect logs + if: always() + run: | + mkdir /tmp/logs + cp ./strace* /tmp/logs + - uses: actions/upload-artifact@v1 + if: always() + with: + path: /tmp/logs + name: tests-trace + nightly-checks: runs-on: ubuntu-20.04 steps: diff --git a/Cargo.lock b/Cargo.lock index 633dbdec..40005da3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -33,6 +33,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "ansi_term" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2" +dependencies = [ + "winapi", +] + [[package]] name = "anyhow" version = "1.0.38" @@ -339,6 +348,28 @@ dependencies = [ "regex-automata", ] +[[package]] +name = "loom" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a0e8460f2f2121162705187214720353c517b97bdfb3494c0b1e33d83ebe4bed" +dependencies = [ + "cfg-if 0.1.10", + "generator", + "scoped-tls", + "serde", + "serde_json", +] + +[[package]] +name = "matchers" +version = "0.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f099785f7595cc4b4553a174ce30dd7589ef93391ff414dbb67f62392b9e0ce1" +dependencies = [ + "regex-automata", +] + [[package]] name = "minion" version = "0.1.0" @@ -358,6 +389,7 @@ dependencies = [ "tiny-nix-ipc", "tokio", "tracing", + "winapi", ] [[package]] @@ -391,6 +423,7 @@ dependencies = [ "tempfile", "tokio", "tracing-subscriber", + "winapi", ] [[package]] @@ -441,9 +474,21 @@ dependencies = [ [[package]] name = "nix" -version = "0.19.1" +version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2ccba0cfe4fdf15982d1674c69b1fd80bad427d293849982668dfe454bd61f2" +checksum = "50e4785f2c3b7589a0d0c1dd60285e1188adac4006e8abd6dd578e1567027363" +dependencies = [ + "bitflags", + "cc", + "cfg-if 0.1.10", + "libc", + "void", +] + +[[package]] +name = "nix" +version = "0.19.0" +source = "git+https://github.com/nix-rust/nix#5d2a8c221af121abe73e313bdfce8ae2f4372a83" dependencies = [ "bitflags", "cc", @@ -655,6 +700,31 @@ version = "0.6.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3b181ba2dcf07aaccad5448e8ead58db5b742cf85dfe035e2227f137a539a189" +[[package]] +name = "regex" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38cf2c13ed4745de91a5eb834e11c00bcc3709e773173b2ce4c56c9fbde04b9c" +dependencies = [ + "regex-syntax", +] + +[[package]] +name = "regex-automata" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae1ded71d66a4a97f5e961fd0cb25a5f366a42a41570d16a763a69c092c26ae4" +dependencies = [ + "byteorder", + "regex-syntax", +] + +[[package]] +name = "regex-syntax" +version = "0.6.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b181ba2dcf07aaccad5448e8ead58db5b742cf85dfe035e2227f137a539a189" + [[package]] name = "remove_dir_all" version = "0.5.3" @@ -886,6 +956,15 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "thread_local" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d40c6d1b69745a6ec6fb1ca717914848da4b44ae29d9b3080cbee91d72a69b14" +dependencies = [ + "lazy_static", +] + [[package]] name = "time" version = "0.1.44" diff --git a/Cargo.toml b/Cargo.toml index f1734c32..aff6f82a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,25 @@ futures-util = "0.3.12" tokio = { version = "1.1.0", features = ["net"] } tracing = "0.1.22" itoa = "0.4.7" +serde = { version = "1.0.106", features = ["derive"] } +thiserror = "1.0.19" +anyhow = "1.0.32" +futures-util = "0.3.6" +once_cell = "1.4.1" +backtrace = "0.3.46" + +[target.'cfg(target_os="linux")'.dependencies] +tiny-nix-ipc = { git = "https://github.com/myfreeweb/tiny-nix-ipc", features = ["ser_json", "zero_copy"] } +procfs = "0.7.8" +nix = {git = "https://github.com/nix-rust/nix"} +libc = "0.2.68" +errno = "0.2.5" +serde_json = "1.0.51" +tokio = { version = "0.3.2", features = ["net"] } + +[target.'cfg(target_os="windows")'.dependencies] +winapi = { version = "0.3.9", features = ["std", "processthreadsapi", "jobapi2", "errhandlingapi", + "namedpipeapi", "fileapi", "synchapi", "winnt", "userenv", "handleapi", "securitybaseapi", "sddl"] } [workspace] members = ["minion-ffi", ".", "minion-tests", "minion-cli"] diff --git a/ci/windows.ps1 b/ci/windows.ps1 new file mode 100644 index 00000000..5e95d2e1 --- /dev/null +++ b/ci/windows.ps1 @@ -0,0 +1,25 @@ +$ErrorActionPreference = "Stop" + +Write-Output "::group::Info" +Write-Output "Target: $env:CI_TARGET" +Write-Output "::group::Preparing" +rustup target add $env:CI_TARGET +Write-Output @' +[build] +rustflags=["--cfg", "minion_ci"] +'@ | Out-File -FilePath ./.cargo/config -Encoding 'utf8' + +Write-Output "::group::Compiling tests" +$env:RUSTC_BOOTSTRAP = 1 +cargo build -p minion-tests -Zunstable-options --out-dir=./out --target=$env:CI_TARGET + +if ($LASTEXITCODE -ne 0) { + throw "build failure" +} + +Write-Output "::group::Running tests" +$env:RUST_BACKTRACE = 1 +./out/minion-tests.exe +if ($LASTEXITCODE -ne 0) { + throw "tests failed" +} \ No newline at end of file diff --git a/minion-ffi/src/lib.rs b/minion-ffi/src/lib.rs index fb09328b..56559f04 100644 --- a/minion-ffi/src/lib.rs +++ b/minion-ffi/src/lib.rs @@ -2,7 +2,7 @@ #![cfg_attr(minion_nightly, warn(unsafe_op_in_unsafe_fn))] use minion::{self}; use std::{ - ffi::{CStr, OsStr, OsString}, + ffi::{CStr, OsString}, mem::{self}, os::raw::c_char, sync::Arc, @@ -42,11 +42,23 @@ pub enum WaitOutcome { /// # Safety /// `buf` must be valid, readable pointer +#[cfg(target_os = "linux")] unsafe fn get_string(buf: *const c_char) -> OsString { use std::os::unix::ffi::OsStrExt; let buf = unsafe { CStr::from_ptr(buf) }; let buf = buf.to_bytes(); - let s = OsStr::from_bytes(buf); + let s = std::ffi::OsStr::from_bytes(buf); + s.to_os_string() +} +/// # Safety +/// `buf` must be valid, readable pointer +#[cfg(target_os = "windows")] +unsafe fn get_string(buf: *const c_char) -> OsString { + use std::os::windows::ffi::OsStringExt; + let buf = unsafe { CStr::from_ptr(buf) }; + let buf = buf.to_bytes(); + assert!(buf.len() % 2 == 0); + let s = OsString::from_wide(unsafe { buf.align_to::().1 }); s.to_os_string() } diff --git a/minion-tests/Cargo.toml b/minion-tests/Cargo.toml index 2444acf1..c25149f5 100644 --- a/minion-tests/Cargo.toml +++ b/minion-tests/Cargo.toml @@ -8,7 +8,13 @@ edition = "2018" once_cell = "1.5.2" minion = {path = ".."} clap = "2.33.3" -nix = "0.19.1" tempfile = "3.2.0" tokio = { version = "1.1.0", features = ["macros", "rt"] } tracing-subscriber = "0.2.15" +tracing-subscriber = "0.2.15" + +[target.'cfg(target_os="linux")'.dependencies] +nix = "0.17.0" + +[target.'cfg(target_os="windows")'.dependencies] +winapi = { version = "0.3.9", features = ["processthreadsapi"] } diff --git a/minion-tests/src/tests/simple.rs b/minion-tests/src/tests/simple.rs index 0510a483..d48af979 100644 --- a/minion-tests/src/tests/simple.rs +++ b/minion-tests/src/tests/simple.rs @@ -50,8 +50,18 @@ impl crate::TestCase for TTlFork { (TODO: verify this is not wall-clock time limit)" } fn test(&self) -> ! { + #[cfg(target_os = "linux")] + nix::unistd::fork().unwrap(); + #[cfg(target_os = "winfows")] unsafe { - nix::unistd::fork().unwrap(); + winapi::um::processthreadsapi::CreateThread( + std::ptr::null_mut(), + 0, + exceed_time_limit, + self, + 0, + std::ptr::null_mut(), + ); } exceed_time_limit() } @@ -73,7 +83,12 @@ impl crate::TestCase for TIdle { checks that it still will be killed" } fn test(&self) -> ! { + #[cfg(target_os = "linux")] nix::unistd::sleep(1_000_000_000); + #[cfg(target_os = "windows")] + unsafe { + winapi::um::synchapi::Sleep(1_000_000_000); + }; std::process::exit(0) } fn check(&self, cp: crate::CompletedChild, _: &dyn Sandbox) { @@ -149,6 +164,7 @@ impl crate::TestCase for TSecurity { fn description(&self) -> &'static str { "verifies that isolated program can not make certain bad things" } + #[cfg(target_os = "linux")] fn test(&self) -> ! { // Check we can not read pid1's environment. let err = std::fs::read("/proc/1/environ").unwrap_err(); @@ -166,6 +182,12 @@ impl crate::TestCase for TSecurity { assert!(matches!(err, nix::Error::Sys(nix::errno::Errno::EPERM))); std::process::exit(24) } + + #[cfg(target_os = "windows")] + fn test(&self) -> ! { + std::process::exit(24) + } + fn check(&self, mut cp: crate::CompletedChild, _sb: &dyn Sandbox) { super::assert_exit_code(cp.by_ref(), minion::ExitCode(24)); super::assert_empty(cp.stdout); diff --git a/minion-tests/src/worker.rs b/minion-tests/src/worker.rs index 62750cfa..82de031a 100644 --- a/minion-tests/src/worker.rs +++ b/minion-tests/src/worker.rs @@ -58,6 +58,7 @@ async fn inner_main(test_cases: &[&'static dyn TestCase]) { } pub fn main(test_cases: &[&'static dyn TestCase]) { + tracing_subscriber::fmt().pretty().init(); let rt = tokio::runtime::Builder::new_current_thread() .enable_all() .build() diff --git a/rustfmt.toml b/rustfmt.toml index 07858358..52935601 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -1,2 +1,2 @@ unstable_features = true -imports_granularity = "Crate" \ No newline at end of file +imports_granularity = "Crate" diff --git a/src/erased.rs b/src/erased.rs index 2628e7df..f4b9a39f 100644 --- a/src/erased.rs +++ b/src/erased.rs @@ -18,7 +18,7 @@ pub trait Sandbox: std::fmt::Debug + Send + Sync + 'static { } impl Sandbox for S { - fn id(&self) -> String { + fn id(&self) -> &str { self.id() } fn check_cpu_tle(&self) -> anyhow::Result { @@ -107,7 +107,11 @@ pub type ChildProcessOptions = crate::ChildProcessOptions; /// Returns backend instance pub fn setup() -> anyhow::Result> { - Ok(Box::new(crate::linux::LinuxBackend::new( + #[cfg(target_os = "linux")] + return Ok(Box::new(crate::linux::LinuxBackend::new( crate::linux::Settings::new(), - )?)) + )?)); + + #[cfg(target_os = "windows")] + return Ok(Box::new(crate::windows::WindowsBackend::new())); } diff --git a/src/lib.rs b/src/lib.rs index 1d832739..99a89a4e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,10 +12,14 @@ mod command; #[cfg(target_os = "linux")] pub mod linux; +#[cfg(target_os = "windows")] +pub mod windows; + pub mod erased; mod check; pub use check::{check, CheckResult}; +mod util; use serde::{Deserialize, Serialize}; @@ -94,7 +98,8 @@ pub struct SandboxOptions { } impl SandboxOptions { - fn make_relative<'a>(&self, p: &'a Path) -> &'a Path { + #[cfg(target_os = "linux")] + fn make_relative<'a>(&self, p: &'a std::path::Path) -> &'a std::path::Path { if p.starts_with("/") { p.strip_prefix("/").unwrap() } else { @@ -102,6 +107,7 @@ impl SandboxOptions { } } + #[cfg(target_os = "linux")] fn postprocess(&mut self) { let mut paths = std::mem::replace(&mut self.shared_items, Vec::new()); for x in &mut paths { @@ -114,7 +120,7 @@ impl SandboxOptions { /// Represents highly-isolated sandbox pub trait Sandbox: Debug + Send + Sync + 'static { type Error: StdError + Send + Sync + 'static; - fn id(&self) -> String; + fn id(&self) -> &str; /// Returns true if sandbox exceeded CPU time limit fn check_cpu_tle(&self) -> Result; @@ -164,9 +170,17 @@ impl InputSpecification { /// # Correctness /// See requirements of `handle` + #[cfg(target_os = "linux")] pub fn handle_of(obj: T) -> Self { Self::handle(obj.into_raw_fd() as u64) } + + /// # Safety + /// See requirements of `handle` + #[cfg(target_os = "windows")] + pub unsafe fn handle_of(obj: T) -> Self { + unsafe { Self::handle(obj.into_raw_handle() as u64) } + } } /// Configures stdout and stderr for child @@ -209,9 +223,17 @@ impl OutputSpecification { /// # Correctness /// See requirements of `handle` + #[cfg(target_os = "linux")] pub fn handle_of(obj: T) -> Self { Self::handle(obj.into_raw_fd() as u64) } + + /// # Safety + /// See requirements of `handle` + #[cfg(target_os = "windows")] + pub unsafe fn handle_of(obj: T) -> Self { + unsafe { Self::handle(obj.into_raw_handle() as u64) } + } } #[derive(Debug, Clone)] @@ -238,10 +260,7 @@ pub struct ChildProcessOptions { pub pwd: PathBuf, } -use std::{ - ffi::OsString, - path::{Path, PathBuf}, -}; +use std::{ffi::OsString, path::PathBuf}; /// Child process exit code. #[derive(Debug, Copy, Clone, Eq, PartialEq)] diff --git a/src/linux.rs b/src/linux.rs index e3222579..ffbbd43a 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -122,7 +122,7 @@ fn handle_output_io(spec: OutputSpecification) -> Result<(Option, RawFd), Ok((Some(h_read), f)) } OutputSpecificationData::Ignore => { - let file = fs::File::open("/dev/null")?; + let file = fs::File::create("/dev/null")?; let file = file.into_raw_fd(); let fd = unsafe { libc::dup(file) }; Ok((None, fd)) diff --git a/src/linux/cgroup/v1.rs b/src/linux/cgroup/v1.rs index 9d125ba6..b4d4842a 100644 --- a/src/linux/cgroup/v1.rs +++ b/src/linux/cgroup/v1.rs @@ -4,7 +4,6 @@ use std::{ os::unix::io::{IntoRawFd, RawFd}, path::PathBuf, }; - impl super::Driver { fn v1_write_file( &self, diff --git a/src/linux/jail_common.rs b/src/linux/jail_common.rs index 587876b7..ae76d38e 100644 --- a/src/linux/jail_common.rs +++ b/src/linux/jail_common.rs @@ -20,19 +20,6 @@ pub(crate) struct JailOptions { pub(crate) allow_mount_ns_failure: bool, } -const ID_CHARS: &[u8] = b"qwertyuiopasdfghjklzxcvbnm1234567890"; -const ID_SIZE: usize = 8; - -pub(crate) fn gen_jail_id() -> String { - let mut gen = rand::thread_rng(); - let mut out = Vec::new(); - for _i in 0..ID_SIZE { - let ch = *(ID_CHARS.choose(&mut gen).unwrap()); - out.push(ch); - } - String::from_utf8_lossy(&out[..]).to_string() -} - #[derive(Serialize, Deserialize, Clone, Debug)] pub(crate) struct JobQuery { pub(crate) image_path: PathBuf, diff --git a/src/linux/sandbox.rs b/src/linux/sandbox.rs index c82e2af5..1535f9dd 100644 --- a/src/linux/sandbox.rs +++ b/src/linux/sandbox.rs @@ -153,7 +153,7 @@ impl LinuxSandbox { settings: &crate::linux::Settings, cgroup_driver: Arc, ) -> Result { - 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)?; diff --git a/src/util.rs b/src/util.rs new file mode 100644 index 00000000..11f55730 --- /dev/null +++ b/src/util.rs @@ -0,0 +1,14 @@ +use rand::seq::SliceRandom; + +const ID_CHARS: &[u8] = b"qwertyuiopasdfghjklzxcvbnm1234567890"; +const ID_SIZE: usize = 8; + +pub(crate) fn gen_jail_id() -> String { + let mut gen = rand::thread_rng(); + let mut out = Vec::new(); + for _i in 0..ID_SIZE { + let ch = *(ID_CHARS.choose(&mut gen).unwrap()); + out.push(ch); + } + String::from_utf8_lossy(&out[..]).to_string() +} diff --git a/src/windows.rs b/src/windows.rs new file mode 100644 index 00000000..8c501724 --- /dev/null +++ b/src/windows.rs @@ -0,0 +1,45 @@ +//! Minion on windows + +mod child; +mod constrain; +pub mod error; +mod isolate; +mod pipe; +mod sandbox; +mod spawn; +mod util; +mod wait; + +pub use error::Error; +pub use pipe::{ReadPipe, WritePipe}; +pub use sandbox::WindowsSandbox; + +use error::Cvt; + +/// Minion backend, supporting Windows. +#[derive(Debug)] +pub struct WindowsBackend {} + +impl WindowsBackend { + pub fn new() -> WindowsBackend { + WindowsBackend {} + } +} + +impl crate::Backend for WindowsBackend { + type Error = Error; + type Sandbox = WindowsSandbox; + type ChildProcess = child::WindowsChildProcess; + + fn new_sandbox(&self, options: crate::SandboxOptions) -> Result { + let sandbox = sandbox::WindowsSandbox::create(options)?; + Ok(sandbox) + } + + fn spawn( + &self, + options: crate::ChildProcessOptions, + ) -> Result { + child::WindowsChildProcess::create_process(options) + } +} diff --git a/src/windows/child.rs b/src/windows/child.rs new file mode 100644 index 00000000..ec73d67d --- /dev/null +++ b/src/windows/child.rs @@ -0,0 +1,76 @@ +use crate::windows::{ + spawn::{spawn, ChildParams, Stdio}, + Error, ReadPipe, WindowsSandbox, WritePipe, +}; +use winapi::um::{handleapi::CloseHandle, winnt::HANDLE}; + +#[derive(Debug)] +pub struct WindowsChildProcess { + /// Handle to winapi Process object + child: HANDLE, + + /// Handle to main process of child + main_thread: HANDLE, + + /// Stdin + stdin: Option, + /// Stdout + stdout: Option, + /// Stderr + stderr: Option, +} + +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( + options: crate::ChildProcessOptions, + ) -> Result { + let (stdio, (child_stdin, child_stdout, child_stderr)) = Stdio::make(options.stdio)?; + let info = spawn( + &options.sandbox, + stdio, + ChildParams { + exe: options.path.into(), + argv: options.arguments, + env: options.environment, + cwd: options.pwd.into(), + }, + )?; + + Ok(WindowsChildProcess { + child: info.hProcess, + main_thread: info.hThread, + stdin: child_stdin, + stdout: child_stdout, + stderr: child_stderr, + }) + } +} + +impl crate::ChildProcess for WindowsChildProcess { + type Error = Error; + type PipeIn = WritePipe; + type PipeOut = ReadPipe; + type WaitFuture = crate::windows::wait::WaitFuture; + + fn stdin(&mut self) -> Option { + self.stdin.take() + } + fn stdout(&mut self) -> Option { + self.stdout.take() + } + fn stderr(&mut self) -> Option { + self.stderr.take() + } + fn wait_for_exit(&mut self) -> Result { + todo!() + } +} diff --git a/src/windows/constrain.rs b/src/windows/constrain.rs new file mode 100644 index 00000000..ae21a312 --- /dev/null +++ b/src/windows/constrain.rs @@ -0,0 +1,142 @@ +//! Implements resource limits + +use std::{ffi::OsString, os::windows::ffi::OsStrExt}; + +use crate::{ + windows::{Cvt, Error}, + ResourceUsageData, +}; + +use winapi::um::{ + handleapi::CloseHandle, + jobapi2::{ + AssignProcessToJobObject, CreateJobObjectW, QueryInformationJobObject, + SetInformationJobObject, TerminateJobObject, + }, + winnt::{ + JobObjectBasicAccountingInformation, JobObjectExtendedLimitInformation, + JobObjectLimitViolationInformation, HANDLE, JOBOBJECT_BASIC_ACCOUNTING_INFORMATION, + JOBOBJECT_EXTENDED_LIMIT_INFORMATION, JOBOBJECT_LIMIT_VIOLATION_INFORMATION, + JOB_OBJECT_LIMIT_ACTIVE_PROCESS, JOB_OBJECT_LIMIT_JOB_MEMORY, JOB_OBJECT_LIMIT_JOB_TIME, + JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + }, +}; + +/// Responsible for resource isolation & adding & killing +#[derive(Debug)] +pub(crate) struct Job { + handle: HANDLE, +} + +unsafe impl Send for Job {} +unsafe impl Sync for Job {} + +impl Job { + pub(crate) fn new(jail_id: &str) -> Result { + let name: OsString = format!("minion-sandbox-job-{}", jail_id).into(); + let name: Vec = name.encode_wide().collect(); + let handle = unsafe { + Cvt::nonzero(CreateJobObjectW(std::ptr::null_mut(), name.as_ptr()) as i32)? as HANDLE + }; + Ok(Self { handle }) + } + pub(crate) fn enable_resource_limits( + &mut self, + options: &crate::SandboxOptions, + ) -> Result<(), Error> { + let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = unsafe { std::mem::zeroed() }; + info.JobMemoryLimit = options.memory_limit as usize; + info.BasicLimitInformation.ActiveProcessLimit = options.max_alive_process_count; + unsafe { + *info + .BasicLimitInformation + .PerJobUserTimeLimit + .QuadPart_mut() = (options.cpu_time_limit.as_nanos() / 100) as i64; + }; + info.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_JOB_MEMORY + | JOB_OBJECT_LIMIT_ACTIVE_PROCESS + | JOB_OBJECT_LIMIT_JOB_TIME + // let's make sure sandbox will die if we panic / abort + | JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + unsafe { + Cvt::nonzero(SetInformationJobObject( + self.handle, + JobObjectExtendedLimitInformation, + (&mut info as *mut JOBOBJECT_EXTENDED_LIMIT_INFORMATION).cast(), + sizeof::(), + ))?; + } + Ok(()) + } + pub(crate) fn kill(&self) -> Result<(), Error> { + unsafe { Cvt::nonzero(TerminateJobObject(self.handle, 0xDEADBEEF)).map(|_| ()) } + } + pub(crate) fn add_process(&self, process_handle: HANDLE) -> Result<(), Error> { + unsafe { Cvt::nonzero(AssignProcessToJobObject(self.handle, process_handle)).map(|_| ()) } + } + pub(crate) fn resource_usage(&self) -> Result { + let cpu = unsafe { + let mut info: JOBOBJECT_BASIC_ACCOUNTING_INFORMATION = std::mem::zeroed(); + Cvt::nonzero(QueryInformationJobObject( + self.handle, + JobObjectBasicAccountingInformation, + (&mut info as *mut JOBOBJECT_BASIC_ACCOUNTING_INFORMATION).cast(), + sizeof::(), + std::ptr::null_mut(), + ))?; + + let user_ticks = *info.TotalUserTime.QuadPart() as u64; + let kernel_ticks = *info.TotalKernelTime.QuadPart() as u64; + (user_ticks + kernel_ticks) * 100 + }; + let memory = unsafe { + let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = std::mem::zeroed(); + Cvt::nonzero(QueryInformationJobObject( + self.handle, + JobObjectExtendedLimitInformation, + (&mut info as *mut JOBOBJECT_EXTENDED_LIMIT_INFORMATION).cast(), + sizeof::(), + std::ptr::null_mut(), + ))?; + info.PeakJobMemoryUsed as u64 + }; + + Ok(ResourceUsageData { + time: Some(cpu), + memory: Some(memory), + }) + } + pub(crate) fn check_cpu_tle(&self) -> Result { + unsafe { + let mut info: JOBOBJECT_LIMIT_VIOLATION_INFORMATION = std::mem::zeroed(); + Cvt::nonzero(QueryInformationJobObject( + self.handle, + JobObjectLimitViolationInformation, + (&mut info as *mut JOBOBJECT_LIMIT_VIOLATION_INFORMATION).cast(), + sizeof::(), + std::ptr::null_mut(), + ))?; + let viol = info.ViolationLimitFlags & JOB_OBJECT_LIMIT_JOB_TIME; + Ok(viol != 0) + } + } + + pub(crate) fn check_real_tle(&self) -> Result { + // TODO + Ok(false) + } +} + +impl Drop for Job { + fn drop(&mut self) { + unsafe { + CloseHandle(self.handle); + } + } +} + +fn sizeof() -> u32 { + let sz = std::mem::size_of::(); + assert!(sz <= (u32::max_value() as usize)); + sz as u32 +} diff --git a/src/windows/error.rs b/src/windows/error.rs new file mode 100644 index 00000000..2fbd5b1b --- /dev/null +++ b/src/windows/error.rs @@ -0,0 +1,53 @@ +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("winapi call failed: {errno}")] + Syscall { errno: u32 }, + #[error("hresult call failed: {hresult}")] + Hresult { hresult: i32 }, + #[error("background thread failed")] + BackgroundThreadFailure, +} + +impl From for Error { + fn from(errno: u32) -> Self { + Error::Syscall { errno } + } +} + +impl Error { + pub(crate) fn last() -> Self { + let errno = unsafe { winapi::um::errhandlingapi::GetLastError() }; + if cfg!(debug_assertions) { + tracing::error!(errno = errno, backtrace = ?backtrace::Backtrace::new(), "win32 error"); + } else { + tracing::error!(errno = errno, "win32 error"); + }; + Error::Syscall { errno } + } +} + +/// Helper for checking return values +pub(crate) struct Cvt { + _priv: (), +} + +impl Cvt { + /// checks that operation returned non-zero + pub fn nonzero(ret: i32) -> Result { + if ret != 0 { + Ok(ret) + } else { + Err(Error::last()) + } + } + + /// Checks HRESULT is successful + pub fn hresult(hr: winapi::shared::winerror::HRESULT) -> Result<(), Error> { + if winapi::shared::winerror::SUCCEEDED(hr) { + Ok(()) + } else { + tracing::error!(result = hr, "Unsuccessful HRESULT"); + Err(Error::Hresult { hresult: hr }) + } + } +} diff --git a/src/windows/isolate.rs b/src/windows/isolate.rs new file mode 100644 index 00000000..2b6c765d --- /dev/null +++ b/src/windows/isolate.rs @@ -0,0 +1,80 @@ +//! Implements security restrictions +use crate::windows::{Cvt, Error}; +use std::{ + ffi::OsString, + os::windows::ffi::{OsStrExt, OsStringExt}, +}; +use tracing::instrument; +use winapi::{ + shared::sddl::ConvertSidToStringSidW, + um::{ + securitybaseapi::FreeSid, + userenv::{CreateAppContainerProfile, DeleteAppContainerProfile}, + winbase::LocalFree, + winnt::{PSID, SECURITY_CAPABILITIES}, + }, +}; + +/// Represents one AppContainer +#[derive(Debug)] +pub(crate) struct Profile { + /// Pointer to the Security Identifier (SID) of this container + sid: PSID, +} + +unsafe impl Send for Profile {} +unsafe impl Sync for Profile {} + +impl Profile { + /// Creates new profile. Takes new, unique sandbox_id. + #[instrument(skip(sandbox_id))] + pub(crate) fn new(sandbox_id: &str) -> Result { + tracing::info!(sandbox_id, "creating profile"); + let profile_name = OsString::from(format!("minion-sandbox-appcontainer-{}", sandbox_id)); + let mut profile_name: Vec = profile_name.encode_wide().collect(); + profile_name.push(0); + let profile_name = profile_name.as_ptr(); + let mut sid = std::ptr::null_mut(); + unsafe { + let mut sid_string_repr = std::ptr::null_mut(); + let res = ConvertSidToStringSidW(sid, &mut sid_string_repr); + if res != 0 { + let mut cnt = 0; + while *(sid_string_repr.add(cnt)) == 0 { + cnt += 1; + } + let repr = OsString::from_wide(std::slice::from_raw_parts(sid_string_repr, cnt)); + let repr = repr.to_string_lossy(); + tracing::info!(sid=%repr, "obtained SID"); + LocalFree(sid_string_repr.cast()); + } + } + unsafe { + Cvt::hresult(CreateAppContainerProfile( + profile_name, + profile_name, + profile_name, + std::ptr::null_mut(), + 0, + &mut sid, + ))?; + } + Ok(Profile { sid }) + } + /// Returns `SECURITY_CAPABILITIES` representing this container. + #[instrument(skip(self))] + pub(crate) fn get_security_capabilities(&self) -> SECURITY_CAPABILITIES { + let mut caps: SECURITY_CAPABILITIES = unsafe { std::mem::zeroed() }; + caps.CapabilityCount = 0; + caps.AppContainerSid = self.sid; + caps + } +} + +impl Drop for Profile { + fn drop(&mut self) { + unsafe { + FreeSid(self.sid); + } + } +} diff --git a/src/windows/pipe.rs b/src/windows/pipe.rs new file mode 100644 index 00000000..7f6d1cdd --- /dev/null +++ b/src/windows/pipe.rs @@ -0,0 +1,137 @@ +use crate::windows::{Cvt, Error}; +use std::os::windows::io::{FromRawHandle, IntoRawHandle, RawHandle}; +use winapi::{ + shared::minwindef::TRUE, + um::{ + handleapi::CloseHandle, minwinbase::SECURITY_ATTRIBUTES, namedpipeapi::CreatePipe, + winnt::HANDLE, + }, +}; + +#[derive(Debug)] +pub struct ReadPipe { + handle: HANDLE, +} + +unsafe impl Send for ReadPipe {} +unsafe impl Sync for ReadPipe {} + +impl IntoRawHandle for ReadPipe { + fn into_raw_handle(self) -> RawHandle { + let h = self.handle; + std::mem::forget(self); + h + } +} + +impl FromRawHandle for ReadPipe { + unsafe fn from_raw_handle(handle: RawHandle) -> Self { + ReadPipe { handle } + } +} + +impl std::io::Read for ReadPipe { + fn read(&mut self, mut buf: &mut [u8]) -> std::io::Result { + if buf.len() > i32::max_value() as usize { + buf = &mut buf[..(i32::max_value() as usize)] + } + let mut read_cnt = 0; + let res = unsafe { + winapi::um::fileapi::ReadFile( + self.handle, + buf.as_mut_ptr().cast(), + buf.len() as u32, + &mut read_cnt, + std::ptr::null_mut(), + ) + }; + + if res == 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(read_cnt as usize) + } +} + +impl Drop for ReadPipe { + fn drop(&mut self) { + unsafe { + CloseHandle(self.handle); + } + } +} + +#[derive(Debug)] +pub struct WritePipe { + handle: HANDLE, +} + +unsafe impl Send for WritePipe {} +unsafe impl Sync for WritePipe {} + +impl IntoRawHandle for WritePipe { + fn into_raw_handle(self) -> RawHandle { + let h = self.handle; + std::mem::forget(self); + h + } +} + +impl std::io::Write for WritePipe { + fn write(&mut self, mut buf: &[u8]) -> std::io::Result { + if buf.len() > (i32::max_value() as usize) { + buf = &buf[..(i32::max_value() as usize)]; + } + let mut written_cnt = 0; + let res = unsafe { + winapi::um::fileapi::WriteFile( + self.handle, + buf.as_ptr().cast(), + buf.len() as u32, + &mut written_cnt, + std::ptr::null_mut(), + ) + }; + if res != 0 { + Ok(written_cnt as usize) + } else { + Err(std::io::Error::last_os_error()) + } + } + + fn flush(&mut self) -> std::io::Result<()> { + // no need to flush + Ok(()) + } +} + +impl Drop for WritePipe { + fn drop(&mut self) { + unsafe { + CloseHandle(self.handle); + } + } +} + +pub(in crate::windows) enum InheritKind { + Allow, +} + +pub(in crate::windows) fn make(inherit: InheritKind) -> Result<(ReadPipe, WritePipe), Error> { + let mut read = std::ptr::null_mut(); + let mut write = std::ptr::null_mut(); + unsafe { + let mut security_attributes: SECURITY_ATTRIBUTES = std::mem::zeroed(); + security_attributes.nLength = std::mem::size_of::() as u32; + if matches!(inherit, InheritKind::Allow) { + security_attributes.bInheritHandle = TRUE; + } + Cvt::nonzero(CreatePipe( + &mut read, + &mut write, + &mut security_attributes, + 0, + ))?; + } + Ok((ReadPipe { handle: read }, WritePipe { handle: write })) +} diff --git a/src/windows/sandbox.rs b/src/windows/sandbox.rs new file mode 100644 index 00000000..0e4aa233 --- /dev/null +++ b/src/windows/sandbox.rs @@ -0,0 +1,45 @@ +//! Implements Sandbox on the top of `constrain` and `isolate` +use crate::windows::{constrain::Job, isolate::Profile, Error}; +use tracing::instrument; +#[derive(Debug)] +pub struct WindowsSandbox { + pub(crate) job: Job, + pub(crate) profile: Profile, + id: String, +} + +impl WindowsSandbox { + #[instrument] + pub(in crate::windows) fn create(options: crate::SandboxOptions) -> Result { + let id = crate::util::gen_jail_id(); + let mut job = Job::new(&id)?; + let profile = Profile::new(&id)?; + job.enable_resource_limits(&options)?; + + Ok(Self { job, id, profile }) + } +} + +impl crate::Sandbox for WindowsSandbox { + type Error = Error; + + fn id(&self) -> &str { + todo!() + } + + fn kill(&self) -> Result<(), Self::Error> { + self.job.kill() + } + + fn resource_usage(&self) -> Result { + self.job.resource_usage() + } + + fn check_cpu_tle(&self) -> Result { + self.job.check_cpu_tle() + } + + fn check_real_tle(&self) -> Result { + self.job.check_real_tle() + } +} diff --git a/src/windows/spawn.rs b/src/windows/spawn.rs new file mode 100644 index 00000000..89d06fb9 --- /dev/null +++ b/src/windows/spawn.rs @@ -0,0 +1,278 @@ +use crate::{ + windows::{ + pipe::{self, ReadPipe, WritePipe}, + Cvt, Error, WindowsSandbox, + }, + InputSpecificationData, OutputSpecificationData, +}; +use std::{ + ffi::{OsStr, OsString}, + mem::size_of, + os::windows::{ + ffi::OsStrExt, + io::{FromRawHandle, IntoRawHandle}, + }, +}; +use winapi::{ + shared::{minwindef::TRUE, winerror::ERROR_INSUFFICIENT_BUFFER}, + um::{ + errhandlingapi::GetLastError, + handleapi::{CloseHandle, INVALID_HANDLE_VALUE}, + minwinbase::SECURITY_ATTRIBUTES, + processthreadsapi::{ + CreateProcessW, DeleteProcThreadAttributeList, GetCurrentProcess, + InitializeProcThreadAttributeList, UpdateProcThreadAttribute, PROCESS_INFORMATION, + PROC_THREAD_ATTRIBUTE_LIST, + }, + userenv::{CreateAppContainerProfile, DeleteAppContainerProfile}, + winbase::{ + CreateFileMappingA, CREATE_UNICODE_ENVIRONMENT, EXTENDED_STARTUPINFO_PRESENT, + STARTF_USESTDHANDLES, STARTUPINFOEXW, + }, + winnt::{HANDLE, PAGE_READWRITE, SECURITY_CAPABILITIES}, + }, +}; + +pub(in crate::windows) struct Stdio { + pub stdin: HANDLE, + pub stdout: HANDLE, + pub stderr: HANDLE, +} + +impl Stdio { + fn make_input( + spec: crate::InputSpecificationData, + ) -> Result<(HANDLE, Option), Error> { + match spec { + InputSpecificationData::Handle(h) => Ok((h as HANDLE, None)), + InputSpecificationData::Pipe => { + let (reader, writer) = pipe::make(pipe::InheritKind::Allow)?; + Ok((reader.into_raw_handle(), Some(writer))) + } + InputSpecificationData::Empty => Ok((open_empty_readable_file(), None)), + InputSpecificationData::Null => Ok((-1_i32 as usize as HANDLE, None)), + } + } + + fn make_output( + spec: crate::OutputSpecificationData, + ) -> Result<(HANDLE, Option), Error> { + match spec { + OutputSpecificationData::Handle(h) => Ok((h as HANDLE, None)), + OutputSpecificationData::Pipe => { + let (reader, writer) = pipe::make(pipe::InheritKind::Allow)?; + Ok((writer.into_raw_handle(), Some(reader))) + } + OutputSpecificationData::Null => Ok((-1_i32 as usize as HANDLE, None)), + OutputSpecificationData::Ignore => { + let file = std::fs::File::create("C:\\NUL").map_err(|io_err| Error::Syscall { + errno: io_err.raw_os_error().unwrap_or(-1) as u32, + })?; + let file = file.into_raw_handle(); + let cloned_file = crate::windows::util::duplicate_with_inheritance(file)?; + unsafe { + CloseHandle(file); + } + Ok((cloned_file, None)) + } + OutputSpecificationData::Buffer(sz) => unsafe { + let sz = sz.unwrap_or(usize::max_value()) as u64; + let mmap = CreateFileMappingA( + INVALID_HANDLE_VALUE, + std::ptr::null_mut(), + PAGE_READWRITE, + (sz >> 32) as u32, + sz as u32, + std::ptr::null(), + ); + if mmap.is_null() { + Cvt::nonzero(0)?; + } + let child_side = crate::windows::util::duplicate_with_inheritance(mmap)?; + Ok((child_side, Some(ReadPipe::from_raw_handle(mmap)))) + }, + } + } + + pub(in crate::windows) fn make( + params: crate::StdioSpecification, + ) -> Result< + ( + Self, + (Option, Option, Option), + ), + Error, + > { + let (h_stdin, p_stdin) = Self::make_input(params.stdin.0)?; + let (h_stdout, p_stdout) = Self::make_output(params.stdout.0)?; + let (h_stderr, p_stderr) = Self::make_output(params.stderr.0)?; + Ok(( + Stdio { + stdin: h_stdin, + stdout: h_stdout, + stderr: h_stderr, + }, + (p_stdin, p_stdout, p_stderr), + )) + } +} + +fn open_empty_readable_file() -> HANDLE { + static EMPTY_FILE_HANDLE: once_cell::sync::Lazy = once_cell::sync::Lazy::new(|| { + let (reader, writer) = + pipe::make(pipe::InheritKind::Allow).expect("failed to create a pipe"); + drop(writer); + reader.into_raw_handle() as usize + }); + *EMPTY_FILE_HANDLE as HANDLE +} + +pub(in crate::windows) struct ChildParams { + pub exe: OsString, + /// Does not contain argv[0] - will be prepended automatically. + pub argv: Vec, + pub env: Vec, + 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, + params: ChildParams, +) -> Result { + let mut proc_thread_attr_list_storage: Vec; + let mut security_capabilities; + let mut startup_info = unsafe { + let mut startup_info: STARTUPINFOEXW = std::mem::zeroed(); + let mut proc_thread_attr_list_len = 0; + { + InitializeProcThreadAttributeList( + std::ptr::null_mut(), + // we need only one attribute: security capabilities. + 1, + 0, + &mut proc_thread_attr_list_len, + ); + if GetLastError() != ERROR_INSUFFICIENT_BUFFER { + return Err(Error::last()); + } + } + proc_thread_attr_list_storage = Vec::with_capacity((proc_thread_attr_list_len - 1) / 8 + 1); + let proc_thread_attr_list: *mut u8 = proc_thread_attr_list_storage.as_mut_ptr().cast(); + proc_thread_attr_list.write_bytes(0, proc_thread_attr_list_len); + startup_info.lpAttributeList = proc_thread_attr_list.cast(); + Cvt::nonzero(InitializeProcThreadAttributeList( + startup_info.lpAttributeList, + 1, + 0, + &mut proc_thread_attr_list_len, + ))?; + security_capabilities = sandbox.profile.get_security_capabilities(); + Cvt::nonzero(UpdateProcThreadAttribute( + startup_info.lpAttributeList, + 0, + MAGIC_PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES, + (&mut security_capabilities as *mut SECURITY_CAPABILITIES).cast(), + std::mem::size_of::(), + std::ptr::null_mut(), + std::ptr::null_mut(), + ))?; + + startup_info.StartupInfo.cb = size_of::() as u32; + startup_info.StartupInfo.dwFlags = STARTF_USESTDHANDLES; + startup_info.StartupInfo.hStdInput = stdio.stdin; + startup_info.StartupInfo.hStdOutput = stdio.stdout; + startup_info.StartupInfo.hStdError = stdio.stderr; + 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 = 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(¶ms.env); + if let EncodeEnvResult::Partial = env_status { + tracing::warn!("skipped zero chars in provided environment"); + } + let cwd: Vec = 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, + // TEMP DEBUG + std::ptr::null_mut(), + //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, 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'"')); +} + +#[derive(Eq, PartialEq)] +enum EncodeEnvResult { + /// Success + Ok, + /// Partial success: zero chars were skipped + Partial, +} + +/// Returns None if data contains zero char. +fn encode_env(data: &[OsString]) -> (Vec, 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); + if res == EncodeEnvResult::Ok { + debug_assert_eq!(out.len(), capacity); + } + (out, res) +} diff --git a/src/windows/util.rs b/src/windows/util.rs new file mode 100644 index 00000000..9072d54d --- /dev/null +++ b/src/windows/util.rs @@ -0,0 +1,26 @@ +use winapi::{ + shared::minwindef::TRUE, + um::{ + handleapi::DuplicateHandle, + processthreadsapi::GetCurrentProcess, + winnt::{DUPLICATE_SAME_ACCESS, HANDLE}, + }, +}; + +use crate::windows::{Cvt, Error}; + +pub(in crate::windows) fn duplicate_with_inheritance(handle: HANDLE) -> Result { + let mut cloned_handle = std::ptr::null_mut(); + unsafe { + Cvt::nonzero(DuplicateHandle( + GetCurrentProcess(), + handle, + GetCurrentProcess(), + &mut cloned_handle, + 0, + TRUE, + DUPLICATE_SAME_ACCESS, + ))?; + } + Ok(cloned_handle) +} diff --git a/src/windows/wait.rs b/src/windows/wait.rs new file mode 100644 index 00000000..fa8f48f9 --- /dev/null +++ b/src/windows/wait.rs @@ -0,0 +1,137 @@ +//! Implements windows WaitFuture +use crate::windows::{Cvt, Error}; +use futures_util::task::AtomicWaker; +use std::{ + pin::Pin, + sync::{ + atomic::{ + AtomicBool, AtomicU32, + Ordering::{Acquire, Relaxed, Release}, + }, + Arc, + }, + task::{Context, Poll}, +}; +use winapi::{ + shared::winerror::WAIT_TIMEOUT, + um::{ + minwinbase::STILL_ACTIVE, + processthreadsapi::{GetExitCodeProcess, GetProcessId}, + synchapi::WaitForSingleObject, + winbase::WAIT_OBJECT_0, + winnt::HANDLE, + }, +}; + +/// Resolves when child has finished +pub struct WaitFuture { + /// Child handle + child: HANDLE, + /// None if background thread has not been started yet + shared: Option>, +} + +// HANDLE is Send-able... +unsafe impl Send for WaitFuture {} +// ..and Sync +unsafe impl Sync for WaitFuture {} + +impl WaitFuture { + fn get_exit_code(&self) -> Result, Error> { + let mut exit_code = 0; + unsafe { + Cvt::nonzero(GetExitCodeProcess(self.child, &mut exit_code))?; + } + if exit_code == STILL_ACTIVE { + return Ok(None); + } + Ok(Some(crate::ExitCode(exit_code.into()))) + } +} + +impl std::future::Future for WaitFuture { + type Output = Result; + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + let this = Pin::into_inner(self); + + if this.shared.is_none() { + // we should start background thread + + let shared = Shared { + waker: AtomicWaker::new(), + error: AtomicBool::new(false), + }; + + // immediately register before starting background thread + shared.waker.register(cx.waker()); + + let shared = Arc::new(shared); + this.shared.replace(shared.clone()); + + let thread_name = unsafe { + format!( + "minion-background-wait-{}", + Cvt::nonzero(GetProcessId(this.child) as i32).unwrap_or(-1) + ) + }; + + let child_handle = this.child.clone() as usize; + + std::thread::Builder::new() + .name(thread_name) + .spawn(move || background_waiter(shared, child_handle as HANDLE)) + .expect("Failed to create a thread"); + } + + let shared = this.shared.as_mut().expect("initialized upper"); + + // now register in the waker + shared.waker.register(cx.waker()); + + // win path + if shared.error.load(Acquire) { + return Poll::Ready(Err(Error::BackgroundThreadFailure)); + } + + if let Some(ec) = this.get_exit_code().transpose() { + return Poll::Ready(ec); + } + + Poll::Pending + } +} + +struct Shared { + /// Task waiting for finish + waker: AtomicWaker, + /// Set when wait has errored + error: AtomicBool, +} + +fn background_waiter(mut shared: Arc, handle: HANDLE) { + loop { + // check if someone is still interested in our work + if Arc::get_mut(&mut shared).is_some() { + // only our Shared part is alive. + // it means WaitFuture is no longer alive. + return; + } + + // wait for one second + let res = unsafe { WaitForSingleObject(handle, 1000) }; + if res == WAIT_OBJECT_0 { + shared.waker.wake(); + return; + } + if res == WAIT_TIMEOUT { + continue; + } + tracing::error!( + return_value = res, + "Unexpected return from WaitForSingleObject", + ); + shared.error.store(true, Release); + shared.waker.wake(); + return; + } +} From 03006085aaba43ee68acc1f5205c326d3a6eecad Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 4 Nov 2020 19:24:18 +0300 Subject: [PATCH 02/18] procthreadattrlisf no --- .github/workflows/rust.yml | 10 +++++--- ci/windows.ps1 | 2 +- minion-tests/src/master.rs | 51 ++++++++++++++++++++++++++++++++++---- minion-tests/src/worker.rs | 5 +++- src/windows/spawn.rs | 41 ++++++++++++++++++++++++------ src/windows/wait.rs | 4 +-- 6 files changed, 93 insertions(+), 20 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 19dcda99..7c5159ff 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -122,13 +122,15 @@ jobs: - name: Collect logs if: always() run: | - mkdir /tmp/logs - cp ./strace* /tmp/logs + ls + mkdir D:/ci-logs + cp ./strace* D:/ci-logs + ls D:/ci-logs - uses: actions/upload-artifact@v1 if: always() with: - path: /tmp/logs - name: tests-trace + path: D:/ci-logs + name: tests-ntcalls nightly-checks: runs-on: ubuntu-20.04 diff --git a/ci/windows.ps1 b/ci/windows.ps1 index 5e95d2e1..c2691e8f 100644 --- a/ci/windows.ps1 +++ b/ci/windows.ps1 @@ -19,7 +19,7 @@ if ($LASTEXITCODE -ne 0) { Write-Output "::group::Running tests" $env:RUST_BACKTRACE = 1 -./out/minion-tests.exe +./out/minion-tests.exe --trace if ($LASTEXITCODE -ne 0) { throw "tests failed" } \ No newline at end of file diff --git a/minion-tests/src/master.rs b/minion-tests/src/master.rs index a83ac65e..6683ae96 100644 --- a/minion-tests/src/master.rs +++ b/minion-tests/src/master.rs @@ -73,12 +73,53 @@ fn execute_tests(test_cases: &[&dyn TestCase], exec_opts: ExecuteOptions) -> Out fn execute_single_test(case: &dyn TestCase, exec_opts: ExecuteOptions) -> Outcome { println!("------ {} ------", case.name()); let self_exe = std::env::current_exe().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let mut temp_logs_dir = None; let mut cmd = if exec_opts.trace { - let mut cmd = std::process::Command::new("strace"); - cmd.arg("-f"); // follow forks - cmd.arg("-o").arg(format!("strace-log-{}.txt", case.name())); - cmd.arg(self_exe); - cmd + if cfg!(target_os = "linux") { + let mut cmd = std::process::Command::new("strace"); + cmd.arg("-f"); // follow forks + cmd.arg("-o").arg(format!("strace-log-{}.txt", case.name())); + cmd.arg(self_exe); + cmd + } else if cfg!(target_os = "windows") { + let mut cmd = std::process::Command::new( + "C:\\Program Files (x86)\\Windows Kits\\10\\Debuggers\\x64\\cdb.exe", + ); + // disable debug heap + cmd.arg("-hd"); + // follow children + cmd.arg("-o"); + + let script_file_path = tmp.path().join("cdb-script.txt"); + let logs_path: std::path::PathBuf = format!(".\\strace-ntapi-{}", case.name()).into(); + std::fs::create_dir(&logs_path).unwrap(); + println!("Saving logs to {}", logs_path.display()); + std::fs::write( + &script_file_path, + [ + &format!("!logexts.loge {}", logs_path.display()), + "!logexts.logc e *", + "!logexts.logo e d", + "!logexts.logo e t", + "!logexts.logo e v", + "g", + "!logexts.logb f", + "q", + ] + .join("\n"), + ) + .unwrap(); + + temp_logs_dir.replace(logs_path); + + cmd.arg("-cf").arg(&script_file_path); + + cmd.arg(self_exe); + cmd + } else { + panic!("--trace unsupported on this target") + } } else { std::process::Command::new(self_exe) }; diff --git a/minion-tests/src/worker.rs b/minion-tests/src/worker.rs index 82de031a..51a48a1c 100644 --- a/minion-tests/src/worker.rs +++ b/minion-tests/src/worker.rs @@ -58,7 +58,10 @@ async fn inner_main(test_cases: &[&'static dyn TestCase]) { } pub fn main(test_cases: &[&'static dyn TestCase]) { - tracing_subscriber::fmt().pretty().init(); + tracing_subscriber::fmt() + .pretty() + .with_writer(std::io::stderr) + .init(); let rt = tokio::runtime::Builder::new_current_thread() .enable_all() .build() diff --git a/src/windows/spawn.rs b/src/windows/spawn.rs index 89d06fb9..5c42511f 100644 --- a/src/windows/spawn.rs +++ b/src/windows/spawn.rs @@ -138,12 +138,38 @@ pub(in crate::windows) struct ChildParams { // TODO: upstream to winapi: https://github.com/retep998/winapi-rs/pull/933/ const MAGIC_PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES: usize = 131081; +struct AlignedMemBlock(*mut u8, usize); + +impl AlignedMemBlock { + fn layout(cnt: usize) -> std::alloc::Layout { + assert!(cnt > 0); + std::alloc::Layout::from_size_align(cnt, 8).unwrap() + } + + fn new(cnt: usize) -> AlignedMemBlock { + let ptr = unsafe { std::alloc::alloc_zeroed(Self::layout(cnt)) }; + AlignedMemBlock(ptr, cnt) + } + + fn ptr(&self) -> *mut u8 { + self.0 + } +} + +impl Drop for AlignedMemBlock { + fn drop(&mut self) { + unsafe { + std::alloc::dealloc(self.0, Self::layout(self.1)); + } + } +} + pub(in crate::windows) fn spawn( sandbox: &WindowsSandbox, stdio: Stdio, params: ChildParams, ) -> Result { - let mut proc_thread_attr_list_storage: Vec; + let proc_thread_attr_list_storage; let mut security_capabilities; let mut startup_info = unsafe { let mut startup_info: STARTUPINFOEXW = std::mem::zeroed(); @@ -160,9 +186,8 @@ pub(in crate::windows) fn spawn( return Err(Error::last()); } } - proc_thread_attr_list_storage = Vec::with_capacity((proc_thread_attr_list_len - 1) / 8 + 1); - let proc_thread_attr_list: *mut u8 = proc_thread_attr_list_storage.as_mut_ptr().cast(); - proc_thread_attr_list.write_bytes(0, proc_thread_attr_list_len); + proc_thread_attr_list_storage = AlignedMemBlock::new(proc_thread_attr_list_len); + let proc_thread_attr_list = proc_thread_attr_list_storage.ptr(); startup_info.lpAttributeList = proc_thread_attr_list.cast(); Cvt::nonzero(InitializeProcThreadAttributeList( startup_info.lpAttributeList, @@ -173,11 +198,14 @@ pub(in crate::windows) fn spawn( security_capabilities = sandbox.profile.get_security_capabilities(); Cvt::nonzero(UpdateProcThreadAttribute( startup_info.lpAttributeList, + // reserved 0, MAGIC_PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES, (&mut security_capabilities as *mut SECURITY_CAPABILITIES).cast(), std::mem::size_of::(), + // reserved std::ptr::null_mut(), + // reserved std::ptr::null_mut(), ))?; @@ -211,13 +239,12 @@ pub(in crate::windows) fn spawn( // inherit handles TRUE, creation_flags, - // TEMP DEBUG - std::ptr::null_mut(), - //env.as_mut_ptr().cast(), + env.as_mut_ptr().cast(), cwd.as_ptr(), (&mut startup_info as *mut STARTUPINFOEXW).cast(), &mut info, ))?; + DeleteProcThreadAttributeList(startup_info.lpAttributeList); } Ok(info) } diff --git a/src/windows/wait.rs b/src/windows/wait.rs index fa8f48f9..1c3ba13e 100644 --- a/src/windows/wait.rs +++ b/src/windows/wait.rs @@ -5,8 +5,8 @@ use std::{ pin::Pin, sync::{ atomic::{ - AtomicBool, AtomicU32, - Ordering::{Acquire, Relaxed, Release}, + AtomicBool, + Ordering::{Acquire, Release}, }, Arc, }, From bca89a282a738c1df7b5599c856a30a2f46fd3ff Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Tue, 9 Feb 2021 20:35:35 +0300 Subject: [PATCH 03/18] fixup cargo config after rebase --- Cargo.lock | 289 +++++++++++---------------------------- Cargo.toml | 29 ++-- minion-tests/Cargo.toml | 1 - src/erased.rs | 2 +- src/lib.rs | 2 +- src/linux/jail_common.rs | 1 - 6 files changed, 95 insertions(+), 229 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 40005da3..d3edbe07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -33,15 +33,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "ansi_term" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2" -dependencies = [ - "winapi", -] - [[package]] name = "anyhow" version = "1.0.38" @@ -87,9 +78,9 @@ checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" [[package]] name = "byteorder" -version = "1.3.4" +version = "1.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "08c48aae112d48ed9f069b33538ea9e3e90aa263cfa3d1c24309612b1f7472de" +checksum = "ae44d1a3d5a19df61dd0c8beb138458ac2a53a7ac09eba97d55592540004306b" [[package]] name = "cbindgen" @@ -101,19 +92,19 @@ dependencies = [ "indexmap", "log", "proc-macro2 1.0.24", - "quote 1.0.7", + "quote 1.0.8", "serde", "serde_json", - "syn 1.0.59", + "syn 1.0.60", "tempfile", "toml", ] [[package]] name = "cc" -version = "1.0.61" +version = "1.0.66" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed67cbde08356238e75fc4656be4749481eeffb09e19f320a25237d5221c985d" +checksum = "4c0496836a84f8d0495758516b8621a622beb77c0fed418570e50764093ced48" [[package]] name = "cfg-if" @@ -200,8 +191,8 @@ checksum = "c287d25add322d9f9abdcdc5927ca398917996600182178774032e9f8258fedd" dependencies = [ "proc-macro-hack", "proc-macro2 1.0.24", - "quote 1.0.7", - "syn 1.0.59", + "quote 1.0.8", + "syn 1.0.60", ] [[package]] @@ -235,19 +226,6 @@ version = "0.3.55" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f5f3913fa0bfe7ee1fd8248b6b9f42a5af4b9d65ec2dd2c3c26132b950ecfc2" -[[package]] -name = "generator" -version = "0.6.23" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8cdc09201b2e8ca1b19290cf7e65de2246b8e91fb6874279722189c4de7b94dc" -dependencies = [ - "cc", - "libc", - "log", - "rustc_version", - "winapi", -] - [[package]] name = "getrandom" version = "0.2.2" @@ -273,18 +251,18 @@ checksum = "d7afe4a420e3fe79967a00898cc1f4db7c8a49a9333a29f8a4bd76a253d5cd04" [[package]] name = "heck" -version = "0.3.1" +version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "20564e78d53d2bb135c343b3f47714a56af2061f1c928fdb541dc7b9fdd94205" +checksum = "87cbf45460356b7deeb5e3415b5563308c0a9b057c85e12b06ad551f98d0a6ac" dependencies = [ "unicode-segmentation", ] [[package]] name = "hermit-abi" -version = "0.1.17" +version = "0.1.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5aca5565f760fb5b220e499d72710ed156fdb74e631659e99377d9ebfbd13ae8" +checksum = "322f4de77956e22ed0e5032c359a0f1273f1f7f0d79bfa3b8ffbc730d7fbcc5c" dependencies = [ "libc", ] @@ -313,52 +291,17 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" -version = "0.2.82" +version = "0.2.86" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89203f3fba0a3795506acaad8ebce3c80c0af93f994d5a1d7a0b1eeb23271929" +checksum = "b7282d924be3275cec7f6756ff4121987bc6481325397dde6ba3e7802b1a8b1c" [[package]] name = "log" -version = "0.4.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4fabed175da42fed1fa0746b0ea71f412aa9d35e76e95e59b192c64b9dc2bf8b" -dependencies = [ - "cfg-if 0.1.10", -] - -[[package]] -name = "loom" -version = "0.3.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a0e8460f2f2121162705187214720353c517b97bdfb3494c0b1e33d83ebe4bed" -dependencies = [ - "cfg-if 0.1.10", - "generator", - "scoped-tls", - "serde", - "serde_json", -] - -[[package]] -name = "matchers" -version = "0.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f099785f7595cc4b4553a174ce30dd7589ef93391ff414dbb67f62392b9e0ce1" -dependencies = [ - "regex-automata", -] - -[[package]] -name = "loom" -version = "0.3.6" +version = "0.4.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a0e8460f2f2121162705187214720353c517b97bdfb3494c0b1e33d83ebe4bed" +checksum = "51b9bbe6c47d51fc3e1a9b945965946b4c44142ab8792c50835a980d362c2710" dependencies = [ - "cfg-if 0.1.10", - "generator", - "scoped-tls", - "serde", - "serde_json", + "cfg-if 1.0.0", ] [[package]] @@ -380,7 +323,7 @@ dependencies = [ "futures-util", "itoa", "libc", - "nix 0.19.1", + "nix 0.19.0", "once_cell", "rand", "serde", @@ -418,7 +361,7 @@ version = "0.1.0" dependencies = [ "clap", "minion", - "nix 0.19.1", + "nix 0.17.0", "once_cell", "tempfile", "tokio", @@ -488,7 +431,7 @@ dependencies = [ [[package]] name = "nix" version = "0.19.0" -source = "git+https://github.com/nix-rust/nix#5d2a8c221af121abe73e313bdfce8ae2f4372a83" +source = "git+https://github.com/nix-rust/nix#3b8180c430fe838e4fd71b83e5f92db6386e5c57" dependencies = [ "bitflags", "cc", @@ -507,9 +450,9 @@ dependencies = [ [[package]] name = "num-integer" -version = "0.1.43" +version = "0.1.44" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8d59457e662d541ba17869cf51cf177c0b5f0cbf476c66bdc90bf1edac4f875b" +checksum = "d2cc698a63b549a70bc047073d2949cce27cd1c7b0a4a862d08a8031bc2801db" dependencies = [ "autocfg", "num-traits", @@ -517,9 +460,9 @@ dependencies = [ [[package]] name = "num-traits" -version = "0.2.12" +version = "0.2.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac267bcc07f48ee5f8935ab0d24f316fb722d7a1292e2913f0cc196b29ffd611" +checksum = "9a64b1ec5cda2586e284722486d802acf1f7dbdc623e2bfc57e65ca1cd099290" dependencies = [ "autocfg", ] @@ -550,9 +493,9 @@ checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" [[package]] name = "ppv-lite86" -version = "0.2.9" +version = "0.2.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c36fa947111f5c62a733b652544dd0016a43ce89619538a8ef92724a6f501a20" +checksum = "ac74c624d6b2d21f425f752262f42188365d7b8ff1aff74c82e45136510a4857" [[package]] name = "proc-macro-error" @@ -562,8 +505,8 @@ checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c" dependencies = [ "proc-macro-error-attr", "proc-macro2 1.0.24", - "quote 1.0.7", - "syn 1.0.59", + "quote 1.0.8", + "syn 1.0.60", "version_check", ] @@ -574,7 +517,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869" dependencies = [ "proc-macro2 1.0.24", - "quote 1.0.7", + "quote 1.0.8", "version_check", ] @@ -586,9 +529,9 @@ checksum = "dbf0c48bc1d91375ae5c3cd81e3722dff1abcf81a30960240640d223f59fe0e5" [[package]] name = "proc-macro-nested" -version = "0.1.6" +version = "0.1.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eba180dafb9038b050a4c280019bbedf9f2467b61e5d892dcad585bb57aadc5a" +checksum = "bc881b2c22681370c6a780e47af9840ef841837bc98118431d4e1868bd0c1086" [[package]] name = "proc-macro2" @@ -619,18 +562,18 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.7" +version = "1.0.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa563d17ecb180e500da1cfd2b028310ac758de548efdd203e18f283af693f37" +checksum = "991431c3519a3f36861882da93630ce66b52918dcf1b8e2fd66b397fc96f28df" dependencies = [ "proc-macro2 1.0.24", ] [[package]] name = "rand" -version = "0.8.2" +version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "18519b42a40024d661e1714153e9ad0c3de27cd495760ceb09710920f1098b1e" +checksum = "0ef9e7e66b4468674bfcb0c81af8b7fa0bb154fa9f28eb840da5c447baeb8d7e" dependencies = [ "libc", "rand_chacha", @@ -677,34 +620,9 @@ dependencies = [ [[package]] name = "regex" -version = "1.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "38cf2c13ed4745de91a5eb834e11c00bcc3709e773173b2ce4c56c9fbde04b9c" -dependencies = [ - "regex-syntax", -] - -[[package]] -name = "regex-automata" -version = "0.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae1ded71d66a4a97f5e961fd0cb25a5f366a42a41570d16a763a69c092c26ae4" -dependencies = [ - "byteorder", - "regex-syntax", -] - -[[package]] -name = "regex-syntax" -version = "0.6.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b181ba2dcf07aaccad5448e8ead58db5b742cf85dfe035e2227f137a539a189" - -[[package]] -name = "regex" -version = "1.4.2" +version = "1.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "38cf2c13ed4745de91a5eb834e11c00bcc3709e773173b2ce4c56c9fbde04b9c" +checksum = "d9251239e129e16308e70d853559389de218ac275b515068abc96829d05b948a" dependencies = [ "regex-syntax", ] @@ -721,9 +639,9 @@ dependencies = [ [[package]] name = "regex-syntax" -version = "0.6.21" +version = "0.6.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b181ba2dcf07aaccad5448e8ead58db5b742cf85dfe035e2227f137a539a189" +checksum = "b5eb417147ba9860a96cfe72a0b93bf88fee1744b5636ec99ab20c1aa9376581" [[package]] name = "remove_dir_all" @@ -736,18 +654,9 @@ dependencies = [ [[package]] name = "rustc-demangle" -version = "0.1.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2610b7f643d18c87dff3b489950269617e6601a51f1f05aa5daefee36f64f0b" - -[[package]] -name = "rustc_version" -version = "0.2.3" +version = "0.1.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a" -dependencies = [ - "semver", -] +checksum = "6e3bad0ee36814ca07d7968269dd4b7ec89ec2da10c4bb613928d3077083c232" [[package]] name = "ryu" @@ -755,52 +664,31 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "71d301d4193d031abdd79ff7e3dd721168a9572ef3fe51a1517aba235bd8f86e" -[[package]] -name = "scoped-tls" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ea6a9290e3c9cf0f18145ef7ffa62d68ee0bf5fcd651017e586dc7fd5da448c2" - -[[package]] -name = "semver" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d7eb9ef2c18661902cc47e535f9bc51b78acd254da71d375c2f6720d9a40403" -dependencies = [ - "semver-parser", -] - -[[package]] -name = "semver-parser" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" - [[package]] name = "serde" -version = "1.0.121" +version = "1.0.123" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6159e3c76cab06f6bc466244d43b35e77e9500cd685da87620addadc2a4c40b1" +checksum = "92d5161132722baa40d802cc70b15262b98258453e85e5d1d365c757c73869ae" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.121" +version = "1.0.123" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3fcab8778dc651bc65cfab2e4eb64996f3c912b74002fb379c94517e1f27c46" +checksum = "9391c295d64fc0abb2c556bad848f33cb8296276b1ad2677d1ae1ace4f258f31" dependencies = [ "proc-macro2 1.0.24", - "quote 1.0.7", - "syn 1.0.59", + "quote 1.0.8", + "syn 1.0.60", ] [[package]] name = "serde_json" -version = "1.0.61" +version = "1.0.62" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4fceb2595057b6891a4ee808f70054bd2d12f0e97f1cbb78689b59f676df325a" +checksum = "ea1c6153794552ea7cf7cf63b1231a25de00ec90db326ba6264440fa08e31486" dependencies = [ "itoa", "ryu", @@ -809,12 +697,11 @@ dependencies = [ [[package]] name = "sharded-slab" -version = "0.1.0" +version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7b4921be914e16899a80adefb821f8ddb7974e3f1250223575a44ed994882127" +checksum = "79c719719ee05df97490f80a45acfc99e5a30ce98a1e4fb67aee422745ae14e3" dependencies = [ "lazy_static", - "loom", ] [[package]] @@ -825,9 +712,9 @@ checksum = "c111b5bd5695e56cffe5129854aa230b39c93a305372fdbb2668ca2394eea9f8" [[package]] name = "smallvec" -version = "1.5.1" +version = "1.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae524f056d7d770e174287294f562e95044c68e88dec909a00d2094805db9d75" +checksum = "fe0f37c9e8f3c5a4a66ad655a93c74daac4ad00c441533bf5c6e7990bb42604e" [[package]] name = "socket2" @@ -866,8 +753,8 @@ dependencies = [ "heck", "proc-macro-error", "proc-macro2 1.0.24", - "quote 1.0.7", - "syn 1.0.59", + "quote 1.0.8", + "syn 1.0.60", ] [[package]] @@ -883,12 +770,12 @@ dependencies = [ [[package]] name = "syn" -version = "1.0.59" +version = "1.0.60" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "07cb8b1b4ebf86a89ee88cbd201b022b94138c623644d035185c84d3f41b7e66" +checksum = "c700597eca8a5a762beb35753ef6b94df201c81cca676604f547495a0d7f0081" dependencies = [ "proc-macro2 1.0.24", - "quote 1.0.7", + "quote 1.0.8", "unicode-xid 0.2.1", ] @@ -943,36 +830,26 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9be73a2caec27583d0046ef3796c3794f868a5bc813db689eed00c7631275cd1" dependencies = [ "proc-macro2 1.0.24", - "quote 1.0.7", - "syn 1.0.59", + "quote 1.0.8", + "syn 1.0.60", ] [[package]] name = "thread_local" -version = "1.0.1" +version = "1.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d40c6d1b69745a6ec6fb1ca717914848da4b44ae29d9b3080cbee91d72a69b14" +checksum = "8018d24e04c95ac8790716a5987d0fec4f8b27249ffa0f7d33f1369bdfb88cbd" dependencies = [ - "lazy_static", -] - -[[package]] -name = "thread_local" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d40c6d1b69745a6ec6fb1ca717914848da4b44ae29d9b3080cbee91d72a69b14" -dependencies = [ - "lazy_static", + "once_cell", ] [[package]] name = "time" -version = "0.1.44" +version = "0.1.43" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6db9e6914ab8b1ae1c260a4ae7a49b6c5611b40328a735b21862567685e73255" +checksum = "ca8a50ef2360fbd1eeb0ecd46795a87a19024eb4b53c5dc916ca1fd95fe62438" dependencies = [ "libc", - "wasi", "winapi", ] @@ -990,9 +867,9 @@ dependencies = [ [[package]] name = "tokio" -version = "1.1.0" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8efab2086f17abcddb8f756117665c958feee6b2e39974c2f1600592ab3a4195" +checksum = "e8190d04c665ea9e6b6a0dc45523ade572c088d2e6566244c1122671dbf4ae3a" dependencies = [ "autocfg", "libc", @@ -1003,29 +880,29 @@ dependencies = [ [[package]] name = "tokio-macros" -version = "1.0.0" +version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42517d2975ca3114b22a16192634e8241dc5cc1f130be194645970cc1c371494" +checksum = "caf7b11a536f46a809a8a9f0bb4237020f70ecbf115b842360afb127ea2fda57" dependencies = [ "proc-macro2 1.0.24", - "quote 1.0.7", - "syn 1.0.59", + "quote 1.0.8", + "syn 1.0.60", ] [[package]] name = "toml" -version = "0.5.7" +version = "0.5.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "75cf45bb0bef80604d001caaec0d09da99611b3c0fd39d3080468875cdb65645" +checksum = "a31142970826733df8241ef35dc040ef98c679ab14d7c3e54d827099b3acecaa" dependencies = [ "serde", ] [[package]] name = "tracing" -version = "0.1.22" +version = "0.1.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f47026cdc4080c07e49b37087de021820269d996f581aac150ef9e5583eefe3" +checksum = "f7d40a22fd029e33300d8d89a5cc8ffce18bb7c587662f54629e94c9de5487f3" dependencies = [ "cfg-if 1.0.0", "pin-project-lite", @@ -1035,13 +912,13 @@ dependencies = [ [[package]] name = "tracing-attributes" -version = "0.1.11" +version = "0.1.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "80e0ccfc3378da0cce270c946b676a376943f5cd16aeba64568e7939806f4ada" +checksum = "43f080ea7e4107844ef4766459426fa2d5c1ada2e47edba05dc7fa99d9629f47" dependencies = [ "proc-macro2 1.0.24", - "quote 1.0.7", - "syn 1.0.59", + "quote 1.0.8", + "syn 1.0.60", ] [[package]] @@ -1098,9 +975,9 @@ dependencies = [ [[package]] name = "unicode-segmentation" -version = "1.6.0" +version = "1.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e83e153d1053cbb5a118eeff7fd5be06ed99153f00dbcd8ae310c5fb2b22edc0" +checksum = "bb0d2e7be6ae3a5fa87eed5fb451aff96f2573d2694942e40543ae0bbe19c796" [[package]] name = "unicode-width" @@ -1140,9 +1017,9 @@ checksum = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d" [[package]] name = "wasi" -version = "0.10.0+wasi-snapshot-preview1" +version = "0.10.2+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a143597ca7c7793eff794def352d41792a93c481eb1042423ff7ff72ba2c31f" +checksum = "fd6fbd9a79829dd1ad0cc20627bf1ed606756a7f77edff7b66b7064f9cb327c6" [[package]] name = "winapi" diff --git a/Cargo.toml b/Cargo.toml index aff6f82a..38a88d2c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,36 +5,27 @@ authors = ["Mikail Bagishov "] edition = "2018" [dependencies] -libc = "0.2.82" +libc = "0.2.86" errno = "0.2.7" -rand = "0.8.2" -serde = { version = "1.0.121", features = ["derive"] } -serde_json = "1.0.61" -tiny-nix-ipc = { git = "https://github.com/myfreeweb/tiny-nix-ipc", features = ["ser_json", "zero_copy"] } -nix = "0.19.1" +rand = "0.8.3" +serde = { version = "1.0.123", features = ["derive"] } +serde_json = "1.0.62" backtrace = "0.3.56" thiserror = "1.0.23" anyhow = "1.0.38" once_cell = "1.5.2" futures-util = "0.3.12" -tokio = { version = "1.1.0", features = ["net"] } -tracing = "0.1.22" +tokio = { version = "1.2.0", features = ["net"] } +tracing = "0.1.23" itoa = "0.4.7" -serde = { version = "1.0.106", features = ["derive"] } -thiserror = "1.0.19" -anyhow = "1.0.32" -futures-util = "0.3.6" -once_cell = "1.4.1" -backtrace = "0.3.46" [target.'cfg(target_os="linux")'.dependencies] tiny-nix-ipc = { git = "https://github.com/myfreeweb/tiny-nix-ipc", features = ["ser_json", "zero_copy"] } -procfs = "0.7.8" nix = {git = "https://github.com/nix-rust/nix"} -libc = "0.2.68" -errno = "0.2.5" -serde_json = "1.0.51" -tokio = { version = "0.3.2", features = ["net"] } +libc = "0.2.86" +errno = "0.2.7" +serde_json = "1.0.62" +tokio = { version = "1.2.0", features = ["net"] } [target.'cfg(target_os="windows")'.dependencies] winapi = { version = "0.3.9", features = ["std", "processthreadsapi", "jobapi2", "errhandlingapi", diff --git a/minion-tests/Cargo.toml b/minion-tests/Cargo.toml index c25149f5..4a533bfa 100644 --- a/minion-tests/Cargo.toml +++ b/minion-tests/Cargo.toml @@ -11,7 +11,6 @@ clap = "2.33.3" tempfile = "3.2.0" tokio = { version = "1.1.0", features = ["macros", "rt"] } tracing-subscriber = "0.2.15" -tracing-subscriber = "0.2.15" [target.'cfg(target_os="linux")'.dependencies] nix = "0.17.0" diff --git a/src/erased.rs b/src/erased.rs index f4b9a39f..5fd1ba9e 100644 --- a/src/erased.rs +++ b/src/erased.rs @@ -18,7 +18,7 @@ pub trait Sandbox: std::fmt::Debug + Send + Sync + 'static { } impl Sandbox for S { - fn id(&self) -> &str { + fn id(&self) -> String { self.id() } fn check_cpu_tle(&self) -> anyhow::Result { diff --git a/src/lib.rs b/src/lib.rs index 99a89a4e..61e5b4bc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -120,7 +120,7 @@ impl SandboxOptions { /// Represents highly-isolated sandbox pub trait Sandbox: Debug + Send + Sync + 'static { type Error: StdError + Send + Sync + 'static; - fn id(&self) -> &str; + fn id(&self) -> String; /// Returns true if sandbox exceeded CPU time limit fn check_cpu_tle(&self) -> Result; diff --git a/src/linux/jail_common.rs b/src/linux/jail_common.rs index ae76d38e..555ae76a 100644 --- a/src/linux/jail_common.rs +++ b/src/linux/jail_common.rs @@ -1,5 +1,4 @@ use crate::{linux::util::Pid, SharedItem}; -use rand::seq::SliceRandom; use serde::{Deserialize, Serialize}; use std::{ffi::OsString, os::unix::io::RawFd, path::PathBuf, time::Duration}; use tiny_nix_ipc::Socket; From a36d9a843f6f95e54dc97a337adcf3a6476e8e27 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Tue, 9 Feb 2021 20:50:03 +0300 Subject: [PATCH 04/18] fix tracing --- Cargo.toml | 4 +- minion-tests/src/main.rs | 3 +- minion-tests/src/worker.rs | 4 -- src/windows/child.rs | 19 ++---- src/windows/pipe.rs | 96 +++++++------------------------ src/windows/sandbox.rs | 4 +- src/windows/spawn.rs | 81 +++++++++++++++----------- src/windows/util.rs | 115 +++++++++++++++++++++++++++++++------ src/windows/wait.rs | 27 +++++---- 9 files changed, 191 insertions(+), 162 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 38a88d2c..5f1bb299 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,8 +5,6 @@ authors = ["Mikail Bagishov "] edition = "2018" [dependencies] -libc = "0.2.86" -errno = "0.2.7" rand = "0.8.3" serde = { version = "1.0.123", features = ["derive"] } serde_json = "1.0.62" @@ -17,7 +15,6 @@ once_cell = "1.5.2" futures-util = "0.3.12" tokio = { version = "1.2.0", features = ["net"] } tracing = "0.1.23" -itoa = "0.4.7" [target.'cfg(target_os="linux")'.dependencies] tiny-nix-ipc = { git = "https://github.com/myfreeweb/tiny-nix-ipc", features = ["ser_json", "zero_copy"] } @@ -26,6 +23,7 @@ libc = "0.2.86" errno = "0.2.7" serde_json = "1.0.62" tokio = { version = "1.2.0", features = ["net"] } +itoa = "0.4.7" [target.'cfg(target_os="windows")'.dependencies] winapi = { version = "0.3.9", features = ["std", "processthreadsapi", "jobapi2", "errhandlingapi", diff --git a/minion-tests/src/main.rs b/minion-tests/src/main.rs index 92f3a56d..da80c2f2 100644 --- a/minion-tests/src/main.rs +++ b/minion-tests/src/main.rs @@ -64,7 +64,8 @@ static WORKER_ENV_NAME: &str = "__MINION_ROLE_IS_WORKER__"; static TEST_ENV_NAME: &str = "__MINION_ROLE_IS_TEST__"; fn main() { tracing_subscriber::fmt() - .with_writer(std::io::stdout) + .pretty() + .with_writer(std::io::stderr) .init(); let test_cases = &*tests::TESTS; let role = get_role(); diff --git a/minion-tests/src/worker.rs b/minion-tests/src/worker.rs index 51a48a1c..62750cfa 100644 --- a/minion-tests/src/worker.rs +++ b/minion-tests/src/worker.rs @@ -58,10 +58,6 @@ async fn inner_main(test_cases: &[&'static dyn TestCase]) { } pub fn main(test_cases: &[&'static dyn TestCase]) { - tracing_subscriber::fmt() - .pretty() - .with_writer(std::io::stderr) - .init(); let rt = tokio::runtime::Builder::new_current_thread() .enable_all() .build() diff --git a/src/windows/child.rs b/src/windows/child.rs index ec73d67d..4c35bece 100644 --- a/src/windows/child.rs +++ b/src/windows/child.rs @@ -1,16 +1,16 @@ use crate::windows::{ spawn::{spawn, ChildParams, Stdio}, + util::OwnedHandle, Error, ReadPipe, WindowsSandbox, WritePipe, }; -use winapi::um::{handleapi::CloseHandle, winnt::HANDLE}; #[derive(Debug)] pub struct WindowsChildProcess { /// Handle to winapi Process object - child: HANDLE, + child: OwnedHandle, /// Handle to main process of child - main_thread: HANDLE, + main_thread: OwnedHandle, /// Stdin stdin: Option, @@ -20,15 +20,6 @@ pub struct WindowsChildProcess { stderr: Option, } -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( options: crate::ChildProcessOptions, @@ -46,8 +37,8 @@ impl WindowsChildProcess { )?; Ok(WindowsChildProcess { - child: info.hProcess, - main_thread: info.hThread, + child: OwnedHandle::new(info.hProcess), + main_thread: OwnedHandle::new(info.hThread), stdin: child_stdin, stdout: child_stdout, stderr: child_stderr, diff --git a/src/windows/pipe.rs b/src/windows/pipe.rs index 7f6d1cdd..d4f5be8c 100644 --- a/src/windows/pipe.rs +++ b/src/windows/pipe.rs @@ -1,102 +1,49 @@ -use crate::windows::{Cvt, Error}; +use crate::windows::{util::OwnedHandle, Cvt, Error}; use std::os::windows::io::{FromRawHandle, IntoRawHandle, RawHandle}; use winapi::{ shared::minwindef::TRUE, - um::{ - handleapi::CloseHandle, minwinbase::SECURITY_ATTRIBUTES, namedpipeapi::CreatePipe, - winnt::HANDLE, - }, + um::{minwinbase::SECURITY_ATTRIBUTES, namedpipeapi::CreatePipe}, }; #[derive(Debug)] pub struct ReadPipe { - handle: HANDLE, + handle: OwnedHandle, } -unsafe impl Send for ReadPipe {} -unsafe impl Sync for ReadPipe {} - impl IntoRawHandle for ReadPipe { fn into_raw_handle(self) -> RawHandle { - let h = self.handle; - std::mem::forget(self); - h + self.handle.into_inner() } } impl FromRawHandle for ReadPipe { unsafe fn from_raw_handle(handle: RawHandle) -> Self { - ReadPipe { handle } - } -} - -impl std::io::Read for ReadPipe { - fn read(&mut self, mut buf: &mut [u8]) -> std::io::Result { - if buf.len() > i32::max_value() as usize { - buf = &mut buf[..(i32::max_value() as usize)] - } - let mut read_cnt = 0; - let res = unsafe { - winapi::um::fileapi::ReadFile( - self.handle, - buf.as_mut_ptr().cast(), - buf.len() as u32, - &mut read_cnt, - std::ptr::null_mut(), - ) - }; - - if res == 0 { - return Err(std::io::Error::last_os_error()); + ReadPipe { + handle: OwnedHandle::new(handle), } - Ok(read_cnt as usize) } } -impl Drop for ReadPipe { - fn drop(&mut self) { - unsafe { - CloseHandle(self.handle); - } +impl std::io::Read for ReadPipe { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + self.handle.read(buf) } } #[derive(Debug)] pub struct WritePipe { - handle: HANDLE, + handle: OwnedHandle, } -unsafe impl Send for WritePipe {} -unsafe impl Sync for WritePipe {} - impl IntoRawHandle for WritePipe { fn into_raw_handle(self) -> RawHandle { - let h = self.handle; - std::mem::forget(self); - h + self.handle.into_inner() } } impl std::io::Write for WritePipe { - fn write(&mut self, mut buf: &[u8]) -> std::io::Result { - if buf.len() > (i32::max_value() as usize) { - buf = &buf[..(i32::max_value() as usize)]; - } - let mut written_cnt = 0; - let res = unsafe { - winapi::um::fileapi::WriteFile( - self.handle, - buf.as_ptr().cast(), - buf.len() as u32, - &mut written_cnt, - std::ptr::null_mut(), - ) - }; - if res != 0 { - Ok(written_cnt as usize) - } else { - Err(std::io::Error::last_os_error()) - } + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.handle.write(buf) } fn flush(&mut self) -> std::io::Result<()> { @@ -105,14 +52,6 @@ impl std::io::Write for WritePipe { } } -impl Drop for WritePipe { - fn drop(&mut self) { - unsafe { - CloseHandle(self.handle); - } - } -} - pub(in crate::windows) enum InheritKind { Allow, } @@ -133,5 +72,12 @@ pub(in crate::windows) fn make(inherit: InheritKind) -> Result<(ReadPipe, WriteP 0, ))?; } - Ok((ReadPipe { handle: read }, WritePipe { handle: write })) + Ok(( + ReadPipe { + handle: OwnedHandle::new(read), + }, + WritePipe { + handle: OwnedHandle::new(write), + }, + )) } diff --git a/src/windows/sandbox.rs b/src/windows/sandbox.rs index 0e4aa233..120ff687 100644 --- a/src/windows/sandbox.rs +++ b/src/windows/sandbox.rs @@ -23,8 +23,8 @@ impl WindowsSandbox { impl crate::Sandbox for WindowsSandbox { type Error = Error; - fn id(&self) -> &str { - todo!() + fn id(&self) -> String { + self.id.clone() } fn kill(&self) -> Result<(), Self::Error> { diff --git a/src/windows/spawn.rs b/src/windows/spawn.rs index 5c42511f..9047744d 100644 --- a/src/windows/spawn.rs +++ b/src/windows/spawn.rs @@ -1,6 +1,7 @@ use crate::{ windows::{ pipe::{self, ReadPipe, WritePipe}, + util::OwnedHandle, Cvt, Error, WindowsSandbox, }, InputSpecificationData, OutputSpecificationData, @@ -17,7 +18,7 @@ use winapi::{ shared::{minwindef::TRUE, winerror::ERROR_INSUFFICIENT_BUFFER}, um::{ errhandlingapi::GetLastError, - handleapi::{CloseHandle, INVALID_HANDLE_VALUE}, + handleapi::INVALID_HANDLE_VALUE, minwinbase::SECURITY_ATTRIBUTES, processthreadsapi::{ CreateProcessW, DeleteProcThreadAttributeList, GetCurrentProcess, @@ -34,46 +35,49 @@ use winapi::{ }; pub(in crate::windows) struct Stdio { - pub stdin: HANDLE, - pub stdout: HANDLE, - pub stderr: HANDLE, + pub stdin: Option, + pub stdout: Option, + pub stderr: Option, } impl Stdio { fn make_input( spec: crate::InputSpecificationData, - ) -> Result<(HANDLE, Option), Error> { + ) -> Result<(Option, Option), Error> { match spec { - InputSpecificationData::Handle(h) => Ok((h as HANDLE, None)), + InputSpecificationData::Handle(h) => Ok((Some(OwnedHandle::new(h as HANDLE)), None)), InputSpecificationData::Pipe => { let (reader, writer) = pipe::make(pipe::InheritKind::Allow)?; - Ok((reader.into_raw_handle(), Some(writer))) + Ok(( + Some(OwnedHandle::new(reader.into_raw_handle())), + Some(writer), + )) } - InputSpecificationData::Empty => Ok((open_empty_readable_file(), None)), - InputSpecificationData::Null => Ok((-1_i32 as usize as HANDLE, None)), + InputSpecificationData::Empty => Ok((Some(open_empty_readable_file()), None)), + InputSpecificationData::Null => Ok((None, None)), } } fn make_output( spec: crate::OutputSpecificationData, - ) -> Result<(HANDLE, Option), Error> { + ) -> Result<(Option, Option), Error> { match spec { - OutputSpecificationData::Handle(h) => Ok((h as HANDLE, None)), + OutputSpecificationData::Handle(h) => Ok((Some(OwnedHandle::new(h as HANDLE)), None)), OutputSpecificationData::Pipe => { let (reader, writer) = pipe::make(pipe::InheritKind::Allow)?; - Ok((writer.into_raw_handle(), Some(reader))) + Ok(( + Some(OwnedHandle::new(writer.into_raw_handle())), + Some(reader), + )) } - OutputSpecificationData::Null => Ok((-1_i32 as usize as HANDLE, None)), + OutputSpecificationData::Null => Ok((None, None)), OutputSpecificationData::Ignore => { let file = std::fs::File::create("C:\\NUL").map_err(|io_err| Error::Syscall { errno: io_err.raw_os_error().unwrap_or(-1) as u32, })?; - let file = file.into_raw_handle(); - let cloned_file = crate::windows::util::duplicate_with_inheritance(file)?; - unsafe { - CloseHandle(file); - } - Ok((cloned_file, None)) + let file = OwnedHandle::new(file.into_raw_handle()); + let cloned_file = file.try_clone_with_inheritance()?; + Ok((Some(cloned_file), None)) } OutputSpecificationData::Buffer(sz) => unsafe { let sz = sz.unwrap_or(usize::max_value()) as u64; @@ -88,8 +92,12 @@ impl Stdio { if mmap.is_null() { Cvt::nonzero(0)?; } - let child_side = crate::windows::util::duplicate_with_inheritance(mmap)?; - Ok((child_side, Some(ReadPipe::from_raw_handle(mmap)))) + let mmap = OwnedHandle::new(mmap); + let child_side = mmap.try_clone_with_inheritance()?; + Ok(( + Some(child_side), + Some(ReadPipe::from_raw_handle(mmap.into_inner())), + )) }, } } @@ -117,14 +125,17 @@ impl Stdio { } } -fn open_empty_readable_file() -> HANDLE { - static EMPTY_FILE_HANDLE: once_cell::sync::Lazy = once_cell::sync::Lazy::new(|| { - let (reader, writer) = - pipe::make(pipe::InheritKind::Allow).expect("failed to create a pipe"); - drop(writer); - reader.into_raw_handle() as usize - }); - *EMPTY_FILE_HANDLE as HANDLE +fn open_empty_readable_file() -> OwnedHandle { + static EMPTY_FILE_HANDLE: once_cell::sync::Lazy = + once_cell::sync::Lazy::new(|| { + let (reader, writer) = + pipe::make(pipe::InheritKind::Allow).expect("failed to create a pipe"); + drop(writer); + OwnedHandle::new(reader.into_raw_handle()) + }); + EMPTY_FILE_HANDLE + .try_clone() + .expect("failed to clone a handle") } pub(in crate::windows) struct ChildParams { @@ -211,9 +222,15 @@ pub(in crate::windows) fn spawn( startup_info.StartupInfo.cb = size_of::() as u32; startup_info.StartupInfo.dwFlags = STARTF_USESTDHANDLES; - startup_info.StartupInfo.hStdInput = stdio.stdin; - startup_info.StartupInfo.hStdOutput = stdio.stdout; - startup_info.StartupInfo.hStdError = stdio.stderr; + + let cvt_handle = |h: Option| { + h.map(OwnedHandle::into_inner) + .unwrap_or(INVALID_HANDLE_VALUE) + }; + + startup_info.StartupInfo.hStdInput = cvt_handle(stdio.stdin); + startup_info.StartupInfo.hStdOutput = cvt_handle(stdio.stdout); + startup_info.StartupInfo.hStdError = cvt_handle(stdio.stderr); startup_info }; let creation_flags = CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT; diff --git a/src/windows/util.rs b/src/windows/util.rs index 9072d54d..72b9f018 100644 --- a/src/windows/util.rs +++ b/src/windows/util.rs @@ -1,26 +1,107 @@ +use crate::windows::{Cvt, Error}; +use std::mem::ManuallyDrop; use winapi::{ - shared::minwindef::TRUE, + shared::minwindef::{FALSE, TRUE}, um::{ - handleapi::DuplicateHandle, + handleapi::{CloseHandle, DuplicateHandle, INVALID_HANDLE_VALUE}, processthreadsapi::GetCurrentProcess, winnt::{DUPLICATE_SAME_ACCESS, HANDLE}, }, }; -use crate::windows::{Cvt, Error}; +#[derive(Debug)] +pub struct OwnedHandle(HANDLE); + +unsafe impl Send for OwnedHandle {} +unsafe impl Sync for OwnedHandle {} + +impl OwnedHandle { + pub fn new(h: HANDLE) -> Self { + assert_ne!(h, INVALID_HANDLE_VALUE); + OwnedHandle(h) + } + + pub fn as_raw(&self) -> HANDLE { + self.0 + } + + pub fn into_inner(self) -> HANDLE { + let this = ManuallyDrop::new(self); + this.0 + } + + pub fn read(&self, mut buf: &mut [u8]) -> std::io::Result { + if buf.len() > i32::max_value() as usize { + buf = &mut buf[..(i32::max_value() as usize)] + } + let mut read_cnt = 0; + let res = unsafe { + winapi::um::fileapi::ReadFile( + self.0, + buf.as_mut_ptr().cast(), + buf.len() as u32, + &mut read_cnt, + std::ptr::null_mut(), + ) + }; + + if res == 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(read_cnt as usize) + } + + pub fn write(&self, mut buf: &[u8]) -> std::io::Result { + if buf.len() > (i32::max_value() as usize) { + buf = &buf[..(i32::max_value() as usize)]; + } + let mut written_cnt = 0; + let res = unsafe { + winapi::um::fileapi::WriteFile( + self.0, + buf.as_ptr().cast(), + buf.len() as u32, + &mut written_cnt, + std::ptr::null_mut(), + ) + }; + if res != 0 { + Ok(written_cnt as usize) + } else { + Err(std::io::Error::last_os_error()) + } + } -pub(in crate::windows) fn duplicate_with_inheritance(handle: HANDLE) -> Result { - let mut cloned_handle = std::ptr::null_mut(); - unsafe { - Cvt::nonzero(DuplicateHandle( - GetCurrentProcess(), - handle, - GetCurrentProcess(), - &mut cloned_handle, - 0, - TRUE, - DUPLICATE_SAME_ACCESS, - ))?; - } - Ok(cloned_handle) + fn duplicate(&self, inherit: bool) -> Result { + let mut cloned_handle = std::ptr::null_mut(); + unsafe { + Cvt::nonzero(DuplicateHandle( + GetCurrentProcess(), + self.as_raw(), + GetCurrentProcess(), + &mut cloned_handle, + 0, + if inherit { TRUE } else { FALSE }, + DUPLICATE_SAME_ACCESS, + ))?; + } + Ok(Self::new(cloned_handle)) + } + + pub fn try_clone(&self) -> Result { + self.duplicate(false) + } + + pub fn try_clone_with_inheritance(&self) -> Result { + self.duplicate(true) + } +} + +impl Drop for OwnedHandle { + fn drop(&mut self) { + let ret = unsafe { CloseHandle(self.0) }; + if ret == 0 { + panic!("failed to close handle {}", self.0 as usize); + } + } } diff --git a/src/windows/wait.rs b/src/windows/wait.rs index 1c3ba13e..45e4d547 100644 --- a/src/windows/wait.rs +++ b/src/windows/wait.rs @@ -1,5 +1,5 @@ //! Implements windows WaitFuture -use crate::windows::{Cvt, Error}; +use crate::windows::{util::OwnedHandle, Cvt, Error}; use futures_util::task::AtomicWaker; use std::{ pin::Pin, @@ -19,28 +19,22 @@ use winapi::{ processthreadsapi::{GetExitCodeProcess, GetProcessId}, synchapi::WaitForSingleObject, winbase::WAIT_OBJECT_0, - winnt::HANDLE, }, }; /// Resolves when child has finished pub struct WaitFuture { /// Child handle - child: HANDLE, + child: OwnedHandle, /// None if background thread has not been started yet shared: Option>, } -// HANDLE is Send-able... -unsafe impl Send for WaitFuture {} -// ..and Sync -unsafe impl Sync for WaitFuture {} - impl WaitFuture { fn get_exit_code(&self) -> Result, Error> { let mut exit_code = 0; unsafe { - Cvt::nonzero(GetExitCodeProcess(self.child, &mut exit_code))?; + Cvt::nonzero(GetExitCodeProcess(self.child.as_raw(), &mut exit_code))?; } if exit_code == STILL_ACTIVE { return Ok(None); @@ -71,15 +65,20 @@ impl std::future::Future for WaitFuture { let thread_name = unsafe { format!( "minion-background-wait-{}", - Cvt::nonzero(GetProcessId(this.child) as i32).unwrap_or(-1) + Cvt::nonzero(GetProcessId(this.child.as_raw()) as i32).unwrap_or(-1) ) }; - let child_handle = this.child.clone() as usize; + let child_handle = match this.child.try_clone() { + Ok(cl) => cl, + Err(err) => { + return Poll::Ready(Err(err)); + } + }; std::thread::Builder::new() .name(thread_name) - .spawn(move || background_waiter(shared, child_handle as HANDLE)) + .spawn(move || background_waiter(shared, child_handle)) .expect("Failed to create a thread"); } @@ -108,7 +107,7 @@ struct Shared { error: AtomicBool, } -fn background_waiter(mut shared: Arc, handle: HANDLE) { +fn background_waiter(mut shared: Arc, handle: OwnedHandle) { loop { // check if someone is still interested in our work if Arc::get_mut(&mut shared).is_some() { @@ -118,7 +117,7 @@ fn background_waiter(mut shared: Arc, handle: HANDLE) { } // wait for one second - let res = unsafe { WaitForSingleObject(handle, 1000) }; + let res = unsafe { WaitForSingleObject(handle.as_raw(), 1000) }; if res == WAIT_OBJECT_0 { shared.waker.wake(); return; From e1f8f325244cf80093e1ac2a5053a8f894580830 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Tue, 9 Feb 2021 22:46:37 +0300 Subject: [PATCH 05/18] more cleanup --- Cargo.toml | 2 -- minion-tests/src/master.rs | 3 +++ src/windows/isolate.rs | 14 +++++++++----- src/windows/spawn.rs | 3 +-- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5f1bb299..83774704 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,13 +7,11 @@ edition = "2018" [dependencies] rand = "0.8.3" serde = { version = "1.0.123", features = ["derive"] } -serde_json = "1.0.62" backtrace = "0.3.56" thiserror = "1.0.23" anyhow = "1.0.38" once_cell = "1.5.2" futures-util = "0.3.12" -tokio = { version = "1.2.0", features = ["net"] } tracing = "0.1.23" [target.'cfg(target_os="linux")'.dependencies] diff --git a/minion-tests/src/master.rs b/minion-tests/src/master.rs index 6683ae96..5d1a9446 100644 --- a/minion-tests/src/master.rs +++ b/minion-tests/src/master.rs @@ -152,6 +152,9 @@ fn get_filter(matches: &clap::ArgMatches) -> Box, } unsafe impl Send for Profile {} @@ -33,7 +35,6 @@ impl Profile { let profile_name = OsString::from(format!("minion-sandbox-appcontainer-{}", sandbox_id)); let mut profile_name: Vec = profile_name.encode_wide().collect(); profile_name.push(0); - let profile_name = profile_name.as_ptr(); let mut sid = std::ptr::null_mut(); unsafe { let mut sid_string_repr = std::ptr::null_mut(); @@ -51,15 +52,15 @@ impl Profile { } unsafe { Cvt::hresult(CreateAppContainerProfile( - profile_name, - profile_name, - profile_name, + profile_name.as_ptr(), + profile_name.as_ptr(), + profile_name.as_ptr(), std::ptr::null_mut(), 0, &mut sid, ))?; } - Ok(Profile { sid }) + Ok(Profile { sid, profile_name }) } /// Returns `SECURITY_CAPABILITIES` representing this container. #[instrument(skip(self))] @@ -75,6 +76,9 @@ impl Drop for Profile { fn drop(&mut self) { unsafe { FreeSid(self.sid); + if let Err(e) = Cvt::hresult(DeleteAppContainerProfile(self.profile_name.as_ptr())) { + tracing::warn!(error = %e, "Ignoring resource cleanup error"); + } } } } diff --git a/src/windows/spawn.rs b/src/windows/spawn.rs index 9047744d..469204f6 100644 --- a/src/windows/spawn.rs +++ b/src/windows/spawn.rs @@ -21,11 +21,10 @@ use winapi::{ handleapi::INVALID_HANDLE_VALUE, minwinbase::SECURITY_ATTRIBUTES, processthreadsapi::{ - CreateProcessW, DeleteProcThreadAttributeList, GetCurrentProcess, + CreateProcessW, DeleteProcThreadAttributeList, InitializeProcThreadAttributeList, UpdateProcThreadAttribute, PROCESS_INFORMATION, PROC_THREAD_ATTRIBUTE_LIST, }, - userenv::{CreateAppContainerProfile, DeleteAppContainerProfile}, winbase::{ CreateFileMappingA, CREATE_UNICODE_ENVIRONMENT, EXTENDED_STARTUPINFO_PRESENT, STARTF_USESTDHANDLES, STARTUPINFOEXW, From 3230496941814d4166c341772dd7d1a19432a9a1 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Tue, 9 Feb 2021 22:49:41 +0300 Subject: [PATCH 06/18] cleanup job --- src/windows/constrain.rs | 35 +++++++++++++++-------------------- src/windows/spawn.rs | 5 ++--- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/windows/constrain.rs b/src/windows/constrain.rs index ae21a312..3711b197 100644 --- a/src/windows/constrain.rs +++ b/src/windows/constrain.rs @@ -3,12 +3,11 @@ use std::{ffi::OsString, os::windows::ffi::OsStrExt}; use crate::{ - windows::{Cvt, Error}, + windows::{util::OwnedHandle, Cvt, Error}, ResourceUsageData, }; use winapi::um::{ - handleapi::CloseHandle, jobapi2::{ AssignProcessToJobObject, CreateJobObjectW, QueryInformationJobObject, SetInformationJobObject, TerminateJobObject, @@ -25,12 +24,9 @@ use winapi::um::{ /// Responsible for resource isolation & adding & killing #[derive(Debug)] pub(crate) struct Job { - handle: HANDLE, + handle: OwnedHandle, } -unsafe impl Send for Job {} -unsafe impl Sync for Job {} - impl Job { pub(crate) fn new(jail_id: &str) -> Result { let name: OsString = format!("minion-sandbox-job-{}", jail_id).into(); @@ -38,6 +34,7 @@ impl Job { let handle = unsafe { Cvt::nonzero(CreateJobObjectW(std::ptr::null_mut(), name.as_ptr()) as i32)? as HANDLE }; + let handle = OwnedHandle::new(handle); Ok(Self { handle }) } pub(crate) fn enable_resource_limits( @@ -60,7 +57,7 @@ impl Job { | JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; unsafe { Cvt::nonzero(SetInformationJobObject( - self.handle, + self.handle.as_raw(), JobObjectExtendedLimitInformation, (&mut info as *mut JOBOBJECT_EXTENDED_LIMIT_INFORMATION).cast(), sizeof::(), @@ -69,16 +66,22 @@ impl Job { Ok(()) } pub(crate) fn kill(&self) -> Result<(), Error> { - unsafe { Cvt::nonzero(TerminateJobObject(self.handle, 0xDEADBEEF)).map(|_| ()) } + unsafe { Cvt::nonzero(TerminateJobObject(self.handle.as_raw(), 0xDEADBEEF)).map(|_| ()) } } pub(crate) fn add_process(&self, process_handle: HANDLE) -> Result<(), Error> { - unsafe { Cvt::nonzero(AssignProcessToJobObject(self.handle, process_handle)).map(|_| ()) } + unsafe { + Cvt::nonzero(AssignProcessToJobObject( + self.handle.as_raw(), + process_handle, + )) + .map(|_| ()) + } } pub(crate) fn resource_usage(&self) -> Result { let cpu = unsafe { let mut info: JOBOBJECT_BASIC_ACCOUNTING_INFORMATION = std::mem::zeroed(); Cvt::nonzero(QueryInformationJobObject( - self.handle, + self.handle.as_raw(), JobObjectBasicAccountingInformation, (&mut info as *mut JOBOBJECT_BASIC_ACCOUNTING_INFORMATION).cast(), sizeof::(), @@ -92,7 +95,7 @@ impl Job { let memory = unsafe { let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = std::mem::zeroed(); Cvt::nonzero(QueryInformationJobObject( - self.handle, + self.handle.as_raw(), JobObjectExtendedLimitInformation, (&mut info as *mut JOBOBJECT_EXTENDED_LIMIT_INFORMATION).cast(), sizeof::(), @@ -110,7 +113,7 @@ impl Job { unsafe { let mut info: JOBOBJECT_LIMIT_VIOLATION_INFORMATION = std::mem::zeroed(); Cvt::nonzero(QueryInformationJobObject( - self.handle, + self.handle.as_raw(), JobObjectLimitViolationInformation, (&mut info as *mut JOBOBJECT_LIMIT_VIOLATION_INFORMATION).cast(), sizeof::(), @@ -127,14 +130,6 @@ impl Job { } } -impl Drop for Job { - fn drop(&mut self) { - unsafe { - CloseHandle(self.handle); - } - } -} - fn sizeof() -> u32 { let sz = std::mem::size_of::(); assert!(sz <= (u32::max_value() as usize)); diff --git a/src/windows/spawn.rs b/src/windows/spawn.rs index 469204f6..7495aa09 100644 --- a/src/windows/spawn.rs +++ b/src/windows/spawn.rs @@ -21,9 +21,8 @@ use winapi::{ handleapi::INVALID_HANDLE_VALUE, minwinbase::SECURITY_ATTRIBUTES, processthreadsapi::{ - CreateProcessW, DeleteProcThreadAttributeList, - InitializeProcThreadAttributeList, UpdateProcThreadAttribute, PROCESS_INFORMATION, - PROC_THREAD_ATTRIBUTE_LIST, + CreateProcessW, DeleteProcThreadAttributeList, InitializeProcThreadAttributeList, + UpdateProcThreadAttribute, PROCESS_INFORMATION, PROC_THREAD_ATTRIBUTE_LIST, }, winbase::{ CreateFileMappingA, CREATE_UNICODE_ENVIRONMENT, EXTENDED_STARTUPINFO_PRESENT, From db5266702e760d6314d003e43a4eadbdc890fea0 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Tue, 9 Feb 2021 23:15:53 +0300 Subject: [PATCH 07/18] refactor attr list handling --- src/windows/spawn.rs | 102 +++++++----------------------- src/windows/spawn/attr_list.rs | 112 +++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 78 deletions(-) create mode 100644 src/windows/spawn/attr_list.rs diff --git a/src/windows/spawn.rs b/src/windows/spawn.rs index 7495aa09..5ac350ba 100644 --- a/src/windows/spawn.rs +++ b/src/windows/spawn.rs @@ -1,3 +1,5 @@ +mod attr_list; + use crate::{ windows::{ pipe::{self, ReadPipe, WritePipe}, @@ -6,6 +8,7 @@ use crate::{ }, InputSpecificationData, OutputSpecificationData, }; +use attr_list::AttrList; use std::{ ffi::{OsStr, OsString}, mem::size_of, @@ -15,15 +18,10 @@ use std::{ }, }; use winapi::{ - shared::{minwindef::TRUE, winerror::ERROR_INSUFFICIENT_BUFFER}, + shared::minwindef::TRUE, um::{ - errhandlingapi::GetLastError, handleapi::INVALID_HANDLE_VALUE, - minwinbase::SECURITY_ATTRIBUTES, - processthreadsapi::{ - CreateProcessW, DeleteProcThreadAttributeList, InitializeProcThreadAttributeList, - UpdateProcThreadAttribute, PROCESS_INFORMATION, PROC_THREAD_ATTRIBUTE_LIST, - }, + processthreadsapi::{CreateProcessW, PROCESS_INFORMATION}, winbase::{ CreateFileMappingA, CREATE_UNICODE_ENVIRONMENT, EXTENDED_STARTUPINFO_PRESENT, STARTF_USESTDHANDLES, STARTUPINFOEXW, @@ -147,76 +145,22 @@ pub(in crate::windows) struct ChildParams { // TODO: upstream to winapi: https://github.com/retep998/winapi-rs/pull/933/ const MAGIC_PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES: usize = 131081; -struct AlignedMemBlock(*mut u8, usize); - -impl AlignedMemBlock { - fn layout(cnt: usize) -> std::alloc::Layout { - assert!(cnt > 0); - std::alloc::Layout::from_size_align(cnt, 8).unwrap() - } - - fn new(cnt: usize) -> AlignedMemBlock { - let ptr = unsafe { std::alloc::alloc_zeroed(Self::layout(cnt)) }; - AlignedMemBlock(ptr, cnt) - } - - fn ptr(&self) -> *mut u8 { - self.0 - } -} - -impl Drop for AlignedMemBlock { - fn drop(&mut self) { - unsafe { - std::alloc::dealloc(self.0, Self::layout(self.1)); - } - } -} - pub(in crate::windows) fn spawn( sandbox: &WindowsSandbox, stdio: Stdio, params: ChildParams, ) -> Result { - let proc_thread_attr_list_storage; let mut security_capabilities; + let mut proc_thread_attr_list = AttrList::new(1)?; let mut startup_info = unsafe { let mut startup_info: STARTUPINFOEXW = std::mem::zeroed(); - let mut proc_thread_attr_list_len = 0; - { - InitializeProcThreadAttributeList( - std::ptr::null_mut(), - // we need only one attribute: security capabilities. - 1, - 0, - &mut proc_thread_attr_list_len, - ); - if GetLastError() != ERROR_INSUFFICIENT_BUFFER { - return Err(Error::last()); - } - } - proc_thread_attr_list_storage = AlignedMemBlock::new(proc_thread_attr_list_len); - let proc_thread_attr_list = proc_thread_attr_list_storage.ptr(); - startup_info.lpAttributeList = proc_thread_attr_list.cast(); - Cvt::nonzero(InitializeProcThreadAttributeList( - startup_info.lpAttributeList, - 1, - 0, - &mut proc_thread_attr_list_len, - ))?; + security_capabilities = sandbox.profile.get_security_capabilities(); - Cvt::nonzero(UpdateProcThreadAttribute( - startup_info.lpAttributeList, - // reserved - 0, + + proc_thread_attr_list.add_attr::( MAGIC_PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES, - (&mut security_capabilities as *mut SECURITY_CAPABILITIES).cast(), - std::mem::size_of::(), - // reserved - std::ptr::null_mut(), - // reserved - std::ptr::null_mut(), - ))?; + &mut security_capabilities, + )?; startup_info.StartupInfo.cb = size_of::() as u32; startup_info.StartupInfo.dwFlags = STARTF_USESTDHANDLES; @@ -229,21 +173,24 @@ pub(in crate::windows) fn spawn( startup_info.StartupInfo.hStdInput = cvt_handle(stdio.stdin); startup_info.StartupInfo.hStdOutput = cvt_handle(stdio.stdout); startup_info.StartupInfo.hStdError = cvt_handle(stdio.stderr); + + startup_info.lpAttributeList = proc_thread_attr_list.borrow_ptr(); startup_info }; let creation_flags = CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT; let mut info: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; + let application_name: Vec = 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(¶ms.env); + if let EncodeEnvResult::Partial = env_status { + tracing::warn!("skipped zero chars in provided environment"); + } + let cwd: Vec = params.cwd.encode_wide().collect(); + unsafe { - let application_name: Vec = 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(¶ms.env); - if let EncodeEnvResult::Partial = env_status { - tracing::warn!("skipped zero chars in provided environment"); - } - let cwd: Vec = params.cwd.encode_wide().collect(); Cvt::nonzero(CreateProcessW( application_name.as_ptr(), cmd_line.as_mut_ptr(), @@ -259,7 +206,6 @@ pub(in crate::windows) fn spawn( (&mut startup_info as *mut STARTUPINFOEXW).cast(), &mut info, ))?; - DeleteProcThreadAttributeList(startup_info.lpAttributeList); } Ok(info) } diff --git a/src/windows/spawn/attr_list.rs b/src/windows/spawn/attr_list.rs new file mode 100644 index 00000000..5837b233 --- /dev/null +++ b/src/windows/spawn/attr_list.rs @@ -0,0 +1,112 @@ +//! RAII wrapper for ProcThreadAttrList +use std::marker::PhantomData; + +use crate::windows::{Cvt, Error}; +use winapi::{ + shared::winerror::ERROR_INSUFFICIENT_BUFFER, + um::{ + errhandlingapi::GetLastError, + processthreadsapi::{ + DeleteProcThreadAttributeList, InitializeProcThreadAttributeList, + UpdateProcThreadAttribute, PROC_THREAD_ATTRIBUTE_LIST, + }, + }, +}; + +pub struct AttrList<'a> { + storage: AlignedMemBlock, + cap: usize, + len: usize, + phantom: PhantomData<&'a mut &'a mut ()>, +} + +impl<'a> AttrList<'a> { + pub fn new(capacity: usize) -> Result { + let mut proc_thread_attr_list_len = 0; + unsafe { + InitializeProcThreadAttributeList( + std::ptr::null_mut(), + // we need only one attribute: security capabilities. + 1, + 0, + &mut proc_thread_attr_list_len, + ); + if GetLastError() != ERROR_INSUFFICIENT_BUFFER { + return Err(Error::last()); + } + } + let storage = AlignedMemBlock::new(proc_thread_attr_list_len); + unsafe { + Cvt::nonzero(InitializeProcThreadAttributeList( + storage.ptr().cast(), + 1, + 0, + &mut proc_thread_attr_list_len, + ))?; + } + + Ok(AttrList { + storage, + cap: capacity, + len: 0, + phantom: PhantomData, + }) + } + + pub fn add_attr(&mut self, attr_name: usize, attr_val: &'a mut T) -> Result<(), Error> { + assert!(self.len < self.cap); + unsafe { + Cvt::nonzero(UpdateProcThreadAttribute( + self.storage.ptr().cast(), + // reserved + 0, + attr_name, + (attr_val as *mut T).cast(), + std::mem::size_of::(), + // reserved + std::ptr::null_mut(), + // reserved + std::ptr::null_mut(), + ))?; + } + self.len += 1; + Ok(()) + } + + pub fn borrow_ptr(&self) -> *mut PROC_THREAD_ATTRIBUTE_LIST { + assert_eq!(self.len, self.cap); + self.storage.ptr().cast() + } +} + +impl<'a> Drop for AttrList<'a> { + fn drop(&mut self) { + unsafe { DeleteProcThreadAttributeList(self.storage.ptr().cast()) }; + } +} + +struct AlignedMemBlock(*mut u8, usize); + +impl AlignedMemBlock { + fn layout(cnt: usize) -> std::alloc::Layout { + assert!(cnt > 0); + std::alloc::Layout::from_size_align(cnt, 8).unwrap() + } + + fn new(cnt: usize) -> AlignedMemBlock { + let ptr = unsafe { std::alloc::alloc_zeroed(Self::layout(cnt)) }; + AlignedMemBlock(ptr, cnt) + } + + fn ptr(&self) -> *mut u8 { + self.0 + } +} + +impl Drop for AlignedMemBlock { + fn drop(&mut self) { + unsafe { + std::alloc::dealloc(self.0, Self::layout(self.1)); + } + } +} From 4c6174c47c08fc27aeabb0d5963f4a76f96d8245 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 10 Feb 2021 18:31:46 +0300 Subject: [PATCH 08/18] change log location --- .github/workflows/rust.yml | 44 ++++++++++++++++++++++++++++++-------- ci/windows-build.ps1 | 24 +++++++++++++++++++++ ci/windows.ps1 | 20 +++-------------- 3 files changed, 62 insertions(+), 26 deletions(-) create mode 100644 ci/windows-build.ps1 diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 7c5159ff..b6af481a 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -100,7 +100,33 @@ jobs: with: path: /tmp/logs name: tests-trace-${{ matrix.cgroups }}-${{ matrix.os }} - + tests-windows-build: + runs-on: 'windows-2019' + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1.0.6 + with: + # nightly is used for cargo --out-dir option. + toolchain: nightly + override: true + - name: Install targets + run: | + rustup target add x86_64-pc-windows-gnu + rustup target add x86_64-pc-windows-msvc + - uses: actions/cache@v2 + with: + path: ./target + key: ${{ runner.os }}-${{ hashFiles('Cargo.lock') }} + - name: Build tests (musl) + run: | + powershell ci/windows-builds.ps1 + timeout-minutes: 5 + - uses: actions/upload-artifact@v2 + with: + path: ./tests + name: tests-windows + retention-days: 2 + tests-windows: strategy: matrix: @@ -110,10 +136,10 @@ jobs: runs-on: windows-latest steps: - uses: actions/checkout@v2 - - uses: actions-rs/toolchain@v1.0.6 - with: - toolchain: stable - override: true + - uses: actions/download-artifact@v1 + with: + path: ./tests + name: tests-windows - name: Test run: powershell ci/windows.ps1 timeout-minutes: 5 @@ -123,13 +149,13 @@ jobs: if: always() run: | ls - mkdir D:/ci-logs - cp ./strace* D:/ci-logs - ls D:/ci-logs + mkdir ./ci-logs + cp ./strace* ./ci-logs + ls ./ci-logs - uses: actions/upload-artifact@v1 if: always() with: - path: D:/ci-logs + path: ./ci-logs name: tests-ntcalls nightly-checks: diff --git a/ci/windows-build.ps1 b/ci/windows-build.ps1 new file mode 100644 index 00000000..509a6e1e --- /dev/null +++ b/ci/windows-build.ps1 @@ -0,0 +1,24 @@ +$ErrorActionPreference = "Stop" + + +function Build-JJSForTarget { + param($TargetName) + + New-Item -ItemType directory -Path ./tests/$TargetName + cargo build -p minion-tests -Zunstable-options --out-dir=./tests/$TargetName --target=$TargetName + + if ($LASTEXITCODE -ne 0) { + throw "build failure" + } +} + +Write-Output @' +[build] +rustflags=["--cfg", "minion_ci"] +'@ | Out-File -FilePath ./.cargo/config -Encoding 'utf8' + + + +$env:RUSTC_BOOTSTRAP = 1 +Build-JJSForTarget -TargetName x86_64-pc-windows-gnu +Build-JJSForTarget -TargetName x86_64-pc-windows-msvc \ No newline at end of file diff --git a/ci/windows.ps1 b/ci/windows.ps1 index c2691e8f..633d78e5 100644 --- a/ci/windows.ps1 +++ b/ci/windows.ps1 @@ -2,24 +2,10 @@ $ErrorActionPreference = "Stop" Write-Output "::group::Info" Write-Output "Target: $env:CI_TARGET" -Write-Output "::group::Preparing" -rustup target add $env:CI_TARGET -Write-Output @' -[build] -rustflags=["--cfg", "minion_ci"] -'@ | Out-File -FilePath ./.cargo/config -Encoding 'utf8' - -Write-Output "::group::Compiling tests" -$env:RUSTC_BOOTSTRAP = 1 -cargo build -p minion-tests -Zunstable-options --out-dir=./out --target=$env:CI_TARGET - -if ($LASTEXITCODE -ne 0) { - throw "build failure" -} - Write-Output "::group::Running tests" -$env:RUST_BACKTRACE = 1 -./out/minion-tests.exe --trace + +$env:RUST_BACKTRACE = full +./tests/$env:CI_TARGET/minion-tests.exe --trace if ($LASTEXITCODE -ne 0) { throw "tests failed" } \ No newline at end of file From e92ddb26b52b0e5db56747d4262f74d746d7110f Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 10 Feb 2021 18:43:57 +0300 Subject: [PATCH 09/18] add missing needs --- .github/workflows/rust.yml | 8 +++++--- ci/windows-build.ps1 | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index b6af481a..d5482f99 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -117,9 +117,9 @@ jobs: with: path: ./target key: ${{ runner.os }}-${{ hashFiles('Cargo.lock') }} - - name: Build tests (musl) + - name: Build tests run: | - powershell ci/windows-builds.ps1 + powershell ci/windows-build.ps1 timeout-minutes: 5 - uses: actions/upload-artifact@v2 with: @@ -127,13 +127,15 @@ jobs: name: tests-windows retention-days: 2 - tests-windows: + tests-windows-run: strategy: matrix: rust-target: - 'x86_64-pc-windows-gnu' - 'x86_64-pc-windows-msvc' runs-on: windows-latest + needs: + - tests-windows-build steps: - uses: actions/checkout@v2 - uses: actions/download-artifact@v1 diff --git a/ci/windows-build.ps1 b/ci/windows-build.ps1 index 509a6e1e..4d3ac63e 100644 --- a/ci/windows-build.ps1 +++ b/ci/windows-build.ps1 @@ -5,6 +5,7 @@ function Build-JJSForTarget { param($TargetName) New-Item -ItemType directory -Path ./tests/$TargetName + Write-Output "::group::Building for $TargetName" cargo build -p minion-tests -Zunstable-options --out-dir=./tests/$TargetName --target=$TargetName if ($LASTEXITCODE -ne 0) { From 96a38463a8d92ed1af2110c54192b8ff05524950 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 10 Feb 2021 18:53:22 +0300 Subject: [PATCH 10/18] fix --- ci/windows.ps1 | 2 +- src/windows/child.rs | 4 +++- src/windows/constrain.rs | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/ci/windows.ps1 b/ci/windows.ps1 index 633d78e5..d732e1fb 100644 --- a/ci/windows.ps1 +++ b/ci/windows.ps1 @@ -4,7 +4,7 @@ Write-Output "::group::Info" Write-Output "Target: $env:CI_TARGET" Write-Output "::group::Running tests" -$env:RUST_BACKTRACE = full +$env:RUST_BACKTRACE = "full" ./tests/$env:CI_TARGET/minion-tests.exe --trace if ($LASTEXITCODE -ne 0) { throw "tests failed" diff --git a/src/windows/child.rs b/src/windows/child.rs index 4c35bece..22cb279f 100644 --- a/src/windows/child.rs +++ b/src/windows/child.rs @@ -35,9 +35,11 @@ impl WindowsChildProcess { cwd: options.pwd.into(), }, )?; + let child = OwnedHandle::new(info.hProcess); + options.sandbox.job.add_process(&child)?; Ok(WindowsChildProcess { - child: OwnedHandle::new(info.hProcess), + child, main_thread: OwnedHandle::new(info.hThread), stdin: child_stdin, stdout: child_stdout, diff --git a/src/windows/constrain.rs b/src/windows/constrain.rs index 3711b197..87713bce 100644 --- a/src/windows/constrain.rs +++ b/src/windows/constrain.rs @@ -68,11 +68,11 @@ impl Job { pub(crate) fn kill(&self) -> Result<(), Error> { unsafe { Cvt::nonzero(TerminateJobObject(self.handle.as_raw(), 0xDEADBEEF)).map(|_| ()) } } - pub(crate) fn add_process(&self, process_handle: HANDLE) -> Result<(), Error> { + pub(crate) fn add_process(&self, process_handle: &OwnedHandle) -> Result<(), Error> { unsafe { Cvt::nonzero(AssignProcessToJobObject( self.handle.as_raw(), - process_handle, + process_handle.as_raw(), )) .map(|_| ()) } From 618ab578c277a93140f89fbfb4bd536f8f09364a Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 10 Feb 2021 18:57:42 +0300 Subject: [PATCH 11/18] try quoting --- ci/windows.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/windows.ps1 b/ci/windows.ps1 index d732e1fb..bef0be36 100644 --- a/ci/windows.ps1 +++ b/ci/windows.ps1 @@ -5,7 +5,7 @@ Write-Output "Target: $env:CI_TARGET" Write-Output "::group::Running tests" $env:RUST_BACKTRACE = "full" -./tests/$env:CI_TARGET/minion-tests.exe --trace +"./tests/$env:CI_TARGET/minion-tests.exe" --trace if ($LASTEXITCODE -ne 0) { throw "tests failed" } \ No newline at end of file From 95cefaf19718081e8f48d0106063ef38f3dba4ee Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 10 Feb 2021 18:58:05 +0300 Subject: [PATCH 12/18] create suspended --- src/windows/spawn.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/windows/spawn.rs b/src/windows/spawn.rs index 5ac350ba..061cd7e2 100644 --- a/src/windows/spawn.rs +++ b/src/windows/spawn.rs @@ -23,8 +23,8 @@ use winapi::{ handleapi::INVALID_HANDLE_VALUE, processthreadsapi::{CreateProcessW, PROCESS_INFORMATION}, winbase::{ - CreateFileMappingA, CREATE_UNICODE_ENVIRONMENT, EXTENDED_STARTUPINFO_PRESENT, - STARTF_USESTDHANDLES, STARTUPINFOEXW, + CreateFileMappingA, CREATE_SUSPENDED, CREATE_UNICODE_ENVIRONMENT, + EXTENDED_STARTUPINFO_PRESENT, STARTF_USESTDHANDLES, STARTUPINFOEXW, }, winnt::{HANDLE, PAGE_READWRITE, SECURITY_CAPABILITIES}, }, @@ -177,7 +177,8 @@ pub(in crate::windows) fn spawn( startup_info.lpAttributeList = proc_thread_attr_list.borrow_ptr(); startup_info }; - let creation_flags = CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT; + let creation_flags = + CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT | CREATE_SUSPENDED; let mut info: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; let application_name: Vec = params.exe.encode_wide().collect(); let mut cmd_line = application_name.clone(); From 74345fcb789559064559233a686cb812d3654914 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 10 Feb 2021 19:02:25 +0300 Subject: [PATCH 13/18] does ampersand help& --- ci/windows.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/windows.ps1 b/ci/windows.ps1 index bef0be36..9b48639a 100644 --- a/ci/windows.ps1 +++ b/ci/windows.ps1 @@ -5,7 +5,7 @@ Write-Output "Target: $env:CI_TARGET" Write-Output "::group::Running tests" $env:RUST_BACKTRACE = "full" -"./tests/$env:CI_TARGET/minion-tests.exe" --trace +& "./tests/$env:CI_TARGET/minion-tests.exe" --trace if ($LASTEXITCODE -ne 0) { throw "tests failed" } \ No newline at end of file From c143561e688d100bff6d424722bc81361a42f92b Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 10 Feb 2021 19:07:32 +0300 Subject: [PATCH 14/18] update bors config --- bors.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bors.toml b/bors.toml index a864d031..2154ec6a 100644 --- a/bors.toml +++ b/bors.toml @@ -4,6 +4,8 @@ status = [ 'clippy', 'tests-linux-run (ubuntu-20.04, x86_64-unknown-linux-musl, cgroup-v1)', 'tests-linux-run (macos-latest, x86_64-unknown-linux-musl, cgroup-v2)', +'tests-windows-run (x86_64-pc-windows-gnu)', +'tests-windows-run (x86_64-pc-windows-msvc)', 'nightly-checks', ] delete-merged-branches = true From 11315e6ffc7315466db6c9c8c39dfaf43c75bbf3 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 10 Feb 2021 19:21:20 +0300 Subject: [PATCH 15/18] more logging --- src/windows/sandbox.rs | 2 +- src/windows/spawn.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/windows/sandbox.rs b/src/windows/sandbox.rs index 120ff687..d35df750 100644 --- a/src/windows/sandbox.rs +++ b/src/windows/sandbox.rs @@ -5,7 +5,7 @@ use tracing::instrument; pub struct WindowsSandbox { pub(crate) job: Job, pub(crate) profile: Profile, - id: String, + pub(crate) id: String, } impl WindowsSandbox { diff --git a/src/windows/spawn.rs b/src/windows/spawn.rs index 061cd7e2..35ee87eb 100644 --- a/src/windows/spawn.rs +++ b/src/windows/spawn.rs @@ -30,6 +30,7 @@ use winapi::{ }, }; +#[derive(Debug)] pub(in crate::windows) struct Stdio { pub stdin: Option, pub stdout: Option, @@ -134,6 +135,7 @@ fn open_empty_readable_file() -> OwnedHandle { .expect("failed to clone a handle") } +#[derive(Debug)] pub(in crate::windows) struct ChildParams { pub exe: OsString, /// Does not contain argv[0] - will be prepended automatically. @@ -145,11 +147,13 @@ pub(in crate::windows) struct ChildParams { // TODO: upstream to winapi: https://github.com/retep998/winapi-rs/pull/933/ const MAGIC_PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES: usize = 131081; +#[tracing::instrument(skip(sandbox, stdio, params), fields(sandbox_id = sandbox.id.as_str()))] pub(in crate::windows) fn spawn( sandbox: &WindowsSandbox, stdio: Stdio, params: ChildParams, ) -> Result { + tracing::info!(params = ?params, sandbox = ?sandbox, stdio = ?stdio, "Creating child process"); let mut security_capabilities; let mut proc_thread_attr_list = AttrList::new(1)?; let mut startup_info = unsafe { From 7ef1e81f1f7d943284a80669faf22ec5794378cf Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 10 Feb 2021 19:22:02 +0300 Subject: [PATCH 16/18] change to dbug --- src/windows/spawn.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windows/spawn.rs b/src/windows/spawn.rs index 35ee87eb..d4b81fe6 100644 --- a/src/windows/spawn.rs +++ b/src/windows/spawn.rs @@ -153,7 +153,7 @@ pub(in crate::windows) fn spawn( stdio: Stdio, params: ChildParams, ) -> Result { - tracing::info!(params = ?params, sandbox = ?sandbox, stdio = ?stdio, "Creating child process"); + tracing::debug!(params = ?params, sandbox = ?sandbox, stdio = ?stdio, "Creating child process"); let mut security_capabilities; let mut proc_thread_attr_list = AttrList::new(1)?; let mut startup_info = unsafe { From f28ffb002f98fcea1baa823681fb56ad78f05021 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 10 Feb 2021 19:28:50 +0300 Subject: [PATCH 17/18] collect all lgos --- minion-tests/src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/minion-tests/src/main.rs b/minion-tests/src/main.rs index da80c2f2..4a4579a9 100644 --- a/minion-tests/src/main.rs +++ b/minion-tests/src/main.rs @@ -66,6 +66,7 @@ fn main() { tracing_subscriber::fmt() .pretty() .with_writer(std::io::stderr) + .with_max_level(tracing_subscriber::filter::LevelFilter::TRACE) .init(); let test_cases = &*tests::TESTS; let role = get_role(); From ac70acc8ba2cccc0199f90484a006393f2c5ed82 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 10 Feb 2021 19:37:34 +0300 Subject: [PATCH 18/18] add a hack --- minion-tests/src/worker.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/minion-tests/src/worker.rs b/minion-tests/src/worker.rs index 62750cfa..7f8167ea 100644 --- a/minion-tests/src/worker.rs +++ b/minion-tests/src/worker.rs @@ -30,7 +30,12 @@ async fn inner_main(test_cases: &[&'static dyn TestCase]) { }; let sandbox = backend.new_sandbox(opts).expect("can not create sandbox"); let opts = minion::ChildProcessOptions { - path: "/me".into(), + path: if cfg!(target_os = "linux") { + "/me".into() + } else { + // TEMPORARY WIP-ONLY HACK + std::env::current_exe().unwrap() + }, arguments: vec![test_case.name().into()], environment: vec![format!("{}=1", crate::TEST_ENV_NAME).into()], stdio: minion::StdioSpecification {