Panic in FdtHeader::verify() when using with QEMU and RISC-V virt machine

[rogerta]

I get a panic in FdtHeader::verify() when using with QEMU and RISC-V virt machine:

called 'Result::unwrap()' on an 'Err' value: LastCompVersion

which occurs at line 60 while checking the header's minimum compatibility version. Note that this is the standard QEMU FDT for the RISC-V virt machine.

Version info:

• dtb-walker 0.1.0
• QEMU emulator version 6.0.0 (Debian 1:6.0+dfsg-2expubuntu1.2)

I run QEMU like this:

qemu-system-riscv64 -M virt -m 256M -nographic \
  -bios ../opensbi/build/platform/generic/firmware/fw_jump.elf \
  -kernel <path/to/my/binary>

My code looks like this:

pub fn dump(fdt: &*const u8) {
  use dtb_walker::{Dtb, DtbObj, WalkOperation};
  let dtb = unsafe { Dtb::from_raw_parts(*fdt) }.unwrap();
  ...

If I change it to:

pub fn dump(fdt: &*const u8) {
  use dtb_walker::{Dtb, DtbObj, WalkOperation};
  let dtb = unsafe { Dtb::from_raw_parts_unchecked(*fdt) };
  ...

It seems to work, since I get a dump of the FDT. Is it possible that this library is compatible with FDT versions older than 16? Maybe you can reduce the number?

If this library is compatible only with 16 or later, what parts don't work with older versions?

Thanks.

[rogerta]

In the QEMU RISC-V FDT case, I see that version is 17 and last_comp_version is also 17. The Devicetree specification, sections 5.1 and 5.2, seems to indicate that when version is 17, last_comp_version should be exactly 16. Is this a bug in QEMU?

[YdrMaster]

In the QEMU RISC-V FDT case, I see that version is 17 and last_comp_version is also 17. The Devicetree specification, sections 5.1 and 5.2, seems to indicate that when version is 17, last_comp_version should be exactly 16. Is this a bug in QEMU?

Yes, the spec says:

last_comp_version This field shall contain the lowest version of the devicetree data structure with which the version used is backwards compatible. So, for the structure as defined in this document (version 17), this field shall contain 16 because version 17 is backwards compatible with version 16, but not earlier versions. As per Section 5.1, a DTSpec boot program should provide a devicetree in a format which is backwards compatible with version 16, and thus this field shall always contain 16.

But there are indeed many dtb implementations that do not conform to this specification.

---

I found I forgot to pub use HeaderError 😢

[YdrMaster]

A new verison 0.1.1 was published. Maybe matching the HeaderError would be better (or see example):

let dtb = match unsafe { Dtb::from_raw_parts(aligned.as_ptr() as _) } {
    Ok(ans) => ans,
    Err(HeaderError::LastCompVersion) => {
        // ignore!
        unsafe { Dtb::from_raw_parts_unchecked(aligned.as_ptr() as _) }
    }
    Err(e) => panic!("Verify dtb header failed: {e:?}"),
};

[rogerta]

Cool! I'll try it out shortly and let you know.

[rogerta]

I changed my code to match your example and that seems fine.

But if there are many dtb implementations that do not conform to the spec, it might be interesting for you to follow the Robustness principle: be conservative in what you do, be liberal in what you accept from others. You could change line 60 to be:

if self.last_comp_version.into_u32() < LAST_COMP_VERSION {

[YdrMaster]

I changed my code to match your example and that seems fine.

But if there are many dtb implementations that do not conform to the spec, it might be interesting for you to follow the Robustness principle: be conservative in what you do, be liberal in what you accept from others. You could change line 60 to be:

if self.last_comp_version.into_u32() < LAST_COMP_VERSION {

Emm... I'll need to chew it over. If It's necessary to write it this way, I might rather just remove this check.

[YdrMaster]

A new verison 0.1.3 was published for a new function with a filter closure which allows to ignore some HeaderError (or see example):

let dtb = unsafe {
    Dtb::from_raw_parts_filtered(aligned.as_ptr() as _, |e| {
        matches!(
            e,
            HeaderError::Misaligned(4) | HeaderError::LastCompVersion(16)
        )
    })
}
.unwrap();

---

And there is an english readme for you :)

[rogerta]

Awesome, thanks :-)