use a wrapper for stdio redirection

Signed-off-by: Jorge Prendes <[email protected]>

crates/containerd-shim-wasm/src/libcontainer_instance/container_executor.rs

@@ -1,38 +1,31 @@
 use std::fs::OpenOptions;
 use std::io::Read;
-use std::os::fd::RawFd;
 use std::path::PathBuf;
 
-use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO};
 use libcontainer::workload::default::DefaultExecutor;
 use libcontainer::workload::{Executor, ExecutorError};
-use nix::unistd::{dup, dup2};
 use oci_spec::runtime::Spec;
 
-use crate::sandbox::oci;
+use crate::sandbox::{oci, Stdio};
 
 #[derive(Default)]
 pub struct LinuxContainerExecutor {
-    stdin: Option<RawFd>,
-    stdout: Option<RawFd>,
-    stderr: Option<RawFd>,
+    stdio: Stdio,
     default_executor: DefaultExecutor,
 }
 
 impl LinuxContainerExecutor {
-    pub fn new(stdin: Option<RawFd>, stdout: Option<RawFd>, stderr: Option<RawFd>) -> Self {
+    pub fn new(stdio: Stdio) -> Self {
         Self {
-            stdin,
-            stdout,
-            stderr,
+            stdio,
             ..Default::default()
         }
     }
 }
 
 impl Executor for LinuxContainerExecutor {
     fn exec(&self, spec: &Spec) -> Result<(), ExecutorError> {
-        redirect_io(self.stdin, self.stdout, self.stderr).map_err(|err| {
+        self.stdio.redirect().map_err(|err| {
             log::error!("failed to redirect io: {}", err);
             ExecutorError::Other(format!("failed to redirect io: {}", err))
         })?;
@@ -118,19 +111,3 @@ impl Executor for LinuxContainerExecutor {
         self.default_executor.name()
     }
 }
-
-fn redirect_io(stdin: Option<i32>, stdout: Option<i32>, stderr: Option<i32>) -> anyhow::Result<()> {
-    if let Some(stdin) = stdin {
-        dup(STDIN_FILENO)?;
-        dup2(stdin, STDIN_FILENO)?;
-    }
-    if let Some(stdout) = stdout {
-        dup(STDOUT_FILENO)?;
-        dup2(stdout, STDOUT_FILENO)?;
-    }
-    if let Some(stderr) = stderr {
-        dup(STDERR_FILENO)?;
-        dup2(stderr, STDERR_FILENO)?;
-    }
-    Ok(())
-}

crates/containerd-shim-wasm/src/sandbox/mod.rs

@@ -7,11 +7,13 @@ pub mod instance;
 pub mod instance_utils;
 pub mod manager;
 pub mod shim;
+pub mod stdio;
 
 pub use error::{Error, Result};
 pub use instance::{Instance, InstanceConfig};
 pub use manager::{Sandbox as SandboxService, Service as ManagerService};
 pub use shim::{Cli as ShimCli, Local};
+pub use stdio::Stdio;
 
 pub mod oci;
 

crates/containerd-shim-wasm/src/sandbox/stdio.rs

@@ -0,0 +1,70 @@
+#[cfg_attr(unix, path = "sys/unix/stdio.rs")]
+#[cfg_attr(windows, path = "sys/windows/stdio.rs")]
+mod stdio_impl;
+
+use std::fs::{File, OpenOptions};
+use std::io::ErrorKind::NotFound;
+use std::io::{Error, Result};
+use std::path::Path;
+use std::sync::{Arc, Mutex};
+
+use stdio_impl::*;
+
+#[derive(Default, Clone)]
+pub struct Stdio {
+    pub stdin: Stdin,
+    pub stdout: Stdout,
+    pub stderr: Stderr,
+}
+
+impl Stdio {
+    pub fn redirect(&self) -> Result<()> {
+        self.stdin.redirect()?;
+        self.stdout.redirect()?;
+        self.stderr.redirect()?;
+        Ok(())
+    }
+}
+
+macro_rules! stdio_impl {
+    ( $stdio_type:ident, $fd:expr ) => {
+        #[derive(Default, Clone)]
+        pub struct $stdio_type(Arc<Mutex<Option<File>>>);
+
+        impl<P: AsRef<Path>> TryFrom<Option<P>> for $stdio_type {
+            type Error = std::io::Error;
+            fn try_from(path: Option<P>) -> Result<Self> {
+                path.and_then(|path| match path.as_ref() {
+                    path if path.as_os_str().is_empty() => None,
+                    path => Some(path.to_owned()),
+                })
+                .map(
+                    |path| match OpenOptions::new().read(true).write(true).open(path) {
+                        Err(err) if err.kind() == NotFound => Ok(None),
+                        Ok(f) => Ok(Some(f)),
+                        Err(err) => Err(err),
+                    },
+                )
+                .transpose()
+                .map(|opt| Self(Arc::new(Mutex::new(opt.flatten()))))
+            }
+        }
+
+        impl $stdio_type {
+            pub fn redirect(&self) -> Result<()> {
+                if let Some(f) = self.0.try_lock().ok().and_then(|mut f| f.take()) {
+                    let f = try_into_fd(f)?;
+                    let _ = unsafe { libc::dup($fd) };
+                    if unsafe { libc::dup2(f.as_raw_fd(), $fd) } == -1 {
+                        return Err(Error::last_os_error());
+                    }
+                }
+                Ok(())
+            }
+        }
+    };
+}
+
+stdio_impl!(Stdin, STDIN_FILENO);
+stdio_impl!(Stdout, STDOUT_FILENO);
+stdio_impl!(Stderr, STDERR_FILENO);

crates/containerd-shim-wasm/src/sandbox/sys/unix/stdio.rs

@@ -0,0 +1,9 @@
+use std::io::Result;
+pub use std::os::fd::AsRawFd as StdioAsRawFd;
+use std::os::fd::OwnedFd;
+
+pub use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO};
+
+pub fn try_into_fd(f: impl Into<OwnedFd>) -> Result<impl StdioAsRawFd> {
+    Ok(f.into())
+}

crates/containerd-shim-wasm/src/sandbox/sys/windows/stdio.rs

@@ -0,0 +1,39 @@
+use std::io::ErrorKind::Other;
+use std::io::{Error, Result};
+use std::os::windows::prelude::{AsRawHandle, IntoRawHandle, OwnedHandle};
+
+use libc::{c_int, close, intptr_t, open_osfhandle, O_APPEND};
+
+type StdioRawFd = libc::c_int;
+
+pub static STDIN_FILENO: StdioRawFd = 0;
+pub static STDOUT_FILENO: StdioRawFd = 1;
+pub static STDERR_FILENO: StdioRawFd = 2;
+
+struct StdioOwnedFd(c_int);
+
+pub fn try_into_fd(f: impl Into<OwnedHandle>) -> Result<impl StdioAsRawFd> {
+    let handle = f.into();
+    let fd = unsafe { open_osfhandle(handle.as_raw_handle() as intptr_t, O_APPEND) };
+    if fd == -1 {
+        return Err(Error::new(Other, "Failed to open file descriptor"));
+    }
+    let _ = handle.into_raw_handle(); // drop ownership of the handle, it's managed by fd now
+    Ok(StdioOwnedFd(fd))
+}
+
+pub trait StdioAsRawFd {
+    fn as_raw_fd(&self) -> c_int;
+}
+
+impl StdioAsRawFd for StdioOwnedFd {
+    fn as_raw_fd(&self) -> c_int {
+        self.0
+    }
+}
+
+impl Drop for StdioOwnedFd {
+    fn drop(&mut self) {
+        unsafe { close(self.0) };
+    }
+}

crates/containerd-shim-wasmedge/src/executor.rs

@@ -1,31 +1,22 @@
-use std::os::unix::io::RawFd;
 use std::path::PathBuf;
 
 use anyhow::Result;
-use containerd_shim_wasm::sandbox::oci;
-use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO};
+use containerd_shim_wasm::sandbox::{oci, Stdio};
 use libcontainer::workload::{Executor, ExecutorError};
 use log::debug;
-use nix::unistd::{dup, dup2};
 use oci_spec::runtime::Spec;
 use wasmedge_sdk::config::{CommonConfigOptions, ConfigBuilder, HostRegistrationConfigOptions};
 use wasmedge_sdk::{params, VmBuilder};
 
 const EXECUTOR_NAME: &str = "wasmedge";
 
 pub struct WasmEdgeExecutor {
-    stdin: Option<RawFd>,
-    stdout: Option<RawFd>,
-    stderr: Option<RawFd>,
+    stdio: Stdio,
 }
 
 impl WasmEdgeExecutor {
-    pub fn new(stdin: Option<RawFd>, stdout: Option<RawFd>, stderr: Option<RawFd>) -> Self {
-        Self {
-            stdin,
-            stdout,
-            stderr,
-        }
+    pub fn new(stdio: Stdio) -> Self {
+        Self { stdio }
     }
 }
 
@@ -102,18 +93,9 @@ impl WasmEdgeExecutor {
         let vm = vm
             .register_module_from_file("main", module_name)
             .map_err(|err| ExecutorError::Execution(err))?;
-        if let Some(stdin) = self.stdin {
-            dup(STDIN_FILENO)?;
-            dup2(stdin, STDIN_FILENO)?;
-        }
-        if let Some(stdout) = self.stdout {
-            dup(STDOUT_FILENO)?;
-            dup2(stdout, STDOUT_FILENO)?;
-        }
-        if let Some(stderr) = self.stderr {
-            dup(STDERR_FILENO)?;
-            dup2(stderr, STDERR_FILENO)?;
-        }
+
+        self.stdio.redirect()?;
+
         Ok(vm)
     }
 }

crates/containerd-shim-wasmedge/src/instance/instance_linux.rs

@@ -2,20 +2,17 @@ use std::fs;
 use std::fs::File;
 use std::io::prelude::*;
 use std::io::ErrorKind;
-use std::os::fd::IntoRawFd;
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Condvar, Mutex};
 
-use anyhow::{Context, Result};
+use anyhow::Result;
 use containerd_shim_wasm::libcontainer_instance::{LibcontainerInstance, LinuxContainerExecutor};
 use containerd_shim_wasm::sandbox::error::Error;
 use containerd_shim_wasm::sandbox::instance::ExitCode;
-use containerd_shim_wasm::sandbox::instance_utils::maybe_open_stdio;
-use containerd_shim_wasm::sandbox::InstanceConfig;
+use containerd_shim_wasm::sandbox::{InstanceConfig, Stdio};
 use libcontainer::container::builder::ContainerBuilder;
 use libcontainer::container::Container;
 use libcontainer::syscall::syscall::create_syscall;
-use nix::unistd::close;
 use serde::{Deserialize, Serialize};
 
 use crate::executor::WasmEdgeExecutor;
@@ -27,9 +24,7 @@ pub struct Wasi {
 
     exit_code: ExitCode,
 
-    stdin: String,
-    stdout: String,
-    stderr: String,
+    stdio: Stdio,
     bundle: String,
 
     rootdir: PathBuf,
@@ -70,9 +65,11 @@ impl LibcontainerInstance for Wasi {
             id,
             rootdir: determine_rootdir(bundle.as_str(), namespace).unwrap(),
             exit_code: Arc::new((Mutex::new(None), Condvar::new())),
-            stdin: cfg.get_stdin().unwrap_or_default(),
-            stdout: cfg.get_stdout().unwrap_or_default(),
-            stderr: cfg.get_stderr().unwrap_or_default(),
+            stdio: Stdio {
+                stdin: cfg.get_stdin().try_into().unwrap(),
+                stdout: cfg.get_stdout().try_into().unwrap(),
+                stderr: cfg.get_stderr().try_into().unwrap(),
+            },
             bundle,
         }
     }
@@ -92,20 +89,10 @@ impl LibcontainerInstance for Wasi {
     fn build_container(&self) -> std::result::Result<Container, Error> {
         fs::create_dir_all(&self.rootdir)?;
 
-        let stdin = maybe_open_stdio(self.stdin.as_str())
-            .context("could not open stdin")?
-            .map(|f| f.into_raw_fd());
-        let stdout = maybe_open_stdio(self.stdout.as_str())
-            .context("could not open stdout")?
-            .map(|f| f.into_raw_fd());
-        let stderr = maybe_open_stdio(self.stderr.as_str())
-            .context("could not open stderr")?
-            .map(|f| f.into_raw_fd());
-
         let syscall = create_syscall();
         let err_others = |err| Error::Others(format!("failed to create container: {}", err));
-        let default_executor = Box::new(LinuxContainerExecutor::new(stdin, stdout, stderr));
-        let wasmedge_executor = Box::new(WasmEdgeExecutor::new(stdin, stdout, stderr));
+        let default_executor = Box::new(LinuxContainerExecutor::new(self.stdio.clone()));
+        let wasmedge_executor = Box::new(WasmEdgeExecutor::new(self.stdio.clone()));
 
         let container = ContainerBuilder::new(self.id.clone(), syscall.as_ref())
             .with_executor(vec![default_executor, wasmedge_executor])
@@ -116,11 +103,6 @@ impl LibcontainerInstance for Wasi {
             .with_systemd(false)
             .build()
             .map_err(err_others)?;
-        // Close the fds now that they have been passed to the container process
-        // so that we don't leak them.
-        stdin.map(close);
-        stdout.map(close);
-        stderr.map(close);
 
         Ok(container)
     }