summary refs log tree commit diff
diff options
context:
space:
mode:
authorZach Reizner <zachr@google.com>2020-01-31 17:43:30 -0800
committerCommit Bot <commit-bot@chromium.org>2020-02-06 21:56:39 +0000
commitbc7728f69b804f6cee6769aa9428e83a8c647bf9 (patch)
tree70e026e0cc3a6f3542c2721c32b249248a7380cf
parent787c84b51b29c0715c6d3e73aca0148b6b112440 (diff)
downloadcrosvm-bc7728f69b804f6cee6769aa9428e83a8c647bf9.tar
crosvm-bc7728f69b804f6cee6769aa9428e83a8c647bf9.tar.gz
crosvm-bc7728f69b804f6cee6769aa9428e83a8c647bf9.tar.bz2
crosvm-bc7728f69b804f6cee6769aa9428e83a8c647bf9.tar.lz
crosvm-bc7728f69b804f6cee6769aa9428e83a8c647bf9.tar.xz
crosvm-bc7728f69b804f6cee6769aa9428e83a8c647bf9.tar.zst
crosvm-bc7728f69b804f6cee6769aa9428e83a8c647bf9.zip
vm_control: fix double-close on VmIrqRequest::AllocateOneMsi
The EventFd that wraps the MaybeOwnedFd will close the fd, but so will
MaybeOwnedFd, causing a double-close.

BUG=None
TEST=strace crosvm run

Change-Id: I277386cd20eaa1a8187274cc16084b1936355012
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2034026
Commit-Queue: Zach Reizner <zachr@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Xiong  Zhang <xiong.y.zhang@intel.corp-partner.google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
-rw-r--r--vm_control/src/lib.rs11
1 files changed, 10 insertions, 1 deletions
diff --git a/vm_control/src/lib.rs b/vm_control/src/lib.rs
index 7e5faf5..298d800 100644
--- a/vm_control/src/lib.rs
+++ b/vm_control/src/lib.rs
@@ -13,6 +13,7 @@
 use std::fmt::{self, Display};
 use std::fs::File;
 use std::io::{Seek, SeekFrom};
+use std::mem::ManuallyDrop;
 use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
 
 use libc::{EINVAL, EIO, ENODEV};
@@ -332,7 +333,15 @@ impl VmIrqRequest {
         match *self {
             AllocateOneMsi { ref irqfd } => {
                 if let Some(irq_num) = sys_allocator.allocate_irq() {
-                    let evt = unsafe { EventFd::from_raw_fd(irqfd.as_raw_fd()) };
+                    // Beacuse of the limitation of `MaybeOwnedFd` not fitting into `register_irqfd`
+                    // which expects an `&EventFd`, we use the unsafe `from_raw_fd` to assume that
+                    // the fd given is an `EventFd`, and we ignore the ownership question using
+                    // `ManuallyDrop`. This is safe because `ManuallyDrop` prevents any Drop
+                    // implementation from triggering on `irqfd` which already has an owner, and the
+                    // `EventFd` methods are never called. The underlying fd is merely passed to the
+                    // kernel which doesn't care about ownership and deals with incorrect FDs, in
+                    // the case of bugs on our part.
+                    let evt = unsafe { ManuallyDrop::new(EventFd::from_raw_fd(irqfd.as_raw_fd())) };
                     match vm.register_irqfd(&evt, irq_num) {
                         Ok(_) => VmIrqResponse::AllocateOneMsi { gsi: irq_num },
                         Err(e) => VmIrqResponse::Err(e),