From 7cbd47a71a97c4e76afc326e6f0e50ba74191e86 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Thu, 15 Oct 2020 13:58:13 +0200 Subject: [PATCH] vmm: Prevent KVM device fd from being unusable from VfioContainer When shutting down a VM using VFIO, the following error has been detected: vfio-ioctls/src/vfio_device.rs:312 -- Could not delete VFIO group: KvmSetDeviceAttr(Error(9)) After some investigation, it appears the KVM device file descriptor used for removing a VFIO group was already closed. This is coming from the Rust sequence of Drop, from the DeviceManager all the way down to VfioDevice. Because the DeviceManager owns passthrough_device, which is effectively a KVM device file descriptor, when the DeviceManager is dropped, the passthrough_device follows, with the effect of closing the KVM device file descriptor. Problem is, VfioDevice has not been dropped yet and it still needs a valid KVM device file descriptor. That's why the simple way to fix this issue coming from Rust dropping all resources is to make Linux accountable for it by duplicating the file descriptor. This way, even when the passthrough_device is dropped, the KVM file descriptor is closed, but a duplicated instance is still valid and owned by the VfioContainer. Signed-off-by: Sebastien Boeuf --- vmm/src/device_manager.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 87862f086..45601b88b 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -2613,16 +2613,29 @@ impl DeviceManager { << 3; let memory = self.memory_manager.lock().unwrap().guest_memory(); + + // Safe because we know the RawFd is valid. + // + // This dup() is mandatory to be able to give full ownership of the + // file descriptor to the DeviceFd::from_raw_fd() function later in + // the code. + // + // This is particularly needed so that VfioContainer will still have + // a valid file descriptor even if DeviceManager, and therefore the + // passthrough_device are dropped. In case of Drop, the file descriptor + // would be closed, but Linux would still have the duplicated file + // descriptor opened from DeviceFd, preventing from unexpected behavior + // where the VfioContainer would try to use a closed file descriptor. + let dup_device_fd = unsafe { libc::dup(passthrough_device.as_raw_fd()) }; + // SAFETY the raw fd conversion here is safe because: // 1. This function is only called on KVM, see the feature guard above. // 2. When running on KVM, passthrough_device wraps around DeviceFd. // 3. The conversion here extracts the raw fd and then turns the raw fd into a DeviceFd // of the same (correct) type. let vfio_container = Arc::new( - VfioContainer::new(Arc::new(unsafe { - DeviceFd::from_raw_fd(passthrough_device.as_raw_fd()) - })) - .map_err(DeviceManagerError::VfioCreate)?, + VfioContainer::new(Arc::new(unsafe { DeviceFd::from_raw_fd(dup_device_fd) })) + .map_err(DeviceManagerError::VfioCreate)?, ); let vfio_device = VfioDevice::new(