libfdt: Make ref casts & transmutes less unsafe
Use (safe) zerocopy::transmute! to transmute [u32; _] into [u8; _].
Implement FdtNodeMut::parent() with FdtNode::parent(), removing an
unsafe call to the C FFI fdt_parent_offset().
Give the compiler more information about Fdt::unchecked_from*_slice()
by casting the reference, instead of transmuting its bytes (which are
not the bytes of the type referred to!). The code remains unsafe
(because we're dereferencing a raw pointer) but is already more robust.
Rework the safety comments accordingly.
Clearly distinguish the fdt_property-to-FdtPropertyStruct ref cast from
the (*const)->& cast where the former is safe (thanks to 'transparent')
while the latter is only safe if we blindly trust C (this assumption
will be removed by a future patch).
Test: m pvmfw
Test: atest liblibfdt.integration_test
Change-Id: I42785d2f5ae2dde2163d571869b36a480406cdd9
diff --git a/libs/libfdt/src/iterators.rs b/libs/libfdt/src/iterators.rs
index e818c68..cb7afda 100644
--- a/libs/libfdt/src/iterators.rs
+++ b/libs/libfdt/src/iterators.rs
@@ -23,6 +23,8 @@
use core::marker::PhantomData;
use core::{mem::size_of, ops::Range, slice::ChunksExact};
+use zerocopy::transmute;
+
/// Iterator over nodes sharing a same compatible string.
pub struct CompatibleIterator<'a> {
node: FdtNode<'a>,
@@ -132,12 +134,6 @@
}
}
-// Converts two cells into bytes of the same size
-fn two_cells_to_bytes(cells: [u32; 2]) -> [u8; 2 * size_of::<u32>()] {
- // SAFETY: the size of the two arrays are the same
- unsafe { core::mem::transmute::<[u32; 2], [u8; 2 * size_of::<u32>()]>(cells) }
-}
-
impl Reg<u64> {
const NUM_CELLS: usize = 2;
/// Converts addr and (optional) size to the format that is consumable by libfdt.
@@ -145,14 +141,10 @@
&self,
) -> ([u8; Self::NUM_CELLS * size_of::<u32>()], Option<[u8; Self::NUM_CELLS * size_of::<u32>()]>)
{
- let addr =
- two_cells_to_bytes([((self.addr >> 32) as u32).to_be(), (self.addr as u32).to_be()]);
- let size = if self.size.is_some() {
- let size = self.size.unwrap();
- Some(two_cells_to_bytes([((size >> 32) as u32).to_be(), (size as u32).to_be()]))
- } else {
- None
- };
+ let addr = transmute!([((self.addr >> 32) as u32).to_be(), (self.addr as u32).to_be()]);
+ let size =
+ self.size.map(|sz| transmute!([((sz >> 32) as u32).to_be(), (sz as u32).to_be()]));
+
(addr, size)
}
}
@@ -288,12 +280,8 @@
((self.size >> 32) as u32).to_be(),
(self.size as u32).to_be(),
];
- // SAFETY: the size of the two arrays are the same
- unsafe {
- core::mem::transmute::<[u32; Self::SIZE_CELLS], [u8; Self::SIZE_CELLS * size_of::<u32>()]>(
- buf,
- )
- }
+
+ transmute!(buf)
}
}
diff --git a/libs/libfdt/src/lib.rs b/libs/libfdt/src/lib.rs
index 8a4e251..3248397 100644
--- a/libs/libfdt/src/lib.rs
+++ b/libs/libfdt/src/lib.rs
@@ -27,7 +27,6 @@
use core::cmp::max;
use core::ffi::{c_int, c_void, CStr};
use core::fmt;
-use core::mem;
use core::ops::Range;
use core::ptr;
use core::result;
@@ -201,6 +200,14 @@
#[derive(Debug)]
struct FdtPropertyStruct(libfdt_bindgen::fdt_property);
+impl AsRef<FdtPropertyStruct> for libfdt_bindgen::fdt_property {
+ fn as_ref(&self) -> &FdtPropertyStruct {
+ let ptr = self as *const _ as *const _;
+ // SAFETY: Types have the same layout (transparent) so the valid reference remains valid.
+ unsafe { &*ptr }
+ }
+}
+
impl FdtPropertyStruct {
fn from_offset(fdt: &Fdt, offset: c_int) -> Result<&Self> {
let mut len = 0;
@@ -212,7 +219,8 @@
return Err(FdtError::Internal); // shouldn't happen.
}
// SAFETY: prop is only returned when it points to valid libfdt_bindgen.
- Ok(unsafe { &*prop.cast::<FdtPropertyStruct>() })
+ let prop = unsafe { &*prop };
+ Ok(prop.as_ref())
}
fn name_offset(&self) -> c_int {
@@ -855,10 +863,7 @@
}
fn parent(&'a self) -> Result<FdtNode<'a>> {
- // SAFETY: Accesses (read-only) are constrained to the DT totalsize.
- let ret = unsafe { libfdt_bindgen::fdt_parent_offset(self.fdt.as_ptr(), self.offset) };
-
- Ok(FdtNode { fdt: &*self.fdt, offset: fdt_err(ret)? })
+ self.as_node().parent()
}
/// Returns the compatible node of the given name that is next after this node.
@@ -979,22 +984,22 @@
///
/// # Safety
///
- /// The returned FDT might be invalid, only use on slices containing a valid DT.
+ /// It is undefined to call this function on a slice that does not contain a valid device tree.
pub unsafe fn unchecked_from_slice(fdt: &[u8]) -> &Self {
- // SAFETY: Fdt is a wrapper around a [u8], so the transmute is valid. The caller is
- // responsible for ensuring that it is actually a valid FDT.
- unsafe { mem::transmute::<&[u8], &Self>(fdt) }
+ let self_ptr = fdt as *const _ as *const _;
+ // SAFETY: The pointer is non-null, dereferenceable, and points to allocated memory.
+ unsafe { &*self_ptr }
}
/// Wraps a mutable slice containing a Flattened Device Tree.
///
/// # Safety
///
- /// The returned FDT might be invalid, only use on slices containing a valid DT.
+ /// It is undefined to call this function on a slice that does not contain a valid device tree.
pub unsafe fn unchecked_from_mut_slice(fdt: &mut [u8]) -> &mut Self {
- // SAFETY: Fdt is a wrapper around a [u8], so the transmute is valid. The caller is
- // responsible for ensuring that it is actually a valid FDT.
- unsafe { mem::transmute::<&mut [u8], &mut Self>(fdt) }
+ let self_mut_ptr = fdt as *mut _ as *mut _;
+ // SAFETY: The pointer is non-null, dereferenceable, and points to allocated memory.
+ unsafe { &mut *self_mut_ptr }
}
/// Updates this FDT from a slice containing another FDT.