cros_fdt: remove max_size from FdtWriter::finish()

Rather than checking the maximum size inside FdtWriter, just return the
full Vec<u8> that has already been generated and let the caller do the
check. Additionally, finish() previously always padded the Vec up to the
provided max_size, so the caller could not determine how much fdt space
was actually used; after this change, the padding is removed, so the
caller can observe how much space was actually used.

BUG=b:268397895
TEST=cargo test -p cros_fdt
TEST=Boot Crostini on arm
TEST=Boot ARCVM on x86_64

Change-Id: Ifd7d30581c7afffb09373ce9c34e4d7f9fe0d2b5
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4277061
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
This commit is contained in:
Daniel Verkamp 2023-02-21 10:41:51 -08:00 committed by crosvm LUCI
parent 9ff46a3976
commit 4ba59643b9
4 changed files with 18 additions and 24 deletions

View file

@ -591,12 +591,16 @@ pub fn create_fdt(
// End giant node
fdt.end_node(root_node)?;
let fdt_final = fdt.finish(fdt_max_size)?;
let fdt_final = fdt.finish()?;
if fdt_final.len() > fdt_max_size {
return Err(Error::TotalSizeTooLarge);
}
let written = guest_mem
.write_at_addr(fdt_final.as_slice(), fdt_address)
.map_err(|_| Error::FdtGuestMemoryWriteError)?;
if written < fdt_max_size {
if written < fdt_final.len() {
return Err(Error::FdtGuestMemoryWriteError);
}

View file

@ -70,7 +70,7 @@ const FDT_END: u32 = 0x00000009;
/// fdt.property_string("bootargs", "panic=-1 console=hvc0 root=/dev/vda")?;
/// fdt.end_node(chosen_node)?;
/// fdt.end_node(root_node)?;
/// let dtb = fdt.finish(0x1000)?;
/// let dtb = fdt.finish()?;
/// # Ok(())
/// # }
/// ```
@ -308,7 +308,7 @@ impl FdtWriter {
/// # Arguments
///
/// `max_size` - Maximum size of the finished DTB in bytes.
pub fn finish(mut self, max_size: usize) -> Result<Vec<u8>> {
pub fn finish(mut self) -> Result<Vec<u8>> {
if self.node_depth > 0 {
return Err(Error::UnclosedNode);
}
@ -344,14 +344,7 @@ impl FdtWriter {
// Add the strings block.
self.data.append(&mut self.strings);
if self.data.len() > max_size {
Err(Error::TotalSizeTooLarge)
} else {
// Fill remaining data up to `max_size` with zeroes.
self.pad(max_size - self.data.len());
Ok(self.data)
}
Ok(self.data)
}
}
@ -365,7 +358,7 @@ mod tests {
let root_node = fdt.begin_node("").unwrap();
fdt.end_node(root_node).unwrap();
assert_eq!(
fdt.finish(0x48).unwrap(),
fdt.finish().unwrap(),
[
0xd0, 0x0d, 0xfe, 0xed, // 0000: magic (0xd00dfeed)
0x00, 0x00, 0x00, 0x48, // 0004: totalsize (0x48)
@ -404,7 +397,7 @@ mod tests {
let root_node = fdt.begin_node("").unwrap();
fdt.end_node(root_node).unwrap();
assert_eq!(
fdt.finish(0x68).unwrap(),
fdt.finish().unwrap(),
[
0xd0, 0x0d, 0xfe, 0xed, // 0000: magic (0xd00dfeed)
0x00, 0x00, 0x00, 0x68, // 0004: totalsize (0x68)
@ -443,7 +436,7 @@ mod tests {
fdt.property_null("null").unwrap();
fdt.end_node(root_node).unwrap();
assert_eq!(
fdt.finish(0x59).unwrap(),
fdt.finish().unwrap(),
[
0xd0, 0x0d, 0xfe, 0xed, // 0000: magic (0xd00dfeed)
0x00, 0x00, 0x00, 0x59, // 0004: totalsize (0x59)
@ -478,7 +471,7 @@ mod tests {
fdt.property_u32("u32", 0x12345678).unwrap();
fdt.end_node(root_node).unwrap();
assert_eq!(
fdt.finish(0x5C).unwrap(),
fdt.finish().unwrap(),
[
0xd0, 0x0d, 0xfe, 0xed, // 0000: magic (0xd00dfeed)
0x00, 0x00, 0x00, 0x5c, // 0004: totalsize (0x5C)
@ -522,7 +515,7 @@ mod tests {
.unwrap();
fdt.end_node(root_node).unwrap();
assert_eq!(
fdt.finish(0xEE).unwrap(),
fdt.finish().unwrap(),
[
0xd0, 0x0d, 0xfe, 0xed, // 0000: magic (0xd00dfeed)
0x00, 0x00, 0x00, 0xee, // 0004: totalsize (0xEE)
@ -595,7 +588,7 @@ mod tests {
fdt.end_node(nested_node).unwrap();
fdt.end_node(root_node).unwrap();
assert_eq!(
fdt.finish(0x80).unwrap(),
fdt.finish().unwrap(),
[
0xd0, 0x0d, 0xfe, 0xed, // 0000: magic (0xd00dfeed)
0x00, 0x00, 0x00, 0x80, // 0004: totalsize (0x80)
@ -644,7 +637,7 @@ mod tests {
fdt.end_node(nested_node).unwrap();
fdt.end_node(root_node).unwrap();
assert_eq!(
fdt.finish(0x90).unwrap(),
fdt.finish().unwrap(),
[
0xd0, 0x0d, 0xfe, 0xed, // 0000: magic (0xd00dfeed)
0x00, 0x00, 0x00, 0x90, // 0004: totalsize (0x90)
@ -744,7 +737,6 @@ mod tests {
fdt.property_u32("ok_prop", 1234).unwrap();
let _nested_node = fdt.begin_node("mynode").unwrap();
fdt.property_u32("ok_nested_prop", 5678).unwrap();
fdt.finish(0x100)
.expect_err("finish without ending all nodes");
fdt.finish().expect_err("finish without ending all nodes");
}
}

View file

@ -11,7 +11,6 @@ use cros_fdt::FdtWriter;
use crate::SetupData;
use crate::SetupDataType;
use crate::X86_64_FDT_MAX_SIZE;
/// Creates a flattened device tree containing all of the parameters for the
/// kernel and returns it as `SetupData`.
@ -30,7 +29,7 @@ pub fn create_fdt(
create_android_fdt(&mut fdt, android_fstab)?;
fdt.end_node(root_node)?;
let fdt_final = fdt.finish(X86_64_FDT_MAX_SIZE as usize)?;
let fdt_final = fdt.finish()?;
if let Some(file_path) = dump_device_tree_blob {
std::fs::write(&file_path, &fdt_final)

View file

@ -10,7 +10,6 @@ mod fdt;
const SETUP_DTB: u32 = 2;
const SETUP_RNG_SEED: u32 = 9;
const X86_64_FDT_MAX_SIZE: u64 = 0x20_0000;
#[allow(dead_code)]
#[allow(non_upper_case_globals)]