From 8fd8224ee3b8d1d7ec62e97c1eb04900eaf702b7 Mon Sep 17 00:00:00 2001 From: Bo Chen Date: Wed, 6 May 2026 22:03:45 +0000 Subject: [PATCH] vmm: Use HashSet for VmConfig::preserved_fds Each VM reboot re-entered the VFIO/virtio-net add paths and re-appended the same originating fds, leaving duplicates in preserved_fds and a double-close hazard at final teardown. Switching to HashSet makes add_preserved_fds idempotent. Signed-off-by: Bo Chen Assisted-by: Claude:Opus-4.7 --- vmm/src/config.rs | 14 ++++++-------- vmm/src/device_manager.rs | 2 +- vmm/src/vm_config.rs | 3 ++- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 71d903d01..3f507b830 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // -use std::collections::{BTreeSet, HashMap}; +use std::collections::{BTreeSet, HashMap, HashSet}; #[cfg(feature = "ivshmem")] use std::fs; use std::path::PathBuf; @@ -3560,16 +3560,14 @@ impl VmConfig { /// # Safety /// To use this safely, the caller must guarantee that the input /// fds are all valid. - pub unsafe fn add_preserved_fds(&mut self, mut fds: Vec) { + pub unsafe fn add_preserved_fds(&mut self, fds: Vec) { if fds.is_empty() { return; } - if let Some(preserved_fds) = &self.preserved_fds { - fds.append(&mut preserved_fds.clone()); - } - - self.preserved_fds = Some(fds); + self.preserved_fds + .get_or_insert_with(HashSet::new) + .extend(fds); } #[cfg(feature = "tdx")] @@ -3627,7 +3625,7 @@ impl Clone for VmConfig { impl Drop for VmConfig { fn drop(&mut self) { if let Some(mut fds) = self.preserved_fds.take() { - for fd in fds.drain(..) { + for fd in fds.drain() { // SAFETY: FFI call with valid FDs unsafe { libc::close(fd) }; } diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index d88bdeb31..6843b9151 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -4797,7 +4797,7 @@ impl DeviceManager { debug!("Closing preserved FDs from virtio-net device: id={id}, fds={fds:?}"); for fd in fds { - config.preserved_fds.as_mut().unwrap().retain(|x| *x != fd); + config.preserved_fds.as_mut().unwrap().remove(&fd); // SAFETY: We are closing the only remaining instance of this FD. unsafe { libc::close(fd); diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index 1d1e0d9a9..b85ae0e13 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -2,6 +2,7 @@ // // SPDX-License-Identifier: Apache-2.0 // +use std::collections::HashSet; use std::net::IpAddr; use std::path::{Path, PathBuf}; #[cfg(feature = "fw_cfg")] @@ -1065,7 +1066,7 @@ pub struct VmConfig { // causes the FDs to be closed early. This allows management software to // gracefully clean up resources (e.g., libvirt closes tap devices). #[serde(skip)] - pub preserved_fds: Option>, + pub preserved_fds: Option>, #[serde(default)] pub landlock_enable: bool, pub landlock_rules: Option>,