cleanup namespace creation

src/daemon/link.rs

@@ -9,6 +9,12 @@ use neli::router::synchronous::NlRouter;
 use neli::rtnl::Ifinfomsg;
 use neli::rtnl::IfinfomsgBuilder;
 use neli::utils::Groups;
+use nix::errno::Errno;
+use nix::sched::{CloneFlags, setns};
+use std::fs::File;
+use std::path::{Path, PathBuf};
+use std::thread::spawn;
+use std::io::Error as IoError;
 use thiserror::Error;
 
 #[derive(Debug, Error)]
@@ -17,6 +23,12 @@ pub enum LinkError {
     Netlink,
     #[error("failed to code netlink response")]
     Parse,
+    #[error("unexpected panic in link setup")]
+    Panic,
+    #[error("failed to enter namespace in link setup: {0}")]
+    Namespace(Errno),
+    #[error("Failed to open namespace file {}: {error:#}", path.display())]
+    OpenNamespace { path: PathBuf, error: IoError },
 }
 
 impl<T, P> From<RouterError<T, P>> for LinkError {
@@ -25,8 +37,24 @@ impl<T, P> From<RouterError<T, P>> for LinkError {
     }
 }
 
+/// Set a link to UP inside a namespace
+pub fn link_up_ns(namespace: impl AsRef<Path>, link_name: &'static str) -> Result<(), LinkError> {
+    let namespace = namespace.as_ref();
+    let ns_handle = File::open(namespace).map_err(|error| LinkError::OpenNamespace {
+        error,
+        path: namespace.into(),
+    })?;
+
+    spawn(move || {
+        setns(ns_handle, CloneFlags::CLONE_NEWNET).map_err(LinkError::Namespace)?;
+        link_up(link_name)
+    })
+    .join()
+    .map_err(|_| LinkError::Panic)?
+}
+
 /// Set a link to UP
