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.