bugfix: residual fds of netlink socket and tun

Signed-off-by: Zhang Tianyang <burning9699@gmail.com>
kuasar-io · Apr 7, 2024 · caeb01c · caeb01c
1 parent 1f870f7
commit caeb01c
Show file tree

Hide file tree

Showing 8 changed files with 102 additions and 82 deletions.
diff --git a/vmm/sandbox/src/cloud_hypervisor/mod.rs b/vmm/sandbox/src/cloud_hypervisor/mod.rs
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-use std::{os::unix::io::RawFd, process::Stdio, time::Duration};
+use std::{os::fd::OwnedFd, process::Stdio, time::Duration};
 
 use anyhow::anyhow;
 use async_trait::async_trait;
@@ -72,7 +72,8 @@ pub struct CloudHypervisorVM {
     wait_chan: Option<Receiver<(u32, i128)>>,
     #[serde(skip)]
     client: Option<ChClient>,
-    fds: Vec<RawFd>,
+    #[serde(skip)]
+    fds: Vec<OwnedFd>,
     pids: Pids,
 }
 
@@ -142,7 +143,7 @@ impl CloudHypervisorVM {
         Ok(pid)
     }
 
-    fn append_fd(&mut self, fd: RawFd) -> usize {
+    fn append_fd(&mut self, fd: OwnedFd) -> usize {
         self.fds.push(fd);
         self.fds.len() - 1 + 3
     }
@@ -175,17 +176,19 @@ impl VM for CloudHypervisorVM {
             params.push("-vv".to_string());
         }
 
-        let mut cmd = tokio::process::Command::new(&self.config.path);
-        cmd.args(params.as_slice());
-
-        set_cmd_fd(&mut cmd, self.fds.to_vec())?;
-        set_cmd_netns(&mut cmd, self.netns.to_string())?;
-        cmd.stdout(Stdio::piped());
-        cmd.stderr(Stdio::piped());
-        info!("start cloud hypervisor with cmdline: {:?}", cmd);
-        let child = cmd
-            .spawn()
-            .map_err(|e| anyhow!("failed to spawn cloud hypervisor command: {}", e))?;
+        // Drop cmd immediately to let the fds in pre_exec be closed.
+        let child = {
+            let mut cmd = tokio::process::Command::new(&self.config.path);
+            cmd.args(params.as_slice());
+
+            set_cmd_fd(&mut cmd, self.fds.drain(..).collect())?;
+            set_cmd_netns(&mut cmd, self.netns.to_string())?;
+            cmd.stdout(Stdio::piped());
+            cmd.stderr(Stdio::piped());
+            info!("start cloud hypervisor with cmdline: {:?}", cmd);
+            cmd.spawn()
+                .map_err(|e| anyhow!("failed to spawn cloud hypervisor command: {}", e))?
+        };
         let pid = child.id();
         self.pids.vmm_pid = pid;
         let pid_file = format!("{}/pid", self.base_dir);

diff --git a/vmm/sandbox/src/device.rs b/vmm/sandbox/src/device.rs
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-use std::os::unix::io::RawFd;
+use std::os::fd::OwnedFd;
 
 use containerd_sandbox::error::{Error, Result};
 
@@ -182,49 +182,46 @@ impl Transport {
     }
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub enum DeviceInfo {
     Block(BlockDeviceInfo),
-    #[allow(dead_code)]
     Tap(TapDeviceInfo),
-    #[allow(dead_code)]
     Physical(PhysicalDeviceInfo),
-    #[allow(dead_code)]
     VhostUser(VhostUserDeviceInfo),
     Char(CharDeviceInfo),
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct BlockDeviceInfo {
     pub id: String,
     pub path: String,
     pub read_only: bool,
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct TapDeviceInfo {
     pub id: String,
     pub index: u32,
     pub name: String,
     pub mac_address: String,
-    pub fds: Vec<RawFd>,
+    pub fds: Vec<OwnedFd>,
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct PhysicalDeviceInfo {
     pub id: String,
     pub bdf: String,
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct VhostUserDeviceInfo {
     pub id: String,
     pub socket_path: String,
     pub mac_address: String,
     pub r#type: String,
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct CharDeviceInfo {
     pub id: String,
     pub chardev_id: String,
@@ -235,6 +232,5 @@ pub struct CharDeviceInfo {
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub enum CharBackendType {
     Pipe(String),
-    #[allow(dead_code)]
     Socket(String),
 }
diff --git a/vmm/sandbox/src/network/link.rs b/vmm/sandbox/src/network/link.rs
@@ -324,19 +324,19 @@ impl NetworkInterface {
         Ok(())
     }
 
-    pub async fn attach_to<V: VM>(&self, sandbox: &mut KuasarSandbox<V>) -> Result<()> {
+    pub async fn attach_to<V: VM>(&mut self, sandbox: &mut KuasarSandbox<V>) -> Result<()> {
         let id = format!("intf-{}", self.index);
         match &self.r#type {
             LinkType::Veth => {
-                if let Some(intf) = &self.twin {
+                if let Some(intf) = self.twin.as_mut() {
                     sandbox
                         .vm
                         .attach(DeviceInfo::Tap(TapDeviceInfo {
                             id,
                             index: self.index,
                             name: intf.name.to_string(),
                             mac_address: self.mac_address.to_string(),
-                            fds: intf.fds.iter().map(|fd| fd.as_raw_fd()).collect(),
+                            fds: intf.fds.drain(..).collect(),
                         }))
                         .await?;
                 } else {
@@ -470,6 +470,7 @@ fn get_bdf_for_eth(if_name: &str) -> Result<String> {
             e
         )
     })?;
+    close(sock).unwrap_or_default();
     Ok(bdf.to_string())
 }
 

diff --git a/vmm/sandbox/src/qemu/devices/vsock.rs b/vmm/sandbox/src/qemu/devices/vsock.rs
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-use std::os::unix::io::{IntoRawFd, RawFd};
+use std::os::fd::{AsRawFd, OwnedFd};
 
 use anyhow::anyhow;
 use containerd_sandbox::error::Result;
@@ -60,18 +60,18 @@ impl VSockDevice {
     }
 }
 
-pub async fn find_context_id() -> Result<(RawFd, u64)> {
+pub async fn find_context_id() -> Result<(OwnedFd, u64)> {
     // TODO make sure if this thread_rng is enough, if we should new a seedable rng everytime.
     let vsock_file = tokio::fs::OpenOptions::new()
         .read(true)
         .write(true)
         .mode(0o666)
         .open(VHOST_VSOCK_DEV_PATH)
         .await?;
-    let vsockfd = vsock_file.into_std().await.into_raw_fd();
+    let vsockfd = OwnedFd::from(vsock_file.into_std().await);
     for _i in 0..IOCTL_TRY_TIMES {
         let cid = thread_rng().gen_range(3..i32::MAX as u64);
-        let res = unsafe { set_vhost_guest_cid(vsockfd, &cid) };
+        let res = unsafe { set_vhost_guest_cid(vsockfd.as_raw_fd(), &cid) };
         match res {
             Ok(_) => return Ok((vsockfd, cid)),
             Err(_) => continue,

diff --git a/vmm/sandbox/src/qemu/mod.rs b/vmm/sandbox/src/qemu/mod.rs
@@ -16,7 +16,10 @@ limitations under the License.
 
 use std::{
     collections::HashMap,
-    os::unix::io::{AsRawFd, FromRawFd, RawFd},
+    os::{
+        fd::OwnedFd,
+        unix::io::{AsRawFd, FromRawFd, RawFd},
+    },
     time::{Duration, SystemTime},
 };
 
@@ -82,7 +85,8 @@ pub struct QemuVM {
     devices: Vec<Box<dyn QemuDevice + Sync + Send>>,
     #[serde(skip)]
     hot_attached_devices: Vec<Box<dyn QemuHotAttachable + Sync + Send>>,
-    fds: Vec<RawFd>,
+    #[serde(skip)]
+    fds: Vec<OwnedFd>,
     console_socket: String,
     agent_socket: String,
     netns: String,
@@ -101,8 +105,6 @@ impl VM for QemuVM {
         debug!("start vm {}", self.id);
         let wait_chan = self.launch().await?;
         self.wait_chan = Some(wait_chan);
-        // close the fds after launch qemu
-        self.fds = vec![];
         let start_time = SystemTime::now();
         loop {
             match self.create_client().await {
@@ -342,17 +344,17 @@ impl QemuVM {
         self.devices.push(Box::new(device));
     }
 
-    fn append_fd(&mut self, fd: RawFd) -> usize {
+    fn append_fd(&mut self, fd: OwnedFd) -> usize {
         self.fds.push(fd);
         self.fds.len() - 1 + 3
     }
 
-    async fn launch(&self) -> Result<Receiver<(u32, i128)>> {
+    async fn launch(&mut self) -> Result<Receiver<(u32, i128)>> {
         let mut params = self.config.to_cmdline_params("-");
         for d in self.devices.iter() {
             params.extend(d.to_cmdline_params("-"));
         }
-        let fds = self.fds.to_vec();
+        let fds: Vec<OwnedFd> = self.fds.drain(..).collect();
         let path = self.config.path.to_string();
         // pid file should not be empty
         let pid_file = self.config.pid_file.to_string();
@@ -367,20 +369,17 @@ impl QemuVM {
         spawn_blocking(move || -> Result<()> {
             let mut cmd = unshare::Command::new(&*path_clone);
             cmd.args(params.as_slice());
-            for (i, &x) in fds.iter().enumerate() {
-                cmd.file_descriptor(
-                    (3 + i) as RawFd,
-                    Fd::from_file(unsafe { std::fs::File::from_raw_fd(x) }),
-                );
+            let pipe_writer2 = pipe_writer.try_clone()?;
+            cmd.stdout(unshare::Stdio::from_file(pipe_writer));
+            cmd.stderr(unshare::Stdio::from_file(pipe_writer2));
+            for (i, x) in fds.into_iter().enumerate() {
+                cmd.file_descriptor((3 + i) as RawFd, Fd::from_file(std::fs::File::from(x)));
             }
             if !netns.is_empty() {
                 let netns_fd = nix::fcntl::open(&*netns, OFlag::O_CLOEXEC, Mode::empty())
                     .map_err(|e| anyhow!("failed to open netns {}", e))?;
                 cmd.set_namespace(&netns_fd, unshare::Namespace::Net)?;
             }
-            let pipe_writer2 = pipe_writer.try_clone()?;
-            cmd.stdout(unshare::Stdio::from_file(pipe_writer));
-            cmd.stderr(unshare::Stdio::from_file(pipe_writer2));
             let mut child = cmd
                 .spawn()
                 .map_err(|e| anyhow!("failed to spawn qemu command: {}", e))?;

diff --git a/vmm/sandbox/src/stratovirt/devices/vsock.rs b/vmm/sandbox/src/stratovirt/devices/vsock.rs
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-use std::os::unix::io::{IntoRawFd, RawFd};
+use std::os::fd::{AsRawFd, OwnedFd};
 
 use anyhow::anyhow;
 use containerd_sandbox::error::Result;
@@ -62,18 +62,18 @@ impl VSockDevice {
     }
 }
 
-pub async fn find_context_id() -> Result<(RawFd, u64)> {
+pub async fn find_context_id() -> Result<(OwnedFd, u64)> {
     // TODO make sure if this thread_rng is enough, if we should new a seedable rng everytime.
     let vsock_file = tokio::fs::OpenOptions::new()
         .read(true)
         .write(true)
         .mode(0o666)
         .open(VHOST_VSOCK_DEV_PATH)
         .await?;
-    let vsockfd = vsock_file.into_std().await.into_raw_fd();
+    let vsockfd = OwnedFd::from(vsock_file.into_std().await);
     for _i in 0..IOCTL_TRY_TIMES {
         let cid = thread_rng().gen_range(3..i32::MAX as u64);
-        let res = unsafe { set_vhost_guest_cid(vsockfd, &cid) };
+        let res = unsafe { set_vhost_guest_cid(vsockfd.as_raw_fd(), &cid) };
         match res {
             Ok(_) => return Ok((vsockfd, cid)),
             Err(_) => continue,

diff --git a/vmm/sandbox/src/stratovirt/mod.rs b/vmm/sandbox/src/stratovirt/mod.rs
@@ -16,7 +16,10 @@ limitations under the License.
 
 use std::{
     collections::HashMap,
-    os::unix::io::{AsRawFd, FromRawFd, RawFd},
+    os::{
+        fd::OwnedFd,
+        unix::io::{AsRawFd, FromRawFd, RawFd},
+    },
     time::{Duration, SystemTime},
 };
 
@@ -87,7 +90,8 @@ pub struct StratoVirtVM {
     devices: Vec<Box<dyn StratoVirtDevice + Sync + Send>>,
     #[serde(skip)]
     hot_attached_devices: Vec<Box<dyn StratoVirtHotAttachable + Sync + Send>>,
-    fds: Vec<RawFd>,
+    #[serde(skip)]
+    fds: Vec<OwnedFd>,
     console_socket: String,
     agent_socket: String,
     netns: String,
@@ -309,17 +313,17 @@ impl StratoVirtVM {
         self.devices.push(Box::new(device));
     }
 
-    fn append_fd(&mut self, fd: RawFd) -> usize {
+    fn append_fd(&mut self, fd: OwnedFd) -> usize {
         self.fds.push(fd);
         self.fds.len() - 1 + 3
     }
 
-    async fn launch(&self) -> Result<Receiver<(u32, i128)>> {
+    async fn launch(&mut self) -> Result<Receiver<(u32, i128)>> {
         let mut params = self.config.to_cmdline_params("-");
         for d in self.devices.iter() {
             params.extend(d.to_cmdline_params("-"));
         }
-        let fds = self.fds.to_vec();
+        let fds: Vec<OwnedFd> = self.fds.drain(..).collect();
         let path = self.config.path.to_string();
         // pid file should not be empty
         let pid_file = self.config.pid_file.to_string();
@@ -335,21 +339,18 @@ impl StratoVirtVM {
             let mut cmd = unshare::Command::new(&*path_clone);
             cmd.args(params.as_slice());
 
-            for (i, &x) in fds.iter().enumerate() {
-                cmd.file_descriptor(
-                    (3 + i) as RawFd,
-                    Fd::from_file(unsafe { std::fs::File::from_raw_fd(x) }),
-                );
+            let pipe_writer2 = pipe_writer.try_clone()?;
+            cmd.stdout(unshare::Stdio::from_file(pipe_writer));
+            cmd.stderr(unshare::Stdio::from_file(pipe_writer2));
+            for (i, x) in fds.into_iter().enumerate() {
+                cmd.file_descriptor((3 + i) as RawFd, Fd::from_file(std::fs::File::from(x)));
             }
 
             if !netns.is_empty() {
                 let netns_fd = nix::fcntl::open(&*netns, OFlag::O_CLOEXEC, Mode::empty())
                     .map_err(|e| anyhow!("failed to open netns {}", e))?;
                 cmd.set_namespace(&netns_fd, unshare::Namespace::Net)?;
             }
-            let pipe_writer2 = pipe_writer.try_clone()?;
-            cmd.stdout(unshare::Stdio::from_file(pipe_writer));
-            cmd.stderr(unshare::Stdio::from_file(pipe_writer2));
             let mut child = cmd
                 .spawn()
                 .map_err(|e| anyhow!("failed to spawn stratovirt command: {}", e))?;