libfdt: Make get_property_by_offset() less unsafe
Instead of trusting that any non-NULL pointer returned from C is valid
by casting it to a reference, check that it is a valid pointer (properly
aligned for the type), that it points to a property that is fully
contained within the DT slice (this covers NULL validation), and that
the len field of the struct properly matches what libfdt returned (which
we validated).
Test: m pvmfw
Test: atest liblibfdt.integration_test
Change-Id: I72226debc5403f17250fcb0b2d85bbfbba155c98
diff --git a/libs/libfdt/Android.bp b/libs/libfdt/Android.bp
index ba9e971..b5f7471 100644
--- a/libs/libfdt/Android.bp
+++ b/libs/libfdt/Android.bp
@@ -39,6 +39,7 @@
rustlibs: [
"libcstr",
"liblibfdt_bindgen",
+ "libmemoffset_nostd",
"libzerocopy_nostd",
],
whole_static_libs: [
diff --git a/libs/libfdt/src/libfdt.rs b/libs/libfdt/src/libfdt.rs
index 31b125a..7737718 100644
--- a/libs/libfdt/src/libfdt.rs
+++ b/libs/libfdt/src/libfdt.rs
@@ -19,6 +19,7 @@
//! adapted to their use-cases (e.g. alloc-based userspace or statically allocated no_std).
use core::ffi::{c_int, CStr};
+use core::mem;
use core::ptr;
use crate::{fdt_err, fdt_err_expect_zero, fdt_err_or_option, FdtError, Phandle, Result};
@@ -225,13 +226,24 @@
let fdt = self.as_fdt_slice().as_ptr().cast();
// SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let prop = unsafe { libfdt_bindgen::fdt_get_property_by_offset(fdt, offset, &mut len) };
- if prop.is_null() {
- fdt_err(len)?;
- return Err(FdtError::Internal); // shouldn't happen.
+
+ let data_len = fdt_err(len)?.try_into().unwrap();
+ // TODO(stable_feature(offset_of)): mem::offset_of!(fdt_property, data).
+ let data_offset = memoffset::offset_of!(libfdt_bindgen::fdt_property, data);
+ let len = data_offset.checked_add(data_len).ok_or(FdtError::Internal)?;
+
+ if !is_aligned(prop) || get_slice_at_ptr(self.as_fdt_slice(), prop.cast(), len).is_none() {
+ return Err(FdtError::Internal);
}
- // SAFETY: prop is only returned when it points to valid libfdt_bindgen.
- Ok(unsafe { &*prop })
+ // SAFETY: The pointer is properly aligned, struct is fully contained in the DT slice.
+ let prop = unsafe { &*prop };
+
+ if data_len != u32::from_be(prop.len).try_into().unwrap() {
+ return Err(FdtError::BadLayout);
+ }
+
+ Ok(prop)
}
/// Safe wrapper around `fdt_first_property_offset()` (C function).
@@ -436,3 +448,8 @@
// TODO(stable_feature(ptr_sub_ptr)): p.sub_ptr()
})
}
+
+// TODO(stable_feature(pointer_is_aligned)): p.is_aligned()
+fn is_aligned<T>(p: *const T) -> bool {
+ (p as usize) % mem::align_of::<T>() == 0
+}