From 46d2e22e3a88e51419fb2d5771e84733b50635b5 Mon Sep 17 00:00:00 2001 From: Alyssa Ross Date: Sat, 21 Sep 2024 11:43:26 +0200 Subject: [PATCH] virtio-devices: fix vhost-user connection double close Closing a file descriptor while the std object representing it still exists is a violation of the invariants of those APIs. After shutdown() returns, the device will be dropped, and because that object still exists, it will try to close the file descriptor again. This is unsafe, because the file descriptor number might have been reused, so an unrelated file descriptor could be unexpectedly closed. As a result, the following error was being produced if debug assertions were enabled when shutting down a VM with a vhost-user device: fatal runtime error: IO Safety violation: owned file descriptor already closed In all cases, the device is dropped shortly after shutdown() is called, so it shouldn't make any difference to close the descriptor in shutdown() instead of just letting the file object be dropped when the device is. Even when migrating, shutdown() isn't called until after the snapshot is taken, so it should be fine to wait for drop, though I'm haven't tested this as I don't know any vhost-user devices that support VHOST_F_LOG_ALL. Signed-off-by: Alyssa Ross --- virtio-devices/src/vhost_user/mod.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/virtio-devices/src/vhost_user/mod.rs b/virtio-devices/src/vhost_user/mod.rs index 85d30fee3..d790c4f52 100644 --- a/virtio-devices/src/vhost_user/mod.rs +++ b/virtio-devices/src/vhost_user/mod.rs @@ -374,11 +374,6 @@ impl VhostUserCommon { } pub fn shutdown(&mut self) { - if let Some(vu) = &self.vu { - // SAFETY: trivially safe - let _ = unsafe { libc::close(vu.lock().unwrap().socket_handle().as_raw_fd()) }; - } - // Remove socket path if needed if self.server { let _ = std::fs::remove_file(&self.socket_path);