sys_util: remove unsafe struct_util functions
Replace the uses of read_struct() and read_struct_slice() with the
safe DataInit::from_reader() implementation.
BUG=b:197263364
TEST=./test_all
TEST=Boot bzImage kernel
TEST=Boot raw ELF kernel extracted with extract_vmlinux
Change-Id: I80f98243bfb58a7ae93e1686bc4d92b0cd485cda
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3108249
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
diff --git a/Cargo.lock b/Cargo.lock
index 4df0e96..31be988 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -600,6 +600,7 @@
version = "0.1.0"
dependencies = [
"base",
+ "data_model",
"libc",
"tempfile",
"vm_memory",
diff --git a/kernel_loader/Cargo.toml b/kernel_loader/Cargo.toml
index 3731fa0..9b1dc13 100644
--- a/kernel_loader/Cargo.toml
+++ b/kernel_loader/Cargo.toml
@@ -4,6 +4,7 @@
edition = "2018"
[dependencies]
+data_model = { path = "../data_model" }
libc = "*"
base = { path = "../base" }
vm_memory = { path = "../vm_memory" }
diff --git a/kernel_loader/src/lib.rs b/kernel_loader/src/lib.rs
index 7f45b34..769b91e 100644
--- a/kernel_loader/src/lib.rs
+++ b/kernel_loader/src/lib.rs
@@ -8,6 +8,7 @@
use std::mem;
use base::AsRawDescriptor;
+use data_model::DataInit;
use vm_memory::{GuestAddress, GuestMemory};
#[allow(dead_code)]
@@ -17,6 +18,12 @@
#[allow(clippy::all)]
mod elf;
+// Elf64_Ehdr is plain old data with no implicit padding.
+unsafe impl data_model::DataInit for elf::Elf64_Ehdr {}
+
+// Elf64_Phdr is plain old data with no implicit padding.
+unsafe impl data_model::DataInit for elf::Elf64_Phdr {}
+
#[derive(Debug, PartialEq)]
pub enum Error {
BigEndianElfOnLittle,
@@ -73,19 +80,15 @@
pub fn load_kernel<F>(
guest_mem: &GuestMemory,
kernel_start: GuestAddress,
- kernel_image: &mut F,
+ mut kernel_image: &mut F,
) -> Result<u64>
where
F: Read + Seek + AsRawDescriptor,
{
- let mut ehdr: elf::Elf64_Ehdr = Default::default();
kernel_image
.seek(SeekFrom::Start(0))
.map_err(|_| Error::SeekElfStart)?;
- unsafe {
- // read_struct is safe when reading a POD struct. It can be used and dropped without issue.
- base::read_struct(kernel_image, &mut ehdr).map_err(|_| Error::ReadElfHeader)?;
- }
+ let ehdr = elf::Elf64_Ehdr::from_reader(&mut kernel_image).map_err(|_| Error::ReadElfHeader)?;
// Sanity checks
if ehdr.e_ident[elf::EI_MAG0 as usize] != elf::ELFMAG0 as u8
@@ -109,11 +112,12 @@
kernel_image
.seek(SeekFrom::Start(ehdr.e_phoff))
.map_err(|_| Error::SeekProgramHeader)?;
- let phdrs: Vec<elf::Elf64_Phdr> = unsafe {
- // Reading the structs is safe for a slice of POD structs.
- base::read_struct_slice(kernel_image, ehdr.e_phnum as usize)
- .map_err(|_| Error::ReadProgramHeader)?
- };
+ let phdrs = (0..ehdr.e_phnum)
+ .enumerate()
+ .map(|_| {
+ elf::Elf64_Phdr::from_reader(&mut kernel_image).map_err(|_| Error::ReadProgramHeader)
+ })
+ .collect::<Result<Vec<elf::Elf64_Phdr>>>()?;
let mut kernel_end = 0;
diff --git a/sys_util/src/lib.rs b/sys_util/src/lib.rs
index 17aa7d5..d83cca2 100644
--- a/sys_util/src/lib.rs
+++ b/sys_util/src/lib.rs
@@ -44,7 +44,6 @@
pub mod signal;
mod signalfd;
mod sock_ctrl_msg;
-mod struct_util;
mod terminal;
mod timerfd;
pub mod vsock;
@@ -70,7 +69,6 @@
pub use crate::signal::*;
pub use crate::signalfd::*;
pub use crate::sock_ctrl_msg::*;
-pub use crate::struct_util::*;
pub use crate::terminal::*;
pub use crate::timerfd::*;
pub use descriptor_reflection::{
diff --git a/sys_util/src/struct_util.rs b/sys_util/src/struct_util.rs
deleted file mode 100644
index 964b311..0000000
--- a/sys_util/src/struct_util.rs
+++ /dev/null
@@ -1,149 +0,0 @@
-// Copyright 2017 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-use std::io::Read;
-use std::mem::size_of;
-
-#[derive(Debug)]
-pub enum Error {
- ReadStruct,
-}
-pub type Result<T> = std::result::Result<T, Error>;
-
-/// Reads a struct from an input buffer.
-///
-/// # Arguments
-///
-/// * `f` - The input to read from. Often this is a file.
-/// * `out` - The struct to fill with data read from `f`.
-///
-/// # Safety
-///
-/// This is unsafe because the struct is initialized to unverified data read from the input.
-/// `read_struct` should only be called to fill plain old data structs. It is not endian safe.
-pub unsafe fn read_struct<T: Copy, F: Read>(f: &mut F, out: &mut T) -> Result<()> {
- let out_slice = std::slice::from_raw_parts_mut(out as *mut T as *mut u8, size_of::<T>());
- f.read_exact(out_slice).map_err(|_| Error::ReadStruct)?;
- Ok(())
-}
-
-/// Reads an array of structs from an input buffer. Returns a Vec of structs initialized with data
-/// from the specified input.
-///
-/// # Arguments
-///
-/// * `f` - The input to read from. Often this is a file.
-/// * `len` - The number of structs to fill with data read from `f`.
-///
-/// # Safety
-///
-/// This is unsafe because the structs are initialized to unverified data read from the input.
-/// `read_struct_slice` should only be called for plain old data structs. It is not endian safe.
-pub unsafe fn read_struct_slice<T: Copy, F: Read>(f: &mut F, len: usize) -> Result<Vec<T>> {
- let mut out: Vec<T> = Vec::with_capacity(len);
- out.set_len(len);
- let out_slice =
- std::slice::from_raw_parts_mut(out.as_ptr() as *mut T as *mut u8, size_of::<T>() * len);
- f.read_exact(out_slice).map_err(|_| Error::ReadStruct)?;
- Ok(out)
-}
-
-#[cfg(test)]
-mod test {
- use super::*;
- use std::io::Cursor;
- use std::mem;
-
- #[derive(Clone, Copy, Debug, Default, PartialEq)]
- struct TestRead {
- a: u64,
- b: u8,
- c: u8,
- d: u8,
- e: u8,
- }
-
- #[test]
- fn struct_basic_read() {
- let orig = TestRead {
- a: 0x7766554433221100,
- b: 0x88,
- c: 0x99,
- d: 0xaa,
- e: 0xbb,
- };
- let source = unsafe {
- // Don't worry it's a test
- std::slice::from_raw_parts(
- &orig as *const _ as *const u8,
- std::mem::size_of::<TestRead>(),
- )
- };
- assert_eq!(mem::size_of::<TestRead>(), mem::size_of_val(&source));
- let mut tr: TestRead = Default::default();
- unsafe {
- read_struct(&mut Cursor::new(source), &mut tr).unwrap();
- }
- assert_eq!(orig, tr);
- }
-
- #[test]
- fn struct_read_past_end() {
- let orig = TestRead {
- a: 0x7766554433221100,
- b: 0x88,
- c: 0x99,
- d: 0xaa,
- e: 0xbb,
- };
- let source = unsafe {
- // Don't worry it's a test
- std::slice::from_raw_parts(
- &orig as *const _ as *const u8,
- std::mem::size_of::<TestRead>() - 1,
- )
- };
- let mut tr: TestRead = Default::default();
- unsafe {
- assert!(read_struct(&mut Cursor::new(source), &mut tr).is_err());
- }
- }
-
- #[test]
- fn struct_slice_read() {
- let orig = vec![
- TestRead {
- a: 0x7766554433221100,
- b: 0x88,
- c: 0x99,
- d: 0xaa,
- e: 0xbb,
- },
- TestRead {
- a: 0x7867564534231201,
- b: 0x02,
- c: 0x13,
- d: 0x24,
- e: 0x35,
- },
- TestRead {
- a: 0x7a69584736251403,
- b: 0x04,
- c: 0x15,
- d: 0x26,
- e: 0x37,
- },
- ];
- let source = unsafe {
- // Don't worry it's a test
- std::slice::from_raw_parts(
- orig.as_ptr() as *const u8,
- std::mem::size_of::<TestRead>() * 3,
- )
- };
-
- let tr: Vec<TestRead> = unsafe { read_struct_slice(&mut Cursor::new(source), 3).unwrap() };
- assert_eq!(orig, tr);
- }
-}
diff --git a/x86_64/src/bzimage.rs b/x86_64/src/bzimage.rs
index 7dd3041..a756a50 100644
--- a/x86_64/src/bzimage.rs
+++ b/x86_64/src/bzimage.rs
@@ -9,6 +9,7 @@
use std::io::{Read, Seek, SeekFrom};
use base::AsRawDescriptor;
+use data_model::DataInit;
use vm_memory::{GuestAddress, GuestMemory};
use crate::bootparam::boot_params;
@@ -55,19 +56,15 @@
pub fn load_bzimage<F>(
guest_mem: &GuestMemory,
kernel_start: GuestAddress,
- kernel_image: &mut F,
+ mut kernel_image: &mut F,
) -> Result<(boot_params, u64)>
where
F: Read + Seek + AsRawDescriptor,
{
- let mut params: boot_params = Default::default();
kernel_image
.seek(SeekFrom::Start(0))
.map_err(|_| Error::SeekBootParams)?;
- unsafe {
- // read_struct is safe when reading a POD struct. It can be used and dropped without issue.
- base::read_struct(kernel_image, &mut params).map_err(|_| Error::ReadBootParams)?;
- }
+ let params = boot_params::from_reader(&mut kernel_image).map_err(|_| Error::ReadBootParams)?;
// bzImage header signature "HdrS"
if params.hdr.header != 0x53726448 {
