Handle /proc/self/fd/N specially for kernel, bios and initrd.

This was already done for block device images, so reuse the same logic.

BUG=b:192256642
TEST=cargo test

Change-Id: Ifa69c0170ac39fc13eab61024d31e6ee5b2dd97c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3000986
Auto-Submit: Andrew Walbran <qwandor@google.com>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Commit-Queue: Andrew Walbran <qwandor@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>

src/linux.rs

@@ -512,18 +512,33 @@ fn simple_jail(cfg: &Config, policy: &str) -> Result<Option<Minijail>> {
 
 type DeviceResult<T = VirtioDeviceStub> = std::result::Result<T, Error>;
 
-fn create_block_device(cfg: &Config, disk: &DiskOption, disk_device_tube: Tube) -> DeviceResult {
+/// Open the file with the given path, or if it is of the form `/proc/self/fd/N` then just use the
+/// file descriptor.
+///
+/// Note that this will not work properly if the same `/proc/self/fd/N` path is used twice in
+/// different places, as the metadata (including the offset) will be shared between both file
+/// descriptors.
+fn open_file<F>(path: &Path, read_only: bool, error_constructor: F) -> Result<File>
+where
+    F: FnOnce(PathBuf, io::Error) -> Error,
+{
     // Special case '/proc/self/fd/*' paths. The FD is already open, just use it.
-    let raw_image: File = if disk.path.parent() == Some(Path::new("/proc/self/fd")) {
+    Ok(
-        // Safe because we will validate |raw_fd|.
+        if let Some(raw_descriptor) = raw_descriptor_from_path(path)? {
-        unsafe { File::from_raw_descriptor(raw_descriptor_from_path(&disk.path)?) }
+            // Safe because we will validate |raw_fd|.
-    } else {
+            unsafe { File::from_raw_descriptor(raw_descriptor) }
-        OpenOptions::new()
+        } else {
-            .read(true)
+            OpenOptions::new()
-            .write(!disk.read_only)
+                .read(true)
-            .open(&disk.path)
+                .write(!read_only)
-            .map_err(|e| Error::Disk(disk.path.to_path_buf(), e))?
+                .open(path)
-    };
+                .map_err(|e| error_constructor(path.to_path_buf(), e))?
+        },
+    )
+}
+
+fn create_block_device(cfg: &Config, disk: &DiskOption, disk_device_tube: Tube) -> DeviceResult {
+    let raw_image: File = open_file(&disk.path, disk.read_only, Error::Disk)?;
     // Lock the disk image to prevent other crosvm instances from using it.
     let lock_op = if disk.read_only {
         FlockOperation::LockShared
@@ -1310,17 +1325,7 @@ fn create_pmem_device(
     index: usize,
     pmem_device_tube: Tube,
 ) -> DeviceResult {
-    // Special case '/proc/self/fd/*' paths. The FD is already open, just use it.
+    let fd = open_file(&disk.path, disk.read_only, Error::Disk)?;
-    let fd: File = if disk.path.parent() == Some(Path::new("/proc/self/fd")) {
-        // Safe because we will validate |raw_fd|.
-        unsafe { File::from_raw_descriptor(raw_descriptor_from_path(&disk.path)?) }
-    } else {
-        OpenOptions::new()
-            .read(true)
-            .write(!disk.read_only)
-            .open(&disk.path)
-            .map_err(|e| Error::Disk(disk.path.to_path_buf(), e))?
-    };
 
     let arena_size = {
         let metadata =
@@ -1882,16 +1887,21 @@ fn add_crosvm_user_to_jail(jail: &mut Minijail, feature: &str) -> Result<Ids> {
     })
 }
 
-fn raw_descriptor_from_path(path: &Path) -> Result<RawDescriptor> {
+/// If the given path is of the form /proc/self/fd/N for some N, returns `Ok(Some(N))`. Otherwise
-    if !path.is_file() {
+/// returns `Ok(None`).
-        return Err(Error::InvalidFdPath);
+fn raw_descriptor_from_path(path: &Path) -> Result<Option<RawDescriptor>> {
+    if path.parent() == Some(Path::new("/proc/self/fd")) {
+        let raw_descriptor = path
+            .file_name()
+            .and_then(|fd_osstr| fd_osstr.to_str())
+            .and_then(|fd_str| fd_str.parse::<c_int>().ok())
+            .ok_or(Error::InvalidFdPath)?;
+        validate_raw_descriptor(raw_descriptor)
+            .map_err(Error::ValidateRawDescriptor)
+            .map(Some)
+    } else {
+        Ok(None)
     }
-    let raw_descriptor = path
-        .file_name()
-        .and_then(|fd_osstr| fd_osstr.to_str())
-        .and_then(|fd_str| fd_str.parse::<c_int>().ok())
-        .ok_or(Error::InvalidFdPath)?;
-    validate_raw_descriptor(raw_descriptor).map_err(Error::ValidateRawDescriptor)
 }
 
 trait IntoUnixStream {
@@ -1900,9 +1910,9 @@ trait IntoUnixStream {
 
 impl<'a> IntoUnixStream for &'a Path {
     fn into_unix_stream(self) -> Result<UnixStream> {
-        if self.parent() == Some(Path::new("/proc/self/fd")) {
+        if let Some(raw_descriptor) = raw_descriptor_from_path(self)? {
             // Safe because we will validate |raw_fd|.
-            unsafe { Ok(UnixStream::from_raw_fd(raw_descriptor_from_path(self)?)) }
+            unsafe { Ok(UnixStream::from_raw_fd(raw_descriptor)) }
         } else {
             UnixStream::connect(self).map_err(Error::InputEventsOpen)
         }
@@ -2365,18 +2375,18 @@ where
 
 fn setup_vm_components(cfg: &Config) -> Result<VmComponents> {
     let initrd_image = if let Some(initrd_path) = &cfg.initrd_path {
-        Some(File::open(initrd_path).map_err(|e| Error::OpenInitrd(initrd_path.clone(), e))?)
+        Some(open_file(initrd_path, true, Error::OpenInitrd)?)
     } else {
         None
     };
 
     let vm_image = match cfg.executable_path {
-        Some(Executable::Kernel(ref kernel_path)) => VmImage::Kernel(
+        Some(Executable::Kernel(ref kernel_path)) => {
-            File::open(kernel_path).map_err(|e| Error::OpenKernel(kernel_path.to_path_buf(), e))?,
+            VmImage::Kernel(open_file(kernel_path, true, Error::OpenKernel)?)
-        ),
+        }
-        Some(Executable::Bios(ref bios_path)) => VmImage::Bios(
+        Some(Executable::Bios(ref bios_path)) => {
-            File::open(bios_path).map_err(|e| Error::OpenBios(bios_path.to_path_buf(), e))?,
+            VmImage::Bios(open_file(bios_path, true, Error::OpenBios)?)
-        ),
+        }
         _ => panic!("Did not receive a bios or kernel, should be impossible."),
     };