libfdt: Make unsafe API for DT cloning safe
The Fdt::copy_from_slice() was unsound as it allowed client code to
bypass the (safe) constructor (which validates the class invariant) and
provide arbitrary bytes that would then be available as a "safe" (but
not actually) &Fdt. Instead of marking that API unsafe (which it should
have been), rework it into a safe API taking a &Self, meaning that the
invariant has already been validated for the input.
Update callers to use the new API. In particular, note that this change
forces one of them to be marked unsafe (which is good!).
Test: m pvmfw
Test: atest liblibfdt.integration_test
Change-Id: I7d2d8ecb85c26f5644ca26330c057568a75656ce
diff --git a/libs/libfdt/src/lib.rs b/libs/libfdt/src/lib.rs
index 3248397..39749f0 100644
--- a/libs/libfdt/src/lib.rs
+++ b/libs/libfdt/src/lib.rs
@@ -24,7 +24,6 @@
PropertyIterator, RangesIterator, Reg, RegIterator, SubnodeIterator,
};
-use core::cmp::max;
use core::ffi::{c_int, c_void, CStr};
use core::fmt;
use core::ops::Range;
@@ -1002,19 +1001,22 @@
unsafe { &mut *self_mut_ptr }
}
- /// Updates this FDT from a slice containing another FDT.
- pub fn copy_from_slice(&mut self, new_fdt: &[u8]) -> Result<()> {
- if self.buffer.len() < new_fdt.len() {
- Err(FdtError::NoSpace)
- } else {
- let totalsize = self.totalsize();
- self.buffer[..new_fdt.len()].clone_from_slice(new_fdt);
- // Zeroize the remaining part. We zeroize up to the size of the original DT because
- // zeroizing the entire buffer (max 2MB) is not necessary and may increase the VM boot
- // time.
- self.buffer[new_fdt.len()..max(new_fdt.len(), totalsize)].fill(0_u8);
- Ok(())
+ /// Updates this FDT from another FDT.
+ pub fn clone_from(&mut self, other: &Self) -> Result<()> {
+ let new_len = other.buffer.len();
+ if self.buffer.len() < new_len {
+ return Err(FdtError::NoSpace);
}
+
+ let zeroed_len = self.totalsize().checked_sub(new_len);
+ let (cloned, zeroed) = self.buffer.split_at_mut(new_len);
+
+ cloned.clone_from_slice(&other.buffer);
+ if let Some(len) = zeroed_len {
+ zeroed[..len].fill(0);
+ }
+
+ Ok(())
}
/// Unpacks the DT to cover the whole slice it is contained in.
diff --git a/pvmfw/src/fdt.rs b/pvmfw/src/fdt.rs
index 770fdf0..e83fe5a 100644
--- a/pvmfw/src/fdt.rs
+++ b/pvmfw/src/fdt.rs
@@ -662,7 +662,9 @@
let info = parse_device_tree(fdt, vm_dtbo.as_deref())?;
- fdt.copy_from_slice(pvmfw_fdt_template::RAW).map_err(|e| {
+ // SAFETY: We trust that the template (hardcoded in our RO data) is a valid DT.
+ let fdt_template = unsafe { Fdt::unchecked_from_slice(pvmfw_fdt_template::RAW) };
+ fdt.clone_from(fdt_template).map_err(|e| {
error!("Failed to instantiate FDT from the template DT: {e}");
RebootReason::InvalidFdt
})?;
@@ -936,7 +938,7 @@
// SAFETY: on failure, the corrupted DT is restored using the backup.
if let Err(e) = unsafe { fdt.apply_overlay(overlay) } {
warn!("Failed to apply debug policy: {e}. Recovering...");
- fdt.copy_from_slice(backup_fdt.as_slice())?;
+ fdt.clone_from(backup_fdt)?;
// A successful restoration is considered success because an invalid debug policy
// shouldn't DOS the pvmfw
Ok(false)