Add helper for Read/Write through bounce buffers

roypat · roypat · commit 1f26f4580666 · 2025-04-09T14:12:23.000+01:00
When guest memory is backed by direct map removed guest_memfd ("secret
free"), we cannot load the guest kernel / initrd by read-ing into guest
memory, as the kernel won't be able to access guest memory. Instead
we'll need to read into a userspace buffer, and then memcpy into guest
memory. Add a newtype that wraps Read/Write and performs exactly this
operation, and use it to load guest kernel / initrd.

Signed-off-by: Patrick Roy &lt;roypat@amazon.co.uk&gt;
diff --git a/src/vmm/src/arch/aarch64/mod.rs b/src/vmm/src/arch/aarch64/mod.rs
@@ -18,11 +18,11 @@ pub mod vm;
 
 use std::cmp::min;
 use std::fmt::Debug;
-use std::fs::File;
+use std::io::{Read, Seek};
 
 use linux_loader::loader::pe::PE as Loader;
 use linux_loader::loader::{Cmdline, KernelLoader};
-use vm_memory::GuestMemoryError;
+use vm_memory::{GuestMemoryError, ReadVolatile};
 
 use crate::arch::{BootProtocol, EntryPoint};
 use crate::cpu_config::aarch64::{CpuConfiguration, CpuConfigurationError};
@@ -204,16 +204,10 @@ fn get_fdt_addr(mem: &GuestMemoryMmap) -> u64 {
 }
 
 /// Load linux kernel into guest memory.
-pub fn load_kernel(
-    kernel: &File,
+pub fn load_kernel<R: ReadVolatile + Read + Seek>(
+    mut kernel_file: R,
     guest_memory: &GuestMemoryMmap,
 ) -> Result<EntryPoint, ConfigurationError> {
-    // Need to clone the File because reading from it
-    // mutates it.
-    let mut kernel_file = kernel
-        .try_clone()
-        .map_err(|_| ConfigurationError::KernelFile)?;
-
     let entry_addr = Loader::load(
         guest_memory,
         Some(GuestAddress(get_kernel_start())),
diff --git a/src/vmm/src/arch/x86_64/mod.rs b/src/vmm/src/arch/x86_64/mod.rs
@@ -31,7 +31,7 @@ pub mod xstate;
 #[allow(missing_docs)]
 pub mod generated;
 
-use std::fs::File;
+use std::io::{Read, Seek};
 
 use layout::CMDLINE_START;
 use linux_loader::configurator::linux::LinuxBootConfigurator;
@@ -44,6 +44,7 @@ use linux_loader::loader::elf::start_info::{
 };
 use linux_loader::loader::{Cmdline, KernelLoader, PvhBootCapability, load_cmdline};
 use log::debug;
+use vm_memory::ReadVolatile;
 
 use super::EntryPoint;
 use crate::acpi::create_acpi_tables;
@@ -447,20 +448,14 @@ fn add_e820_entry(
 }
 
 /// Load linux kernel into guest memory.
-pub fn load_kernel(
-    kernel: &File,
+pub fn load_kernel<R: Read + ReadVolatile + Seek>(
+    mut kernel: R,
     guest_memory: &GuestMemoryMmap,
 ) -> Result<EntryPoint, ConfigurationError> {
-    // Need to clone the File because reading from it
-    // mutates it.
-    let mut kernel_file = kernel
-        .try_clone()
-        .map_err(|_| ConfigurationError::KernelFile)?;
-
     let entry_addr = Loader::load(
         guest_memory,
         None,
-        &mut kernel_file,
+        &mut kernel,
         Some(GuestAddress(get_kernel_start())),
     )
     .map_err(ConfigurationError::KernelLoader)?;
diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs
@@ -5,6 +5,7 @@
 
 use std::fmt::Debug;
 use std::io::{self};
+use std::os::unix::fs::MetadataExt;
 #[cfg(feature = "gdb")]
 use std::sync::mpsc;
 use std::sync::{Arc, Mutex};
@@ -57,10 +58,12 @@ use crate::persist::{
 use crate::resources::VmResources;
 use crate::seccomp::BpfThreadMap;
 use crate::snapshot::Persist;
+use crate::utils::u64_to_usize;
 use crate::vmm_config::instance_info::InstanceInfo;
 use crate::vmm_config::machine_config::MachineConfigError;
 use crate::vmm_config::snapshot::{MemBackendConfig, MemBackendType};
 use crate::vstate::kvm::Kvm;
+use crate::vstate::memory::Bounce;
 use crate::vstate::vcpu::{Vcpu, VcpuError};
 use crate::vstate::vm::Vm;
 use crate::{EventManager, Vmm, VmmError, device_manager};
@@ -248,8 +251,31 @@ pub fn build_microvm_for_boot(
             .map_err(VmmError::Vm)?;
     }
 
-    let entry_point = load_kernel(&boot_config.kernel_file, vmm.vm.guest_memory())?;
-    let initrd = InitrdConfig::from_config(boot_config, vmm.vm.guest_memory())?;
+    let entry_point = load_kernel(
+        Bounce(
+            &boot_config.kernel_file,
+            vm_resources.machine_config.mem_config.secret_free,
+        ),
+        vmm.vm.guest_memory(),
+    )?;
+    let initrd = match &boot_config.initrd_file {
+        Some(initrd_file) => {
+            let size = initrd_file
+                .metadata()
+                .map_err(InitrdError::Metadata)?
+                .size();
+
+            Some(InitrdConfig::from_reader(
+                vmm.vm.guest_memory(),
+                Bounce(
+                    initrd_file,
+                    vm_resources.machine_config.mem_config.secret_free,
+                ),
+                u64_to_usize(size),
+            )?)
+        }
+        None => None,
+    };
 
     #[cfg(feature = "gdb")]
     let (gdb_tx, gdb_rx) = mpsc::channel();
diff --git a/src/vmm/src/initrd.rs b/src/vmm/src/initrd.rs
@@ -1,14 +1,9 @@
 // Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use std::fs::File;
-use std::os::unix::fs::MetadataExt;
-
 use vm_memory::{GuestAddress, GuestMemory, ReadVolatile, VolatileMemoryError};
 
 use crate::arch::initrd_load_addr;
-use crate::utils::u64_to_usize;
-use crate::vmm_config::boot_source::BootConfig;
 use crate::vstate::memory::GuestMemoryMmap;
 
 /// Errors associated with initrd loading.
@@ -20,8 +15,6 @@ pub enum InitrdError {
     Load,
     /// Cannot image metadata: {0}
     Metadata(std::io::Error),
-    /// Cannot copy initrd file fd: {0}
-    CloneFd(std::io::Error),
     /// Cannot load initrd due to an invalid image: {0}
     Read(VolatileMemoryError),
 }
@@ -36,31 +29,20 @@ pub struct InitrdConfig {
 }
 
 impl InitrdConfig {
-    /// Load initrd into guest memory based on the boot config.
-    pub fn from_config(
-        boot_cfg: &BootConfig,
-        vm_memory: &GuestMemoryMmap,
-    ) -> Result<Option<Self>, InitrdError> {
-        Ok(match &boot_cfg.initrd_file {
-            Some(f) => {
-                let f = f.try_clone().map_err(InitrdError::CloneFd)?;
-                Some(Self::from_file(vm_memory, f)?)
-            }
-            None => None,
-        })
-    }
-
     /// Loads the initrd from a file into guest memory.
-    pub fn from_file(vm_memory: &GuestMemoryMmap, mut file: File) -> Result<Self, InitrdError> {
-        let size = file.metadata().map_err(InitrdError::Metadata)?.size();
-        let size = u64_to_usize(size);
+    pub fn from_reader<R: ReadVolatile>(
+        vm_memory: &GuestMemoryMmap,
+        mut reader: R,
+        size: usize,
+    ) -> Result<Self, InitrdError> {
         let Some(address) = initrd_load_addr(vm_memory, size) else {
             return Err(InitrdError::Address);
         };
         let mut slice = vm_memory
             .get_slice(GuestAddress(address), size)
             .map_err(|_| InitrdError::Load)?;
-        file.read_exact_volatile(&mut slice)
+        reader
+            .read_exact_volatile(&mut slice)
             .map_err(InitrdError::Read)?;
 
         Ok(InitrdConfig {
@@ -105,7 +87,7 @@ mod tests {
 
         // Need to reset the cursor to read initrd properly.
         tempfile.seek(SeekFrom::Start(0)).unwrap();
-        let initrd = InitrdConfig::from_file(&gm, tempfile).unwrap();
+        let initrd = InitrdConfig::from_reader(&gm, tempfile, image.len()).unwrap();
         assert!(gm.address_in_range(initrd.address));
         assert_eq!(initrd.size, image.len());
     }
@@ -120,7 +102,7 @@ mod tests {
 
         // Need to reset the cursor to read initrd properly.
         tempfile.seek(SeekFrom::Start(0)).unwrap();
-        let res = InitrdConfig::from_file(&gm, tempfile);
+        let res = InitrdConfig::from_reader(&gm, tempfile, image.len());
         assert!(matches!(res, Err(InitrdError::Address)), "{:?}", res);
     }
 
@@ -134,7 +116,7 @@ mod tests {
 
         // Need to reset the cursor to read initrd properly.
         tempfile.seek(SeekFrom::Start(0)).unwrap();
-        let res = InitrdConfig::from_file(&gm, tempfile);
+        let res = InitrdConfig::from_reader(&gm, tempfile, image.len());
         assert!(matches!(res, Err(InitrdError::Address)), "{:?}", res);
     }
 }
diff --git a/src/vmm/src/vstate/memory.rs b/src/vmm/src/vstate/memory.rs
@@ -6,8 +6,9 @@
 // found in the THIRD-PARTY file.
 
 use std::fs::File;
-use std::io::SeekFrom;
+use std::io::{Read, Seek, SeekFrom, Write};
 use std::mem::ManuallyDrop;
+use std::os::fd::AsFd;
 use std::sync::Arc;
 
 use kvm_bindings::{KVM_MEM_LOG_DIRTY_PAGES, kvm_userspace_memory_region};
@@ -20,7 +21,10 @@ pub use vm_memory::{
     Address, ByteValued, Bytes, FileOffset, GuestAddress, GuestMemory, GuestMemoryRegion,
     GuestUsize, MemoryRegionAddress, MmapRegion, address,
 };
-use vm_memory::{Error as VmMemoryError, GuestMemoryError, VolatileSlice, WriteVolatile};
+use vm_memory::{
+    Error as VmMemoryError, GuestMemoryError, ReadVolatile, VolatileMemoryError, VolatileSlice,
+    WriteVolatile,
+};
 use vmm_sys_util::errno;
 
 use crate::DirtyBitmap;
@@ -53,6 +57,61 @@ pub enum MemoryError {
     OffsetTooLarge,
 }
 
+/// Newtype that implements [`ReadVolatile`] and [`WriteVolatile`] if `T` implements `Read` or
+/// `Write` respectively, by reading/writing using a bounce buffer, and memcpy-ing into the
+/// [`VolatileSlice`].
+#[derive(Debug)]
+pub struct Bounce<T>(pub T, pub bool);
+
+// FIXME: replace AsFd with ReadVolatile once &File: ReadVolatile in vm-memory.
+impl<T: Read + AsFd> ReadVolatile for Bounce<T> {
+    fn read_volatile<B: BitmapSlice>(
+        &mut self,
+        buf: &mut VolatileSlice<B>,
+    ) -> Result<usize, VolatileMemoryError> {
+        if self.1 {
+            let mut bbuf = vec![0; buf.len()];
+            let n = self
+                .0
+                .read(bbuf.as_mut_slice())
+                .map_err(VolatileMemoryError::IOError)?;
+            buf.copy_from(&bbuf[..n]);
+            Ok(n)
+        } else {
+            self.0.as_fd().read_volatile(buf)
+        }
+    }
+}
+
+impl<T: Write + AsFd> WriteVolatile for Bounce<T> {
+    fn write_volatile<B: BitmapSlice>(
+        &mut self,
+        buf: &VolatileSlice<B>,
+    ) -> Result<usize, VolatileMemoryError> {
+        if self.1 {
+            let mut bbuf = vec![0; buf.len()];
+            buf.copy_to(bbuf.as_mut_slice());
+            self.0
+                .write(bbuf.as_slice())
+                .map_err(VolatileMemoryError::IOError)
+        } else {
+            self.0.as_fd().write_volatile(buf)
+        }
+    }
+}
+
+impl<R: Read> Read for Bounce<R> {
+    fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
+        self.0.read(buf)
+    }
+}
+
+impl<S: Seek> Seek for Bounce<S> {
+    fn seek(&mut self, pos: SeekFrom) -> std::io::Result<u64> {
+        self.0.seek(pos)
+    }
+}
+
 /// A memory region, described in terms of `kvm_userspace_memory_region`
 #[derive(Debug)]
 pub struct KvmRegion {
@@ -475,6 +534,7 @@ mod tests {
     use std::collections::HashMap;
     use std::io::{Read, Seek};
 
+    use itertools::Itertools;
     use vmm_sys_util::tempfile::TempFile;
 
     use super::*;
@@ -842,4 +902,35 @@ mod tests {
         seals.insert(memfd::FileSeal::SealGrow);
         memfd.add_seals(&seals).unwrap_err();
     }
+
+    #[test]
+    fn test_bounce() {
+        let file_direct = TempFile::new().unwrap();
+        let file_bounced = TempFile::new().unwrap();
+
+        let mut data = (0..=255).collect_vec();
+
+        Bounce(file_direct.as_file(), false)
+            .write_all_volatile(&VolatileSlice::from(data.as_mut_slice()))
+            .unwrap();
+        Bounce(file_bounced.as_file(), true)
+            .write_all_volatile(&VolatileSlice::from(data.as_mut_slice()))
+            .unwrap();
+
+        let mut data_direct = vec![0u8; 256];
+        let mut data_bounced = vec![0u8; 256];
+
+        file_direct.as_file().seek(SeekFrom::Start(0)).unwrap();
+        file_bounced.as_file().seek(SeekFrom::Start(0)).unwrap();
+
+        Bounce(file_direct.as_file(), false)
+            .read_exact_volatile(&mut VolatileSlice::from(data_direct.as_mut_slice()))
+            .unwrap();
+        Bounce(file_bounced.as_file(), true)
+            .read_exact_volatile(&mut VolatileSlice::from(data_bounced.as_mut_slice()))
+            .unwrap();
+
+        assert_eq!(data_direct, data_bounced);
+        assert_eq!(data_direct, data);
+    }
 }
