mirrors/crosvm
1
0
Fork 0
mirror of https://chromium.googlesource.com/crosvm/crosvm synced 2024-11-24 20:48:55 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

sys_util: remove unsafe struct_util functions

Browse source 
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>
This commit is contained in:
Daniel Verkamp 2021-08-19 15:41:42 -07:00 committed by Commit Bot
parent 51200519a2
commit 28359e141f
6 changed files with 20 additions and 168 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Cargo.lock generated
View file

@ -600,6 +600,7 @@ name = "kernel_loader"
version = "0.1.0"
dependencies = [
"base",
"data_model",
"libc",
"tempfile",
"vm_memory",

1
kernel_loader/Cargo.toml
View file

@ -4,6 +4,7 @@ version = "0.1.0"
edition = "2018"
[dependencies]
data_model = { path = "../data_model" }
libc = "*"
base = { path = "../base" }
vm_memory = { path = "../vm_memory" }

26
kernel_loader/src/lib.rs
View file

@ -8,6 +8,7 @@ use std::io::{Read, Seek, SeekFrom};
use std::mem;
use base::AsRawDescriptor;
use data_model::DataInit;
use vm_memory::{GuestAddress, GuestMemory};
#[allow(dead_code)]
@ -17,6 +18,12 @@ use vm_memory::{GuestAddress, GuestMemory};
#[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 @@ impl Display for Error {
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 @@ where
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;

2
sys_util/src/lib.rs
View file

@ -44,7 +44,6 @@ mod shm;
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::shm::*;
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::{

149
sys_util/src/struct_util.rs
View file

@ -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);
}
}

9
x86_64/src/bzimage.rs
View file

@ -9,6 +9,7 @@ use std::fmt::{self, Display};
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 @@ impl Display for Error {
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 {