From b91c52e7e208d673e1c0919aaf11ae1c82cffb80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Ma=C4=87kowski?= Date: Mon, 26 Oct 2026 11:57:41 +0100 Subject: [PATCH] feat: add Fdt::with_options (and variants) Currently, `Fdt::new` does a strict validation of node and property names. According to the DTB spec, these names cannot exceed 31 characters in length. However, in practice, many DTBs do (including ones typically used in Linux), and `dtc` doesn't reject such trees when compiling DTS files. This adds `with_options` method to `Fdt` that allows to specify the exact behaviour. The default is to allow names exceeding 31 characters; UTF-8 validity is still checked. Recently added `from_raw_bounded` was also replaced with this. Fixes #101 --- src/fdt.rs | 241 ++++++++++++++++++++++++++++++++---------- src/fdt/node.rs | 4 +- src/fdt/property.rs | 4 +- src/fdt_mut.rs | 79 +++++++++----- src/fdt_mut/node.rs | 6 +- src/model/node.rs | 2 +- src/model/property.rs | 2 +- src/validate.rs | 76 ++++++++----- tests/fdt.rs | 65 +++++++++++- tests/fdt_mut.rs | 28 +++-- 10 files changed, 377 insertions(+), 130 deletions(-) diff --git a/src/fdt.rs b/src/fdt.rs index 5ef9119..ab5b1a8 100644 --- a/src/fdt.rs +++ b/src/fdt.rs @@ -143,6 +143,53 @@ impl FdtPropertyHeader { } } +/// Configuration options for parsing and validating an [`Fdt`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[non_exhaustive] +pub struct FdtOptions { + /// Enforce the Devicetree Specification's 31-character length limit on node + /// and property names. + /// + /// Defaults to `false`. + pub strict_name_length: bool, + /// Maximum allowed size (in bytes) of the FDT blob (`totalsize`). + /// + /// Defaults to `usize::MAX`. + pub max_size: usize, +} + +impl Default for FdtOptions { + fn default() -> Self { + Self::new() + } +} + +impl FdtOptions { + /// Creates a new [`FdtOptions`] with default settings. + #[must_use] + pub const fn new() -> Self { + Self { + strict_name_length: false, + max_size: usize::MAX, + } + } + + /// Sets whether to enforce the 31-character length limit on node and + /// property names. + #[must_use] + pub const fn strict_name_length(mut self, strict: bool) -> Self { + self.strict_name_length = strict; + self + } + + /// Sets the maximum allowed size (in bytes) of the FDT blob. + #[must_use] + pub const fn max_size(mut self, max_size: usize) -> Self { + self.max_size = max_size; + self + } +} + /// A flattened device tree. #[derive(Clone, Copy)] pub struct Fdt<'a> { @@ -170,6 +217,23 @@ pub(crate) enum FdtToken { End, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum NameValidation { + Skip, + Relaxed, + StrictLength, +} + +impl From for NameValidation { + fn from(options: FdtOptions) -> Self { + if options.strict_name_length { + Self::StrictLength + } else { + Self::Relaxed + } + } +} + impl TryFrom for FdtToken { type Error = u32; @@ -211,8 +275,38 @@ impl<'a> Fdt<'a> { /// let fdt = Fdt::new(dtb).unwrap(); /// ``` pub fn new(data: &'a [u8]) -> Result { + Self::with_options(data, FdtOptions::new()) + } + + /// Creates a new `Fdt` from the given byte slice using the specified + /// [`FdtOptions`]. + /// + /// # Errors + /// + /// Returns [`FdtErrorKind::InvalidLength`] if `data` is too short to + /// contain a valid FDT header, if `data.len()` exceeds `options.max_size`, + /// or if the `totalsize` field in the header does not match the length of + /// `data`. + /// + /// Returns [`FdtErrorKind::InvalidNodeName`] or + /// [`FdtErrorKind::InvalidPropertyName`] if a node or property name is + /// invalid (including exceeding 31 characters when + /// `options.strict_name_length` is `true`). + /// + /// This function can also return any of the other errors returned by + /// [`Fdt::new`]. + /// + /// # Examples + /// + /// ``` + /// # use dtoolkit::fdt::{Fdt, FdtOptions}; + /// # let dtb = include_bytes!("../tests/dtb/test.dtb"); + /// let options = FdtOptions::new().strict_name_length(true); + /// let fdt = Fdt::with_options(dtb, options).unwrap(); + /// ``` + pub fn with_options(data: &'a [u8], options: FdtOptions) -> Result { let fdt = Self::new_unchecked(data); - fdt.validate()?; + fdt.validate(options)?; Ok(fdt) } @@ -259,36 +353,38 @@ impl<'a> Fdt<'a> { Self::new_unchecked(slice) } - /// Creates a new `Fdt` from the given pointer, verifying that the header - /// and `totalsize` fit within `max_len` bytes before reading the rest of - /// the blob. + /// Creates a new `Fdt` from the given pointer using the specified + /// [`FdtOptions`], verifying that the header and `totalsize` fit within + /// `options.max_size` bytes before reading the rest of the blob. /// /// # Safety /// - /// If `max_len >= size_of::()` (40 bytes), `data` must be - /// valid and accessible for reading at least 8 bytes (`magic` and + /// If `options.max_size >= size_of::()` (40 bytes), `data` must + /// be valid and accessible for reading at least 8 bytes (`magic` and /// `totalsize`), and if the header contains a valid FDT magic number and - /// `40 <= totalsize <= max_len`, the memory region starting at `data` and - /// spanning `totalsize` bytes must be valid and accessible for reading for - /// the lifetime `'a`. There must be no existing mutable references to the - /// same data for the lifetime `'a`. + /// `40 <= totalsize <= options.max_size`, the memory region starting at + /// `data` and spanning `totalsize` bytes must be valid and accessible for + /// reading for the lifetime `'a`. There must be no existing mutable + /// references to the same data for the lifetime `'a`. /// /// # Errors /// - /// Returns [`FdtErrorKind::InvalidLength`] if `max_len` is smaller than an - /// FDT header, or if the `totalsize` field in the header is smaller than - /// an FDT header, exceeds `max_len`, or exceeds `isize::MAX` bytes. + /// Returns [`FdtErrorKind::InvalidLength`] if `options.max_size` is smaller + /// than an FDT header, or if the `totalsize` field in the header is smaller + /// than an FDT header, exceeds `options.max_size`, or exceeds `isize::MAX` + /// bytes. /// /// This function can also return any of the errors returned by - /// [`Fdt::new`]. + /// [`Fdt::with_options`]. /// /// # Examples /// /// ``` - /// # use dtoolkit::fdt::Fdt; + /// # use dtoolkit::fdt::{Fdt, FdtOptions}; /// # let dtb = include_bytes!("../tests/dtb/test.dtb"); /// let ptr = dtb.as_ptr(); - /// let fdt = unsafe { Fdt::from_raw_bounded(ptr, dtb.len()).unwrap() }; + /// let options = FdtOptions::new().max_size(dtb.len()); + /// let fdt = unsafe { Fdt::from_raw_with_options(ptr, options).unwrap() }; /// ``` #[expect( unsafe_code, @@ -296,13 +392,16 @@ impl<'a> Fdt<'a> { embedded applications, where the binary only gets a pointer to DT from the firmware or \ a bootloader. The user must ensure it trusts the data." )] - pub unsafe fn from_raw_bounded(data: *const u8, max_len: usize) -> Result { - if max_len < size_of::() { + pub unsafe fn from_raw_with_options( + data: *const u8, + options: FdtOptions, + ) -> Result { + if options.max_size < size_of::() { return Err(FdtParseError::new(FdtErrorKind::InvalidLength, 0)); } - // SAFETY: `max_len >= size_of::()` (40 bytes), and the - // caller guarantees that at least the 8-byte `(magic, totalsize)` + // SAFETY: `options.max_size >= size_of::()` (40 bytes), and + // the caller guarantees that at least the 8-byte `(magic, totalsize)` // prefix at `data` is valid for reading. let [magic, totalsize] = unsafe { ptr::read_unaligned(data.cast::<[big_endian::U32; 2]>()) }; @@ -317,7 +416,7 @@ impl<'a> Fdt<'a> { let size = totalsize.get() as usize; // `core::slice::from_raw_parts` requires that the total slice size // does not exceed `isize::MAX` bytes. - let max_valid_len = max_len.min(isize::MAX.cast_unsigned()); + let max_valid_len = options.max_size.min(isize::MAX.cast_unsigned()); if !(size_of::()..=max_valid_len).contains(&size) { return Err(FdtParseError::new( FdtErrorKind::InvalidLength, @@ -326,11 +425,11 @@ impl<'a> Fdt<'a> { } // SAFETY: We verified that the header magic is valid and that `size` is - // at least `size_of::()` and at most `max_len` (and + // at least `size_of::()` and at most `options.max_size` (and // `isize::MAX`). The caller guarantees that `data` is valid for reading - // `totalsize` bytes when `40 <= totalsize <= max_len`. + // `totalsize` bytes when `40 <= totalsize <= options.max_size`. let slice = unsafe { core::slice::from_raw_parts(data, size) }; - Self::new(slice) + Self::with_options(slice, options) } /// Creates a new `Fdt` from the given pointer. @@ -369,10 +468,10 @@ impl<'a> Fdt<'a> { pub unsafe fn from_raw(data: *const u8) -> Result { // SAFETY: The caller guarantees that `data` points to a readable FDT // header and spans `totalsize` bytes when the header is valid. - unsafe { Self::from_raw_bounded(data, usize::MAX) } + unsafe { Self::from_raw_with_options(data, FdtOptions::new()) } } - fn validate(self) -> Result<(), FdtParseError> { + fn validate(self, options: FdtOptions) -> Result<(), FdtParseError> { if self.data.len() < size_of::() { return Err(FdtParseError::new(FdtErrorKind::InvalidLength, 0)); } @@ -392,7 +491,8 @@ impl<'a> Fdt<'a> { )); } - if header.totalsize() as usize != self.data.len() { + let totalsize = header.totalsize() as usize; + if totalsize != self.data.len() || totalsize > options.max_size { return Err(FdtParseError::new( FdtErrorKind::InvalidLength, offset_of!(FdtHeader, totalsize), @@ -412,7 +512,7 @@ impl<'a> Fdt<'a> { )); } - let end_offset = self.traverse_node(offset, true)?; + let end_offset = self.traverse_node(offset, NameValidation::from(options))?; // Check FDT_END if self.read_token(end_offset)? != FdtToken::End { @@ -450,14 +550,14 @@ impl<'a> Fdt<'a> { pub(crate) fn skip_props( self, offset: usize, - check_strings: bool, + name_validation: NameValidation, ) -> Result { let mut offset = offset; loop { let token = self.read_token(offset)?; match token { FdtToken::Prop => { - offset = self.next_property_offset(offset, check_strings)?; + offset = self.next_property_offset(offset, name_validation)?; } FdtToken::Nop => offset += FDT_TAGSIZE, _ => return Ok(offset), @@ -465,7 +565,11 @@ impl<'a> Fdt<'a> { } } - fn traverse_node(self, offset: usize, check_strings: bool) -> Result { + fn traverse_node( + self, + offset: usize, + name_validation: NameValidation, + ) -> Result { let mut offset = offset; let mut depth = 0; loop { @@ -476,7 +580,8 @@ impl<'a> Fdt<'a> { offset += FDT_TAGSIZE; let end_offset = self.find_string_end(offset)?; // Validate name - if check_strings { + if name_validation != NameValidation::Skip { + let strict_len = name_validation == NameValidation::StrictLength; let name = self.string_at_offset(offset, Some(end_offset))?; if depth == 1 { if !name.is_empty() { @@ -485,12 +590,14 @@ impl<'a> Fdt<'a> { offset, )); } - } else if name.is_empty() || !crate::validate::is_valid_node_name(name) { + } else if name.is_empty() + || !crate::validate::is_valid_node_name(name, strict_len) + { return Err(FdtParseError::new(FdtErrorKind::InvalidNodeName, offset)); } } offset = Self::align_tag_offset(end_offset); - offset = self.skip_props(offset, check_strings)?; + offset = self.skip_props(offset, name_validation)?; } FdtToken::EndNode => { if depth == 0 { @@ -839,13 +946,13 @@ impl<'a> Fdt<'a> { } pub(crate) fn next_sibling_offset(self, offset: usize) -> Result { - self.traverse_node(offset, false) + self.traverse_node(offset, NameValidation::Skip) } pub(crate) fn next_property_offset( self, offset: usize, - check_name: bool, + name_validation: NameValidation, ) -> Result { let (header, _) = FdtPropertyHeader::ref_from_prefix(self.data_at_offset(offset)?) .map_err(|_e| FdtParseError::new(FdtErrorKind::InvalidLength, offset))?; @@ -853,9 +960,10 @@ impl<'a> Fdt<'a> { let len = header.len() as usize; let nameoff = header.nameoff() as usize; - if check_name { + if name_validation != NameValidation::Skip { + let strict_len = name_validation == NameValidation::StrictLength; let name = self.string(nameoff)?; - if !crate::validate::is_valid_property_name(name) { + if !crate::validate::is_valid_property_name(name, strict_len) { return Err(FdtParseError::new( FdtErrorKind::InvalidPropertyName, offset + FDT_TAGSIZE, @@ -1002,22 +1110,29 @@ mod tests { } #[test] - #[expect(unsafe_code, reason = "Testing unsafe from_raw_bounded")] - fn from_raw_bounded_validates_before_slicing() { - // SAFETY: `max_len` is smaller than `FdtHeader`, so `from_raw_bounded` - // returns an error without dereferencing `data`. - let err = - unsafe { Fdt::from_raw_bounded(ptr::null(), size_of::() - 1) }.unwrap_err(); + #[expect(unsafe_code, reason = "Testing unsafe from_raw_with_options")] + fn from_raw_with_options_validates_before_slicing() { + // SAFETY: `max_size` is smaller than `FdtHeader`, so + // `from_raw_with_options` returns an error without dereferencing + // `data`. + let err = unsafe { + Fdt::from_raw_with_options( + ptr::null(), + FdtOptions::new().max_size(size_of::() - 1), + ) + } + .unwrap_err(); assert_eq!(err, FdtParseError::new(FdtErrorKind::InvalidLength, 0)); // Corrupt magic with huge totalsize in an 8-byte buffer is rejected // before reading 40 bytes or slicing totalsize. let mut bad_magic = [0u8; 8]; - bad_magic[0..4].copy_from_slice(&0xdead_beefu32.to_be_bytes()); - bad_magic[4..8].copy_from_slice(&u32::MAX.to_be_bytes()); + bad_magic[0..FDT_TAGSIZE].copy_from_slice(&0xdead_beefu32.to_be_bytes()); + bad_magic[FDT_TAGSIZE..2 * FDT_TAGSIZE].copy_from_slice(&u32::MAX.to_be_bytes()); // SAFETY: `bad_magic` has 8 readable bytes, and the invalid magic is // rejected before reading past byte 8. - let err = unsafe { Fdt::from_raw_bounded(bad_magic.as_ptr(), usize::MAX) }.unwrap_err(); + let err = unsafe { Fdt::from_raw_with_options(bad_magic.as_ptr(), FdtOptions::default()) } + .unwrap_err(); assert_eq!( err, FdtParseError::new(FdtErrorKind::InvalidMagic, offset_of!(FdtHeader, magic)) @@ -1030,7 +1145,7 @@ mod tests { FdtParseError::new(FdtErrorKind::InvalidMagic, offset_of!(FdtHeader, magic)) ); - // totalsize smaller than header or exceeding max_len in an 8-byte + // totalsize smaller than header or exceeding max_size in an 8-byte // buffer is rejected before reading 40 bytes or slicing. for bad_size in [ 0u32, @@ -1043,8 +1158,13 @@ mod tests { hdr[FDT_TAGSIZE..2 * FDT_TAGSIZE].copy_from_slice(&bad_size.to_be_bytes()); // SAFETY: `hdr` has 8 readable bytes and `bad_size` is outside // `size_of::()..=FDT_HEADER_OK.len()`. - let err = - unsafe { Fdt::from_raw_bounded(hdr.as_ptr(), FDT_HEADER_OK.len()) }.unwrap_err(); + let err = unsafe { + Fdt::from_raw_with_options( + hdr.as_ptr(), + FdtOptions::new().max_size(FDT_HEADER_OK.len()), + ) + } + .unwrap_err(); assert_eq!( err, FdtParseError::new( @@ -1056,14 +1176,23 @@ mod tests { // SAFETY: `FDT_HEADER_OK` is a valid FDT of length // `FDT_HEADER_OK.len()`. - let fdt = - unsafe { Fdt::from_raw_bounded(FDT_HEADER_OK.as_ptr(), FDT_HEADER_OK.len()) }.unwrap(); + let fdt = unsafe { + Fdt::from_raw_with_options( + FDT_HEADER_OK.as_ptr(), + FdtOptions::new().max_size(FDT_HEADER_OK.len()), + ) + } + .unwrap(); assert_eq!(fdt.data().len(), FDT_HEADER_OK.len()); // SAFETY: `FDT_HEADER_OK` is a valid FDT of `totalsize == - // FDT_HEADER_OK.len() <= max_len`. - let fdt = - unsafe { Fdt::from_raw_bounded(FDT_HEADER_OK.as_ptr(), FDT_HEADER_OK.len() + 64) } - .unwrap(); + // FDT_HEADER_OK.len() <= max_size`. + let fdt = unsafe { + Fdt::from_raw_with_options( + FDT_HEADER_OK.as_ptr(), + FdtOptions::new().max_size(FDT_HEADER_OK.len() + 64), + ) + } + .unwrap(); assert_eq!(fdt.data().len(), FDT_HEADER_OK.len()); } } diff --git a/src/fdt/node.rs b/src/fdt/node.rs index b703493..4706eda 100644 --- a/src/fdt/node.rs +++ b/src/fdt/node.rs @@ -10,7 +10,7 @@ use core::fmt::{self, Display, Formatter}; -use super::{FDT_TAGSIZE, Fdt, FdtToken}; +use super::{FDT_TAGSIZE, Fdt, FdtToken, NameValidation}; use crate::Node; use crate::fdt::property::{FdtPropIter, FdtProperty, InnerPropIter}; use crate::standard::{AddressSpaceProperties, NodeStandard}; @@ -171,7 +171,7 @@ impl InnerChildIter { FdtToken::BeginNode => return Some(*offset), FdtToken::Prop => { *offset = fdt - .next_property_offset(*offset, false) + .next_property_offset(*offset, NameValidation::Skip) .expect("Fdt should be valid"); } FdtToken::EndNode | FdtToken::End => return None, diff --git a/src/fdt/property.rs b/src/fdt/property.rs index a07e930..4de6439 100644 --- a/src/fdt/property.rs +++ b/src/fdt/property.rs @@ -13,7 +13,7 @@ use core::mem::size_of; use zerocopy::FromBytes; -use super::{FDT_TAGSIZE, Fdt, FdtPropertyHeader, FdtToken}; +use super::{FDT_TAGSIZE, Fdt, FdtPropertyHeader, FdtToken, NameValidation}; use crate::Property; /// A property of a device tree node. @@ -136,7 +136,7 @@ impl InnerPropIter { #[must_use] fn next_property_parsed<'a>(fdt: Fdt<'a>, offset: &mut usize) -> Option> { *offset = fdt - .next_property_offset(*offset, false) + .next_property_offset(*offset, NameValidation::Skip) .expect("Fdt should be valid"); Self::find_property(fdt, offset) } diff --git a/src/fdt_mut.rs b/src/fdt_mut.rs index 2205fa6..12ffa8a 100644 --- a/src/fdt_mut.rs +++ b/src/fdt_mut.rs @@ -25,7 +25,7 @@ use zerocopy::{FromBytes, big_endian}; use crate::ToPropertyValue; use crate::error::{BufferError, FdtParseError}; -use crate::fdt::{FDT_NOP, FDT_TAGSIZE, Fdt, FdtHeader, FdtToken}; +use crate::fdt::{FDT_NOP, FDT_TAGSIZE, Fdt, FdtHeader, FdtOptions, FdtToken}; /// A mutable flattened device tree. pub struct FdtMut { @@ -39,7 +39,18 @@ impl FdtMut { /// /// Returns an [`FdtParseError`] if the data is not a valid device tree. pub fn new(data: B) -> Result { - Fdt::new(data.as_ref())?; + Self::with_options(data, FdtOptions::new()) + } + + /// Creates a new mutable FDT from a buffer using the specified + /// [`FdtOptions`]. + /// + /// # Errors + /// + /// Returns an [`FdtParseError`] if the data is not a valid device tree + /// according to `options`. + pub fn with_options(data: B, options: FdtOptions) -> Result { + Fdt::with_options(data.as_ref(), options)?; Ok(Self { data }) } @@ -393,8 +404,22 @@ impl<'a> FdtMut> { /// let mut node = fdt.find_node_mut("/a/b/c").unwrap(); /// ``` pub fn from_slice(slice: &'a mut [u8]) -> Result { + Self::from_slice_with_options(slice, FdtOptions::new()) + } + + /// Creates a new `FdtMut` from a mutable byte slice using the specified + /// [`FdtOptions`] by wrapping it in a [`SliceBuffer`]. + /// + /// # Errors + /// + /// Returns an [`FdtParseError`] if `slice` does not contain a valid device + /// tree according to `options`. + pub fn from_slice_with_options( + slice: &'a mut [u8], + options: FdtOptions, + ) -> Result { let buffer = SliceBuffer::new(slice)?; - Self::new(buffer) + Self::with_options(buffer, options) } /// Creates a new `FdtMut` from a mutable byte slice without validation. @@ -445,36 +470,38 @@ impl<'a> FdtMut> { Self::from_slice_unchecked(slice) } - /// Creates a new `FdtMut` from the given pointer, verifying that the header - /// and `totalsize` fit within `max_len` bytes before reading the rest of - /// the blob. + /// Creates a new `FdtMut` from the given pointer using the specified + /// [`FdtOptions`], verifying that the header and `totalsize` fit within + /// `options.max_size` bytes before reading the rest of the blob. /// /// The resulting [`SliceBuffer`] is sized to `totalsize` bytes (not - /// `max_len`); callers with a larger writable buffer that want to use the - /// remaining space for in-place expansion should use + /// `options.max_size`); callers with a larger writable buffer that want to + /// use the remaining space for in-place expansion should use /// [`FdtMut::from_slice`]. /// /// # Safety /// - /// If `max_len >= size_of::()` (40 bytes), `data` must be - /// valid and accessible for reading at least 8 bytes (`magic` and + /// If `options.max_size >= size_of::()` (40 bytes), `data` must + /// be valid and accessible for reading at least 8 bytes (`magic` and /// `totalsize`), and if the header contains a valid FDT magic number and - /// `40 <= totalsize <= max_len`, the memory region starting at `data` and - /// spanning `totalsize` bytes must be valid and accessible for reading and - /// writing for the lifetime `'a`. + /// `40 <= totalsize <= options.max_size`, the memory region starting at + /// `data` and spanning `totalsize` bytes must be valid and accessible for + /// reading and writing for the lifetime `'a`. /// /// # Errors /// - /// This function can return the same errors as [`Fdt::from_raw_bounded`]. + /// This function can return the same errors as + /// [`Fdt::from_raw_with_options`]. /// /// # Examples /// /// ``` + /// # use dtoolkit::fdt::FdtOptions; /// # use dtoolkit::fdt_mut::FdtMut; /// # let mut dtb = include_bytes!("../tests/dtb/test.dtb").to_vec(); - /// let len = dtb.len(); + /// let options = FdtOptions::new().max_size(dtb.len()); /// let ptr = dtb.as_mut_ptr(); - /// let fdt = unsafe { FdtMut::from_raw_bounded(ptr, len).unwrap() }; + /// let fdt = unsafe { FdtMut::from_raw_with_options(ptr, options).unwrap() }; /// ``` #[expect( unsafe_code, @@ -482,16 +509,20 @@ impl<'a> FdtMut> { embedded applications, where the binary only gets a pointer to DT from the firmware or \ a bootloader. The user must ensure it trusts the data." )] - pub unsafe fn from_raw_bounded(data: *mut u8, max_len: usize) -> Result { + pub unsafe fn from_raw_with_options( + data: *mut u8, + options: FdtOptions, + ) -> Result { // SAFETY: The caller guarantees that `data` points to a readable FDT - // header prefix (when `max_len >= size_of::()`) and spans - // `totalsize` writable bytes when the FDT is valid and - // `40 <= totalsize <= max_len`. - let size = unsafe { Fdt::from_raw_bounded(data.cast_const(), max_len)? } + // header prefix (when `options.max_size >= size_of::()`) and + // spans `totalsize` writable bytes when the FDT is valid and + // `40 <= totalsize <= options.max_size`. + let size = unsafe { Fdt::from_raw_with_options(data.cast_const(), options)? } .data() .len(); - // SAFETY: `Fdt::from_raw_bounded` validated the entire `size`-byte FDT - // blob and verified `size <= max_len` (and `size <= isize::MAX`). + // SAFETY: `Fdt::from_raw_with_options` validated the entire `size`-byte + // FDT blob and verified `size <= options.max_size` (and + // `size <= isize::MAX`). let slice = unsafe { core::slice::from_raw_parts_mut(data, size) }; Ok(Self::from_slice_unchecked(slice)) } @@ -527,7 +558,7 @@ impl<'a> FdtMut> { pub unsafe fn from_raw(data: *mut u8) -> Result { // SAFETY: The caller guarantees that `data` points to a readable FDT // header and spans `totalsize` writable bytes when the header is valid. - unsafe { Self::from_raw_bounded(data, usize::MAX) } + unsafe { Self::from_raw_with_options(data, FdtOptions::new()) } } } diff --git a/src/fdt_mut/node.rs b/src/fdt_mut/node.rs index f67bf0c..112042c 100644 --- a/src/fdt_mut/node.rs +++ b/src/fdt_mut/node.rs @@ -15,7 +15,7 @@ use crate::error::FdtMutError; use crate::fdt::node::InnerChildIter; use crate::fdt::node::private::FdtChildIter; use crate::fdt::property::{FdtPropIter, InnerPropIter}; -use crate::fdt::{FDT_TAGSIZE, Fdt, FdtNode, FdtProperty, FdtPropertyHeader}; +use crate::fdt::{FDT_TAGSIZE, Fdt, FdtNode, FdtProperty, FdtPropertyHeader, NameValidation}; use crate::fdt_mut::buffer::FdtBuffer; use crate::fdt_mut::property::FdtPropMutIter; use crate::fdt_mut::{FdtMut, FdtPropertyMut}; @@ -97,7 +97,9 @@ impl FdtNodeMut<'_, B> { let mut offset = self.offset + FDT_TAGSIZE; let name_end = fdt.find_string_end(offset).expect("valid node name"); offset = Fdt::align_tag_offset(name_end); - let insert_offset = fdt.skip_props(offset, false).expect("valid dt"); + let insert_offset = fdt + .skip_props(offset, NameValidation::Skip) + .expect("Fdt should be valid"); let val_len = value.property_value_len(); let padded_val_len = Fdt::align_tag_offset(val_len); diff --git a/src/model/node.rs b/src/model/node.rs index 3851606..6efad5a 100644 --- a/src/model/node.rs +++ b/src/model/node.rs @@ -159,7 +159,7 @@ impl DeviceTreeNode { /// ``` pub fn new(name: impl Into) -> Result { let name = name.into(); - if !crate::validate::is_valid_node_name(&name) { + if !crate::validate::is_valid_node_name(&name, false) { return Err(ModelError::InvalidNodeName(name)); } Ok(Self::new_unchecked(name)) diff --git a/src/model/property.rs b/src/model/property.rs index 028b6a7..c963691 100644 --- a/src/model/property.rs +++ b/src/model/property.rs @@ -50,7 +50,7 @@ impl DeviceTreeProperty { /// ``` pub fn new(name: impl Into, value: T) -> Result { let name = name.into(); - if !crate::validate::is_valid_property_name(&name) { + if !crate::validate::is_valid_property_name(&name, false) { return Err(ModelError::InvalidPropertyName(name)); } Ok(Self::new_unchecked(name, value)) diff --git a/src/validate.rs b/src/validate.rs index e48381e..41ae23b 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -8,6 +8,8 @@ //! Validation functions for Device Tree node and property names. +const MAX_NAME_LEN: usize = 31; + const fn is_valid_node_char(c: char) -> bool { c.is_ascii_alphanumeric() || matches!(c, ',' | '.' | '_' | '+' | '-') } @@ -18,8 +20,11 @@ const fn is_valid_property_char(c: char) -> bool { /// Validates a node name according to the Devicetree Specification. /// +/// If `strict_length` is `true`, the node name (before `@`) must not exceed 31 +/// characters. +/// /// Returns `true` if valid, `false` otherwise. -pub(crate) fn is_valid_node_name(name: &str) -> bool { +pub(crate) fn is_valid_node_name(name: &str, strict_length: bool) -> bool { // The root node is a special case with an empty name. if name.is_empty() { return true; @@ -33,7 +38,7 @@ pub(crate) fn is_valid_node_name(name: &str) -> bool { return false; } - if node_name.is_empty() || node_name.len() > 31 { + if node_name.is_empty() || (strict_length && node_name.len() > MAX_NAME_LEN) { return false; } @@ -56,9 +61,12 @@ pub(crate) fn is_valid_node_name(name: &str) -> bool { /// Validates a property name according to the Devicetree Specification. /// +/// If `strict_length` is `true`, the property name must not exceed 31 +/// characters. +/// /// Returns `true` if valid, `false` otherwise. -pub(crate) fn is_valid_property_name(name: &str) -> bool { - if name.is_empty() || name.len() > 31 { +pub(crate) fn is_valid_property_name(name: &str, strict_length: bool) -> bool { + if name.is_empty() || (strict_length && name.len() > MAX_NAME_LEN) { return false; } @@ -70,43 +78,53 @@ mod tests { use super::*; #[test] - fn test_valid_node_names() { - assert!(is_valid_node_name("")); - assert!(is_valid_node_name("cpu")); - assert!(is_valid_node_name("cpu@0")); - assert!(is_valid_node_name("memory@10000000")); - assert!(is_valid_node_name("serial@3f201000")); - assert!(is_valid_node_name("a,b.c_d+e-f")); + fn valid_node_names() { + assert!(is_valid_node_name("", true)); + assert!(is_valid_node_name("cpu", true)); + assert!(is_valid_node_name("cpu@0", true)); + assert!(is_valid_node_name("memory@10000000", true)); + assert!(is_valid_node_name("serial@3f201000", true)); + assert!(is_valid_node_name("a,b.c_d+e-f", true)); + assert!(is_valid_node_name( + "this-is-a-very-long-node-name-that-exceeds-thirty-one-characters", + false + )); } #[test] - fn test_invalid_node_names() { - assert!(!is_valid_node_name("cpu@0@1")); // Too many '@' - assert!(!is_valid_node_name("cpu@")); // Empty unit address - assert!(!is_valid_node_name("@0")); // Empty node name - assert!(!is_valid_node_name("cpu name")); // Invalid space - assert!(!is_valid_node_name("cpu#name")); // Invalid '#' + fn invalid_node_names() { + assert!(!is_valid_node_name("cpu@0@1", false)); // Too many '@' + assert!(!is_valid_node_name("cpu@", false)); // Empty unit address + assert!(!is_valid_node_name("@0", false)); // Empty node name + assert!(!is_valid_node_name("cpu name", false)); // Invalid space + assert!(!is_valid_node_name("cpu#name", false)); // Invalid '#' assert!(!is_valid_node_name( - "this-is-a-very-long-node-name-that-exceeds-thirty-one-characters" + "this-is-a-very-long-node-name-that-exceeds-thirty-one-characters", + true )); // > 31 chars } #[test] - fn test_valid_property_names() { - assert!(is_valid_property_name("reg")); - assert!(is_valid_property_name("#address-cells")); - assert!(is_valid_property_name("device_type")); - assert!(is_valid_property_name("compatible")); - assert!(is_valid_property_name("a,b.c_d+e-f?#")); + fn valid_property_names() { + assert!(is_valid_property_name("reg", true)); + assert!(is_valid_property_name("#address-cells", true)); + assert!(is_valid_property_name("device_type", true)); + assert!(is_valid_property_name("compatible", true)); + assert!(is_valid_property_name("a,b.c_d+e-f?#", true)); + assert!(is_valid_property_name( + "snps,dis-u2-freeclk-exists-quirk", + false + )); } #[test] - fn test_invalid_property_names() { - assert!(!is_valid_property_name("")); // Empty - assert!(!is_valid_property_name("my prop")); // Invalid space - assert!(!is_valid_property_name("my@prop")); // Invalid '@' + fn invalid_property_names() { + assert!(!is_valid_property_name("", false)); // Empty + assert!(!is_valid_property_name("my prop", false)); // Invalid space + assert!(!is_valid_property_name("my@prop", false)); // Invalid '@' assert!(!is_valid_property_name( - "this-is-a-very-long-property-name-that-exceeds-thirty-one-characters" + "this-is-a-very-long-property-name-that-exceeds-thirty-one-characters", + true )); // > 31 chars } } diff --git a/tests/fdt.rs b/tests/fdt.rs index 8719dbc..7900546 100644 --- a/tests/fdt.rs +++ b/tests/fdt.rs @@ -6,7 +6,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use dtoolkit::fdt::{Fdt, FdtNode}; +use dtoolkit::error::FdtErrorKind; +use dtoolkit::fdt::{Fdt, FdtNode, FdtOptions}; #[cfg(feature = "write")] use dtoolkit::fdt_mut::FdtMut; #[cfg(feature = "write")] @@ -353,9 +354,11 @@ fn round_trip_raw() { #[test] #[cfg(feature = "write")] -fn round_trip_raw_bounded() { +fn round_trip_raw_with_options() { // SAFETY: dtb is a valid FDT blob - round_trip_impl(|dtb| unsafe { Fdt::from_raw_bounded(dtb.as_ptr(), dtb.len()).unwrap() }); + round_trip_impl(|dtb| unsafe { + Fdt::from_raw_with_options(dtb.as_ptr(), FdtOptions::new().max_size(dtb.len())).unwrap() + }); } #[test] @@ -386,10 +389,11 @@ fn round_trip_raw_mut() { #[test] #[cfg(feature = "write")] -fn round_trip_raw_bounded_mut() { +fn round_trip_raw_with_options_mut() { // SAFETY: dtb is a valid FDT blob round_trip_impl(|dtb| unsafe { - FdtMut::from_raw_bounded(dtb.as_mut_ptr(), dtb.len()) + let len = dtb.len(); + FdtMut::from_raw_with_options(dtb.as_mut_ptr(), FdtOptions::new().max_size(len)) .unwrap() .into() }); @@ -580,3 +584,54 @@ fn node_at_offset_deep_nesting_preserves_parent_address_space() { assert_eq!(regs[0].address::().unwrap(), 0x1234_5678); assert_eq!(regs[0].size::().unwrap(), 0x100); } + +#[test] +#[cfg(feature = "write")] +fn long_property_and_node_names_accepted_by_default() { + let long_prop = "snps,dis-u2-freeclk-exists-quirk"; + let long_node = "node-name-exceeding-thirty-one-chars@0"; + let mut tree = DeviceTree::new(); + let child = DeviceTreeNode::builder(long_node) + .unwrap() + .property(DeviceTreeProperty::new(long_prop, [0u8; 0]).unwrap()) + .build(); + tree.root.add_child(child); + let dtb = tree.to_dtb(); + + let fdt = Fdt::new(&dtb).unwrap(); + let node = fdt + .find_node("/node-name-exceeding-thirty-one-chars") + .unwrap(); + assert_eq!(node.name(), long_node); + assert!(node.property(long_prop).is_some()); +} + +#[test] +#[cfg(feature = "write")] +fn strict_name_length_rejects_long_property_and_node_names() { + let strict = FdtOptions::new().strict_name_length(true); + + let mut prop_tree = DeviceTree::new(); + prop_tree.root.add_property( + DeviceTreeProperty::new("snps,dis-u2-freeclk-exists-quirk", [0u8; 0]).unwrap(), + ); + let prop_dtb = prop_tree.to_dtb(); + let err = Fdt::with_options(&prop_dtb, strict).unwrap_err(); + assert_eq!(err.kind, FdtErrorKind::InvalidPropertyName); + + let mut node_tree = DeviceTree::new(); + node_tree + .root + .add_child(DeviceTreeNode::new("node-name-exceeding-thirty-one-chars@0").unwrap()); + let node_dtb = node_tree.to_dtb(); + let err = Fdt::with_options(&node_dtb, strict).unwrap_err(); + assert_eq!(err.kind, FdtErrorKind::InvalidNodeName); +} + +#[test] +fn with_options_max_size_rejects_oversized_slice() { + let dtb = include_bytes!("dtb/test.dtb"); + let options = FdtOptions::new().max_size(dtb.len() - 1); + let err = Fdt::with_options(dtb, options).unwrap_err(); + assert_eq!(err.kind, FdtErrorKind::InvalidLength); +} diff --git a/tests/fdt_mut.rs b/tests/fdt_mut.rs index 2656e69..486a4c7 100644 --- a/tests/fdt_mut.rs +++ b/tests/fdt_mut.rs @@ -7,7 +7,7 @@ // except according to those terms. use dtoolkit::error::{BufferError, FdtErrorKind, FdtMutError}; -use dtoolkit::fdt::Fdt; +use dtoolkit::fdt::{Fdt, FdtOptions}; use dtoolkit::fdt_mut::{FdtBuffer, FdtMut, SliceBuffer}; use dtoolkit::{Node, Property}; #[cfg(feature = "alloc")] @@ -635,25 +635,37 @@ fn node_at_offset_mut_and_nop_rejection() { } #[test] -fn from_raw_bounded_mut_rejects_invalid_headers() { +fn from_raw_with_options_mut_rejects_invalid_headers() { let dtb = include_bytes!("dtb/test_traversal.dtb"); - // max_len smaller than header does not dereference pointer. - // SAFETY: `max_len < 40`, so `from_raw_bounded` returns before + // max_size smaller than header does not dereference pointer. + // SAFETY: `max_size < 40`, so `from_raw_with_options` returns before // dereferencing `data`. - let err = unsafe { FdtMut::from_raw_bounded(core::ptr::null_mut(), 39) }.unwrap_err(); + let err = unsafe { + FdtMut::from_raw_with_options(core::ptr::null_mut(), FdtOptions::new().max_size(39)) + } + .unwrap_err(); assert_eq!(err.kind, FdtErrorKind::InvalidLength); // 8-byte buffer with bad magic or out-of-range totalsize is rejected // without reading 40 bytes. let mut bad_magic = [0u8; 8]; // SAFETY: `bad_magic` is 8 readable bytes and has invalid magic. - let err = unsafe { FdtMut::from_raw_bounded(bad_magic.as_mut_ptr(), 1024) }.unwrap_err(); + let err = unsafe { + FdtMut::from_raw_with_options(bad_magic.as_mut_ptr(), FdtOptions::new().max_size(1024)) + } + .unwrap_err(); assert_eq!(err.kind, FdtErrorKind::InvalidMagic); let mut huge_size = [0xd0, 0x0d, 0xfe, 0xed, 0xff, 0xff, 0xff, 0xff]; - // SAFETY: `huge_size` is 8 readable bytes and `totalsize > max_len`. - let err = unsafe { FdtMut::from_raw_bounded(huge_size.as_mut_ptr(), dtb.len()) }.unwrap_err(); + // SAFETY: `huge_size` is 8 readable bytes and `totalsize > max_size`. + let err = unsafe { + FdtMut::from_raw_with_options( + huge_size.as_mut_ptr(), + FdtOptions::new().max_size(dtb.len()), + ) + } + .unwrap_err(); assert_eq!(err.kind, FdtErrorKind::InvalidLength); }