-pub fn up(link_name: &str) -> Result<(), LinkError> {
+pub fn link_up(link_name: &str) -> Result<(), LinkError> {
     // I honestly don't really know how this code works
     // It's mostly a copy from one of neli's examples and seems to do what it needs to
     let (rtnl, _) = NlRouter::connect(NlFamily::Route, None, Groups::empty())?;

src/daemon/mod.rs

@@ -148,7 +148,7 @@ struct ActiveNamespace {
 
 impl ActiveNamespace {
     pub fn new(config: &NamespaceConfig) -> Result<Self, DaemonError> {
-        let ns = NetNs::new(&config.name)?;
+        let ns = NetNs::new(config.name.clone())?;
 
         let mut namespace = ActiveNamespace {
             ns,

src/daemon/namespace.rs

@@ -1,132 +0,0 @@
-use crate::config::NamespaceName;
-use nix::errno::Errno;
-use nix::mount::{mount, umount, MsFlags};
-use nix::sched::{clone, CloneFlags};
-use nix::sys::signal::Signal;
-use nix::sys::wait::{waitpid, WaitStatus};
-use std::fs::{create_dir_all, remove_file, File};
-use std::io::{Error as IoError, ErrorKind};
-use std::path::{Path, PathBuf};
-use thiserror::Error;
-use tracing::{error, info};
-
-pub struct NetNs {
-    name: NamespaceName,
-    path: PathBuf,
-}
-
-impl NetNs {
-    /// Create a new named network namespace that will be removed when dropped
-    pub fn new(name: &NamespaceName) -> Result<Self, NamespaceError> {
-        info!(%name, "creating network namespace");
-        let parent = Path::new("/var/run/netns");
-        create_dir_all(parent).map_err(NamespaceError::Parent)?;
-        let path = parent.join(name);
-        let mount_path = path.clone();
-
-        match File::create_new(&path) {
-            Ok(_) => {}
-            Err(e) if e.kind() == ErrorKind::AlreadyExists => {
-                return Ok(NetNs {
-                    name: name.clone(),
-                    path,
-                });
-            }
-            Err(e) => return Err(NamespaceError::from_create(name, e)),
-        }
-
-        let mut stack = vec![0; 8 * 1024 * 1024];
-        let pid = unsafe {
-            clone(
-                Box::new(move || {
-                    if let Err(e) = mount(
-                        Some("/proc/self/ns/net"),
-                        &mount_path,
-                        Option::<&str>::None,
-                        MsFlags::MS_BIND,
-                        Option::<&str>::None,
-                    ) {
-                        return e as i32 as isize;
-                    }
-
-                    if let Err(error) = super::link::up("lo") {
-                        error!(%error, "error setting link up");
-                        return 1;
-                    }
-
-                    0
-                }),
-                &mut stack,
-                CloneFlags::CLONE_NEWNET,
-                Some(Signal::SIGCHLD as i32),
-            )
-        }
-        .map_err(NamespaceError::Clone)?;
-
-        match waitpid(pid, None).map_err(NamespaceError::Wait)? {
-            WaitStatus::Exited(_, exit) => {
-                if exit > 0 {
-                    if let Err(error) = remove_file(&path) {
-                        error!(%error, path = %path.display(), "Failed to remove namespace file after mount failure");
-                    }
-                    return Err(NamespaceError::from_mount(Errno::from_raw(exit)));
-                }
-            }
-            status => {
-                error!(?status, "unexpected wait status");
-            }
-        }
-
-        Ok(NetNs {
-            name: name.clone(),
-            path,
-        })
-    }
-}
-
-impl NetNs {
-    pub fn name(&self) -> &NamespaceName {
-        &self.name
-    }
-}
-
-impl Drop for NetNs {
-    fn drop(&mut self) {
-        let name = self.path.file_name().unwrap().to_str().unwrap();
-        info!(name, "deleting network namespace");
-        if let Err(error) = umount(&self.path) {
-            error!(%error, path = %self.path.display(), "Failed to unmount network namespace");
-        }
-        if let Err(error) = remove_file(&self.path) {
-            error!(%error, path = %self.path.display(), "Failed to remove namespace file");
-        }
-    }
-}
-
-#[derive(Debug, Error)]
-pub enum NamespaceError {
-    #[error("Failed to create parent directory for namespaces (/var/run/netns): {0:#}")]
-    Parent(IoError),
-    #[error("Failed to create namespace file {}: {error:#}", path.display())]
-    Create { path: PathBuf, error: IoError },
-    #[error("Unexpected error while creating new network namespace with clone: {0:}")]
-    Clone(Errno),
-    #[error("Unexpected error while binding new network namespace: {0:}")]
-    Bind(Errno),
-    #[error("Unexpected error while waiting for network namespace thread: {0:}")]
-    Wait(Errno),
-}
-
-impl NamespaceError {
-    fn from_mount(errno: Errno) -> Self {
-        // todo more specific errors?
-        NamespaceError::Bind(errno)
-    }
-
-    fn from_create(name: &NamespaceName, error: IoError) -> Self {
-        NamespaceError::Create {
-            path: Path::new("/var/run/netns").join(name),
-            error,
-        }
-    }
-}

src/daemon/namespace/mod.rs

@@ -0,0 +1,110 @@
+mod raw;
+
+use crate::config::NamespaceName;
+use crate::daemon::link::{LinkError, link_up_ns};
+use crate::daemon::namespace::raw::{
+    NamespaceSetupError, create_network_namespace,
+};
+use nix::errno::Errno;
+use nix::mount::{MsFlags, mount, umount};
+use std::fs::{File, create_dir_all, remove_file};
+use std::io::{Error as IoError, ErrorKind};
+use std::path::{Path, PathBuf};
+use thiserror::Error;
+use tracing::{error, info};
+
+pub struct NetNs {
+    name: NamespaceName,
+    path: PathBuf,
+}
+
+impl NetNs {
+    /// Create a new named network namespace that will be removed when dropped
+    pub fn new(name: NamespaceName) -> Result<Self, NamespaceError> {
+        info!(%name, "creating network namespace");
+        let parent = Path::new("/var/run/netns");
+        create_dir_all(parent).map_err(NamespaceError::Parent)?;
+        let path = parent.join(&name);
+
+        match File::create_new(&path) {
+            Ok(_) => {}
+            Err(e) if e.kind() == ErrorKind::AlreadyExists => {
+                return Ok(NetNs {
+                    name: name.clone(),
+                    path,
+                });
+            }
+            Err(e) => return Err(NamespaceError::from_create(path.clone(), e)),
+        }
+
+        let ns = create_network_namespace(move |ns| {
+            bind_namespace(&ns, &path)?;
+            Result::<_, NamespaceError>::Ok(NetNs {
+                name,
+                path,
+            })
+        })?;
+
+        ns.setup_interfaces()?;
+
+        Ok(ns)
+    }
+}
+
+fn bind_namespace(namespace: &Path, path: &Path) -> Result<(), NamespaceError> {
+    mount(
+        Some(namespace.as_os_str()),
+        path.as_os_str(),
+        Option::<&str>::None,
+        MsFlags::MS_BIND,
+        Option::<&str>::None,
+    )
+    .map_err(NamespaceError::Mount)
+}
+
+impl NetNs {
+    pub fn name(&self) -> &NamespaceName {
+        &self.name
+    }
+
+    fn setup_interfaces(&self) -> Result<(), NamespaceError> {
+        link_up_ns(&self.path, "lo")?;
+        Ok(())
+    }
+}
+
+impl Drop for NetNs {
+    fn drop(&mut self) {
+        let name = self.path.file_name().unwrap().to_str().unwrap();
+        info!(name, "deleting network namespace");
+        if let Err(error) = umount(&self.path) {
+            error!(%error, path = %self.path.display(), "Failed to unmount network namespace");
+        }
+        if let Err(error) = remove_file(&self.path) {
+            error!(%error, path = %self.path.display(), "Failed to remove namespace file");
+        }
+    }
+}
+
+#[derive(Debug, Error)]
+pub enum NamespaceError {
+    #[error("Failed to create parent directory for namespaces (/var/run/netns): {0:#}")]
+    Parent(IoError),
+    #[error("Failed to create namespace file {}: {error:#}", path.display())]
+    Create { path: PathBuf, error: IoError },
+    #[error("Failed to setup namespace: {0:#}")]
+    Setup(#[from] NamespaceSetupError),
+    #[error("Failed to bind-mount netns handle: {0:?}")]
+    Mount(Errno),
+    #[error("Failed to setup loopback inside namespace: {0:#}")]
+    Link(#[from] LinkError),
+}
+
+impl NamespaceError {
+    fn from_create(path: PathBuf, error: IoError) -> Self {
+        NamespaceError::Create {
+            path,
+            error,
+        }
+    }
+}

src/daemon/namespace/raw.rs

@@ -0,0 +1,49 @@
+use nix::errno::Errno;
+use nix::sched::{CloneFlags, clone};
+use nix::sys::signal::Signal;
+use nix::sys::wait::{WaitStatus, waitpid};
+use std::path::PathBuf;
+use thiserror::Error;
+use tracing::error;
+
+#[derive(Debug, Error)]
+pub enum NamespaceSetupError {
+    #[error("Failed to spawn netns thread: {0}")]
+    Spawn(Errno),
+    #[error("Failed to wait for netns thread exit: {0}")]
+    Wait(Errno),
+    #[error("Unexpected status for netns thread: {0:?}")]
+    Status(WaitStatus),
+}
+
+/// Create a new network namespace and call the provided handler with it's path
+///
+/// the namespace will be cleaned up after the handler finishes unless the handler bind mounts the path
+pub fn create_network_namespace<T, E: From<NamespaceSetupError>>(
+    handler: impl FnOnce(PathBuf) -> Result<T, E>,
+) -> Result<T, E> {
+    let mut stack = vec![0; 8 * 1024];
+    // Safety: we don't execute any code in the new thread
+    // so we use no stack space and don't call non signal-safe syscalls.
+    let pid = unsafe {
+        clone(
+            Box::new(|| 0),
+            &mut stack,
+            CloneFlags::CLONE_NEWNET,
+            Some(Signal::SIGCHLD as i32),
+        )
+    }
+    .map_err(NamespaceSetupError::Spawn)?;
+
+    let path = PathBuf::from(format!("/proc/{}/ns/net", pid));
+
+    let result = handler(path);
+
+    match waitpid(pid, None) {
+        Err(error) => Err(NamespaceSetupError::Wait(error)),
+        Ok(WaitStatus::Exited(_, _)) => Ok(()),
+        Ok(status) => Err(NamespaceSetupError::Status(status)),
+    }?;
+
+    result
+}