From ad41ae89ec99395b3d7c203563033b92f2e8860e Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Fri, 6 Sep 2024 12:32:19 -0700 Subject: [PATCH] kernel_loader: move load_cmdline() to x86_64 This is only used by the x86_64 arch code, and it isn't really related to kernel loading. Moving it to x86_64 will also allow x86-specific changes in upcoming CLs. No functional change intended. BUG=b:362168475 Change-Id: I4d01ba81d4f14fe73b38d62981a2c5a651b1bf03 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5842390 Reviewed-by: Junichi Uekawa Commit-Queue: Daniel Verkamp --- kernel_loader/src/lib.rs | 78 ------------------------------------ x86_64/src/lib.rs | 86 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 79 insertions(+), 85 deletions(-) diff --git a/kernel_loader/src/lib.rs b/kernel_loader/src/lib.rs index 945fe34612..77c3058bcb 100644 --- a/kernel_loader/src/lib.rs +++ b/kernel_loader/src/lib.rs @@ -4,7 +4,6 @@ //! Linux kernel ELF file loader. -use std::ffi::CStr; use std::mem; use base::FileReadWriteAtVolatile; @@ -38,10 +37,6 @@ pub use multiboot::multiboot_header_from_file; pub enum Error { #[error("trying to load big-endian binary on little-endian machine")] BigEndianOnLittle, - #[error("failed writing command line to guest memory")] - CommandLineCopy, - #[error("command line overflowed guest memory")] - CommandLineOverflow, #[error("invalid elf class")] InvalidElfClass, #[error("invalid elf version")] @@ -255,37 +250,6 @@ where }) } -/// Writes the command line string to the given memory slice. -/// -/// # Arguments -/// -/// * `guest_mem` - A u8 slice that will be partially overwritten by the command line. -/// * `guest_addr` - The address in `guest_mem` at which to load the command line. -/// * `cmdline` - The kernel command line. -pub fn load_cmdline( - guest_mem: &GuestMemory, - guest_addr: GuestAddress, - cmdline: &CStr, -) -> Result<()> { - let len = cmdline.to_bytes().len(); - if len == 0 { - return Ok(()); - } - - let end = guest_addr - .checked_add(len as u64 + 1) - .ok_or(Error::CommandLineOverflow)?; // Extra for null termination. - if end > guest_mem.end_addr() { - return Err(Error::CommandLineOverflow); - } - - guest_mem - .write_at_addr(cmdline.to_bytes_with_nul(), guest_addr) - .map_err(|_| Error::CommandLineCopy)?; - - Ok(()) -} - struct Elf64 { file_header: elf::Elf64_Ehdr, program_headers: Vec, @@ -420,48 +384,6 @@ mod test { GuestMemory::new(&[(GuestAddress(0x0), MEM_SIZE)]).unwrap() } - #[test] - fn cmdline_overflow() { - let gm = create_guest_mem(); - let cmdline_address = GuestAddress(MEM_SIZE - 5); - assert_eq!( - Err(Error::CommandLineOverflow), - load_cmdline( - &gm, - cmdline_address, - CStr::from_bytes_with_nul(b"12345\0").unwrap() - ) - ); - } - - #[test] - fn cmdline_write_end() { - let gm = create_guest_mem(); - let mut cmdline_address = GuestAddress(45); - assert_eq!( - Ok(()), - load_cmdline( - &gm, - cmdline_address, - CStr::from_bytes_with_nul(b"1234\0").unwrap() - ) - ); - let val: u8 = gm.read_obj_from_addr(cmdline_address).unwrap(); - assert_eq!(val, b'1'); - cmdline_address = cmdline_address.unchecked_add(1); - let val: u8 = gm.read_obj_from_addr(cmdline_address).unwrap(); - assert_eq!(val, b'2'); - cmdline_address = cmdline_address.unchecked_add(1); - let val: u8 = gm.read_obj_from_addr(cmdline_address).unwrap(); - assert_eq!(val, b'3'); - cmdline_address = cmdline_address.unchecked_add(1); - let val: u8 = gm.read_obj_from_addr(cmdline_address).unwrap(); - assert_eq!(val, b'4'); - cmdline_address = cmdline_address.unchecked_add(1); - let val: u8 = gm.read_obj_from_addr(cmdline_address).unwrap(); - assert_eq!(val, b'\0'); - } - // Elf32 image that prints hello world on x86. fn make_elf32_bin() -> File { // test_elf32.bin built on Linux with gcc -m32 -static-pie diff --git a/x86_64/src/lib.rs b/x86_64/src/lib.rs index 2a5420fb73..3e7b3cdf74 100644 --- a/x86_64/src/lib.rs +++ b/x86_64/src/lib.rs @@ -165,6 +165,10 @@ pub enum Error { CloneTube(TubeError), #[error("the given kernel command line was invalid: {0}")] Cmdline(kernel_cmdline::Error), + #[error("failed writing command line to guest memory")] + CommandLineCopy, + #[error("command line overflowed guest memory")] + CommandLineOverflow, #[error("failed to configure hotplugged pci device: {0}")] ConfigurePciDevice(arch::DeviceRegistrationError), #[error("failed to configure segment registers: {0}")] @@ -222,8 +226,6 @@ pub enum Error { LoadBios(io::Error), #[error("error loading kernel bzImage: {0}")] LoadBzImage(bzimage::Error), - #[error("error loading command line: {0}")] - LoadCmdline(kernel_loader::Error), #[error("error loading initrd: {0}")] LoadInitrd(arch::LoadImageError), #[error("error loading Kernel: {0}")] @@ -1004,12 +1006,11 @@ impl arch::LinuxArch for X8664arch { match components.vm_image { VmImage::Bios(ref mut bios) => { // Allow a bios to hardcode CMDLINE_OFFSET and read the kernel command line from it. - kernel_loader::load_cmdline( + Self::load_cmdline( &mem, GuestAddress(CMDLINE_OFFSET), &CString::new(cmdline).unwrap(), - ) - .map_err(Error::LoadCmdline)?; + )?; Self::load_bios(&mem, bios)?; regs::set_default_msrs(&mut msrs); // The default values for `Regs` and `Sregs` already set up the reset vector. @@ -1344,6 +1345,37 @@ impl X8664arch { Ok(()) } + /// Writes the command line string to the given memory slice. + /// + /// # Arguments + /// + /// * `guest_mem` - A u8 slice that will be partially overwritten by the command line. + /// * `guest_addr` - The address in `guest_mem` at which to load the command line. + /// * `cmdline` - The kernel command line. + fn load_cmdline( + guest_mem: &GuestMemory, + guest_addr: GuestAddress, + cmdline: &CStr, + ) -> Result<()> { + let len = cmdline.to_bytes().len(); + if len == 0 { + return Ok(()); + } + + let end = guest_addr + .checked_add(len as u64 + 1) + .ok_or(Error::CommandLineOverflow)?; // Extra for null termination. + if end > guest_mem.end_addr() { + return Err(Error::CommandLineOverflow); + } + + guest_mem + .write_at_addr(cmdline.to_bytes_with_nul(), guest_addr) + .map_err(|_| Error::CommandLineCopy)?; + + Ok(()) + } + /// Loads the kernel from an open file. /// /// # Arguments @@ -1400,8 +1432,7 @@ impl X8664arch { dump_device_tree_blob: Option, device_tree_overlays: Vec, ) -> Result<()> { - kernel_loader::load_cmdline(mem, GuestAddress(CMDLINE_OFFSET), cmdline) - .map_err(Error::LoadCmdline)?; + Self::load_cmdline(mem, GuestAddress(CMDLINE_OFFSET), cmdline)?; let mut setup_data = Vec::::new(); if let Some(android_fstab) = android_fstab { @@ -2267,4 +2298,45 @@ mod tests { entry2_data ); } + + #[test] + fn cmdline_overflow() { + const MEM_SIZE: u64 = 0x1000; + let gm = GuestMemory::new(&[(GuestAddress(0x0), MEM_SIZE)]).unwrap(); + let cmdline_address = GuestAddress(MEM_SIZE - 5); + let err = X8664arch::load_cmdline( + &gm, + cmdline_address, + CStr::from_bytes_with_nul(b"12345\0").unwrap(), + ) + .unwrap_err(); + assert!(matches!(err, Error::CommandLineOverflow)); + } + + #[test] + fn cmdline_write_end() { + const MEM_SIZE: u64 = 0x1000; + let gm = GuestMemory::new(&[(GuestAddress(0x0), MEM_SIZE)]).unwrap(); + let mut cmdline_address = GuestAddress(45); + X8664arch::load_cmdline( + &gm, + cmdline_address, + CStr::from_bytes_with_nul(b"1234\0").unwrap(), + ) + .unwrap(); + let val: u8 = gm.read_obj_from_addr(cmdline_address).unwrap(); + assert_eq!(val, b'1'); + cmdline_address = cmdline_address.unchecked_add(1); + let val: u8 = gm.read_obj_from_addr(cmdline_address).unwrap(); + assert_eq!(val, b'2'); + cmdline_address = cmdline_address.unchecked_add(1); + let val: u8 = gm.read_obj_from_addr(cmdline_address).unwrap(); + assert_eq!(val, b'3'); + cmdline_address = cmdline_address.unchecked_add(1); + let val: u8 = gm.read_obj_from_addr(cmdline_address).unwrap(); + assert_eq!(val, b'4'); + cmdline_address = cmdline_address.unchecked_add(1); + let val: u8 = gm.read_obj_from_addr(cmdline_address).unwrap(); + assert_eq!(val, b'\0'); + } }