mirrors/crosvm
1
0
Fork 0
mirror of https://chromium.googlesource.com/crosvm/crosvm synced 2024-12-25 04:14:06 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

data_model: prevent unaligned DataInit::from_slice

Browse source 
Because the alignment of the data passed into from_slice is not checked,
it is very easy to pass in unaligned data that will get dereferenced at
a later point in the code. On ARM, this will lead to a SIGBUS.

This change adds an alignment check to prevent getting a signal.
Instead, the caller will get `None`.

BUG=chromium:900962
TEST=cargo test -p data_model

Change-Id: I7a0f835f7d0ffd8c3d44bbcd80a790027f652bc9
Reviewed-on: https://chromium-review.googlesource.com/1343989
Commit-Ready: Zach Reizner <zachr@chromium.org>
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
This commit is contained in:
Zach Reizner 2018-11-20 05:40:05 -08:00 committed by chrome-bot
parent 5bbbf61082
commit da37f7a586
1 changed files with 87 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

125
data_model/src/lib.rs
View file

@ -19,17 +19,22 @@ pub unsafe trait DataInit: Copy + Send + Sync {
/// value of `Self` will depend on the representation of the type in memory, and may change in
/// an unstable fashion.
///
/// This will return `None` if the length of data does not match the size of `Self`.
/// This will return `None` if the length of data does not match the size of `Self`, or if the
/// data is not aligned for the type of `Self`.
fn from_slice(data: &[u8]) -> Option<&Self> {
if data.len() == size_of::<Self>() {
// Safe because the DataInit trait asserts any data is valid for this type, and we
// ensured the size of the pointer's buffer is the correct size. This aliases a pointer,
// but because the pointer is from a const slice reference, there are no mutable
// aliases. Finally, the reference returned can not outlive data because they have equal
// implicit lifetime constraints.
Some(unsafe { &*(data.as_ptr() as *const Self) })
} else {
None
// Early out to avoid an unneeded `align_to` call.
if data.len() != size_of::<Self>() {
return None;
}
// Safe because the DataInit trait asserts any data is valid for this type, and we ensured
// the size of the pointer's buffer is the correct size. The `align_to` method ensures that
// we don't have any unaligned references. This aliases a pointer, but because the pointer
// is from a const slice reference, there are no mutable aliases. Finally, the reference
// returned can not outlive data because they have equal implicit lifetime constraints.
match unsafe { data.align_to::<Self>() } {
([], [mid], []) => Some(mid),
_ => None,
}
}
@ -39,17 +44,23 @@ pub unsafe trait DataInit: Copy + Send + Sync {
/// reference are immediately reflected in `data`. The value of the returned `Self` will depend
/// on the representation of the type in memory, and may change in an unstable fashion.
///
/// This will return `None` if the length of data does not match the size of `Self`.
/// This will return `None` if the length of data does not match the size of `Self`, or if the
/// data is not aligned for the type of `Self`.
fn from_mut_slice(data: &mut [u8]) -> Option<&mut Self> {
if data.len() == size_of::<Self>() {
// Safe because the DataInit trait asserts any data is valid for this type, and we
// ensured the size of the pointer's buffer is the correct size. This aliases a pointer,
// but because the pointer is from a const slice reference, there are no mutable
// aliases. Finally, the reference returned can not outlive data because they have equal
// implicit lifetime constraints.
Some(unsafe { &mut *(data.as_mut_ptr() as *mut Self) })
} else {
None
// Early out to avoid an unneeded `align_to_mut` call.
if data.len() != size_of::<Self>() {
return None;
}
// Safe because the DataInit trait asserts any data is valid for this type, and we ensured
// the size of the pointer's buffer is the correct size. The `align_to` method ensures that
// we don't have any unaligned references. This aliases a pointer, but because the pointer
// is from a mut slice reference, we borrow the passed in mutable reference. Finally, the
// reference returned can not outlive data because they have equal implicit lifetime
// constraints.
match unsafe { data.align_to_mut::<Self>() } {
([], [mid], []) => Some(mid),
_ => None,
}
}
@ -90,27 +101,65 @@ macro_rules! array_data_init {
}
}
macro_rules! data_init_type {
($T:ty) => {
unsafe impl DataInit for $T {}
array_data_init! {
$T,
0 1 2 3 4 5 6 7 8 9
10 11 12 13 14 15 16 17 18 19
20 21 22 23 24 25 26 27 28 29
30 31 32
($($T:ident),*) => {
$(
unsafe impl DataInit for $T {}
array_data_init! {
$T,
0 1 2 3 4 5 6 7 8 9
10 11 12 13 14 15 16 17 18 19
20 21 22 23 24 25 26 27 28 29
30 31 32
}
)*
#[cfg(test)]
mod data_init_tests {
use std::mem::{size_of, align_of};
use DataInit;
#[test]
fn from_slice_alignment() {
let mut v = [0u8; 32];
$(
let pre_len = {
let (pre, _, _) = unsafe { v.align_to::<$T>() };
pre.len()
};
{
let aligned_v = &mut v[pre_len..pre_len + size_of::<$T>()];
{
let from_aligned = $T::from_slice(aligned_v);
assert_eq!(from_aligned, Some(&0));
}
{
let from_aligned_mut = $T::from_mut_slice(aligned_v);
assert_eq!(from_aligned_mut, Some(&mut 0));
}
}
for i in 1..size_of::<$T>() {
let begin = pre_len + i;
let end = begin + size_of::<$T>();
let unaligned_v = &mut v[begin..end];
{
let from_unaligned = $T::from_slice(unaligned_v);
if align_of::<$T>() != 1 {
assert_eq!(from_unaligned, None);
}
}
{
let from_unaligned_mut = $T::from_mut_slice(unaligned_v);
if align_of::<$T>() != 1 {
assert_eq!(from_unaligned_mut, None);
}
}
}
)*
}
}
};
}
data_init_type!(u8);
data_init_type!(u16);
data_init_type!(u32);
data_init_type!(u64);
data_init_type!(usize);
data_init_type!(i8);
data_init_type!(i16);
data_init_type!(i32);
data_init_type!(i64);
data_init_type!(isize);
data_init_type!(u8, u16, u32, u64, usize, i8, i16, i32, i64, isize);
pub mod endian;
pub use endian::*;