From 9b3cab8c722dd90d6e241f964a3903bb9e403562 Mon Sep 17 00:00:00 2001
From: Wei Liu <liuwe@microsoft.com>
Date: Wed, 17 Nov 2021 17:30:42 +0000
Subject: [PATCH] device_manager: check return value of dup(2)

That function call can return -1 when it fails. Wrapping -1 into File
causes the code to panic when the File is dropped.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
---
 vmm/src/device_manager.rs | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs
index 3a5579790..f8e8d3dd3 100644
--- a/vmm/src/device_manager.rs
+++ b/vmm/src/device_manager.rs
@@ -457,6 +457,9 @@ pub enum DeviceManagerError {
 
     /// Failed to update memory mappings for VFIO user device
     UpdateMemoryForVfioUserPciDevice(VfioUserPciDeviceError),
+
+    /// Cannot duplicate file descriptor
+    DupFd(vmm_sys_util::errno::Error),
 }
 pub type DeviceManagerResult<T> = result::Result<T, DeviceManagerError>;
 
@@ -1750,17 +1753,32 @@ impl DeviceManager {
                 }
             }
             ConsoleOutputMode::Tty => {
+                // Duplicating the file descriptors like this is needed as otherwise
+                // they will be closed on a reboot and the numbers reused
+
+                // SAFETY: FFI call to dup. Trivially safe.
+                let stdout = unsafe { libc::dup(libc::STDOUT_FILENO) };
+                if stdout == -1 {
+                    return vmm_sys_util::errno::errno_result().map_err(DeviceManagerError::DupFd);
+                }
+                // SAFETY: stdout is valid and owned solely by us.
+                let stdout = unsafe { File::from_raw_fd(stdout) };
+
                 // If an interactive TTY then we can accept input
                 // SAFETY: FFI call. Trivially safe.
                 if unsafe { libc::isatty(libc::STDIN_FILENO) == 1 } {
-                    Endpoint::FilePair(
-                        // Duplicating the file descriptors like this is needed as otherwise
-                        // they will be closed on a reboot and the numbers reused
-                        unsafe { File::from_raw_fd(libc::dup(libc::STDOUT_FILENO)) },
-                        unsafe { File::from_raw_fd(libc::dup(libc::STDIN_FILENO)) },
-                    )
+                    // SAFETY: FFI call to dup. Trivially safe.
+                    let stdin = unsafe { libc::dup(libc::STDIN_FILENO) };
+                    if stdin == -1 {
+                        return vmm_sys_util::errno::errno_result()
+                            .map_err(DeviceManagerError::DupFd);
+                    }
+                    // SAFETY: stdin is valid and owned solely by us.
+                    let stdin = unsafe { File::from_raw_fd(stdin) };
+
+                    Endpoint::FilePair(stdout, stdin)
                 } else {
-                    Endpoint::File(unsafe { File::from_raw_fd(libc::dup(libc::STDOUT_FILENO)) })
+                    Endpoint::File(stdout)
                 }
             }
             ConsoleOutputMode::Null => Endpoint::Null,
@@ -2885,6 +2903,9 @@ impl DeviceManager {
         // 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()) };
+        if dup_device_fd == -1 {
+            return vmm_sys_util::errno::errno_result().map_err(DeviceManagerError::DupFd);
+        }
 
         // SAFETY the raw fd conversion here is safe because:
         //   1. When running on KVM or MSHV, passthrough_device wraps around DeviceFd.