diff --git a/integration-test/bins/multiboot2_payload/src/verify/mod.rs b/integration-test/bins/multiboot2_payload/src/verify/mod.rs index 407edb81..787d3ebe 100644 --- a/integration-test/bins/multiboot2_payload/src/verify/mod.rs +++ b/integration-test/bins/multiboot2_payload/src/verify/mod.rs @@ -55,7 +55,9 @@ pub(self) fn print_elf_info(mbi: &BootInformation) -> anyhow::Result<()> { let string_table = mbi .elf_sections_tag() .ok_or("Should have elf sections") - .map(|tag| tag.string_table()) + // SAFETY: The bootloader loaded the ELF sections at their reported + // addresses in identity-mapped memory. + .map(|tag| unsafe { tag.string_table() }) .map_err(anyhow::Error::msg)? .ok_or("String table section should be present") .map_err(anyhow::Error::msg)?; diff --git a/multiboot2-common/CHANGELOG.md b/multiboot2-common/CHANGELOG.md index 17130847..d7ead3a3 100644 --- a/multiboot2-common/CHANGELOG.md +++ b/multiboot2-common/CHANGELOG.md @@ -5,6 +5,9 @@ - Added the `raw_type!` macro that generates an ABI-safe `#[repr(transparent)]` newtype plus a corresponding high-level open-set enum, including all conversions between them and the underlying integer. +- **Breaking:** `Header` and `MaybeDynSized` are now `unsafe` traits, as this + crate creates references from raw memory based on their implementations. + The safety contracts implementors must uphold are now documented. ## v0.5.0 (2026-08-24) diff --git a/multiboot2-common/src/lib.rs b/multiboot2-common/src/lib.rs index d3c076f6..bf7f1f16 100644 --- a/multiboot2-common/src/lib.rs +++ b/multiboot2-common/src/lib.rs @@ -281,7 +281,13 @@ pub const ALIGNMENT: usize = 8; /// /// The alignment of implementors **must** be compatible with the requirements /// for the corresponding structure, which typically is [`ALIGNMENT`]. -pub trait Header: Clone + Sized + PartialEq + Eq + Debug { +/// +/// # Safety +/// +/// Implementors must be `#[repr(C)]`, have no padding bytes or interior +/// mutability, allow every bit pattern, and have an alignment of at most +/// [`ALIGNMENT`]. Headers are referenced from raw memory and copied byte-wise. +pub unsafe trait Header: Clone + Sized + PartialEq + Eq + Debug { /// Returns the total size of the structure in bytes, including the fixed /// header and any dynamic payload. #[must_use] @@ -588,7 +594,9 @@ mod tests { b: u32, } - impl MaybeDynSized for CustomSizedTag { + // SAFETY: The tag is repr(C) with the header as first field, any + // bit pattern is valid, and `BASE_SIZE` matches the ABI. + unsafe impl MaybeDynSized for CustomSizedTag { type Header = DummyTestHeader; const BASE_SIZE: usize = size_of::(); @@ -661,7 +669,9 @@ mod tests { b: u32, } - impl MaybeDynSized for CustomSizedTag { + // SAFETY: The tag is repr(C) with the header as first field, any + // bit pattern is valid, and `BASE_SIZE` matches the ABI. + unsafe impl MaybeDynSized for CustomSizedTag { type Header = DummyTestHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2-common/src/tag.rs b/multiboot2-common/src/tag.rs index 6f21a4a3..459d333a 100644 --- a/multiboot2-common/src/tag.rs +++ b/multiboot2-common/src/tag.rs @@ -14,16 +14,21 @@ use ptr_meta::Pointee; /// implementation. Only dynamically sized types need to implement /// [`MaybeDynSized::dst_len`]. /// -/// # ABI -/// Implementors **must** use `#[repr(C)]`. As there might be padding necessary -/// for the proper Rust layout, `size_of_val(&self)` might report additional -/// padding bytes that are not reflected by the actual payload. These additional -/// padding bytes however will be reflected in corresponding [`BytesRef`] -/// instances. +/// # Safety +/// +/// Implementors must be `#[repr(C)]`, start with `Self::Header`, have an +/// alignment of at most [`ALIGNMENT`], and allow every bit pattern. +/// +/// [`MaybeDynSized::BASE_SIZE`], [`MaybeDynSized::dst_len`], and +/// [`Header::total_size`] must correctly describe the initialized, +/// contiguous memory backing the value. Incorrect sizes or implicit padding +/// within the reported range can cause out-of-bounds references. Trailing +/// padding beyond that range is fine. /// /// [`ID`]: Tag::ID +/// [`ALIGNMENT`]: crate::ALIGNMENT /// [`DynSizedStructure`]: crate::DynSizedStructure -pub trait MaybeDynSized: Pointee { +pub unsafe trait MaybeDynSized: Pointee { /// The associated [`Header`] of this tag. type Header: Header; @@ -105,7 +110,9 @@ pub trait Tag: MaybeDynSized { // This implementation is not needed for parsing but for creation, when // downstream types just wrap this type. -impl MaybeDynSized for DynSizedStructure { +// SAFETY: `DynSizedStructure` is repr(C) with the header as first field, +// any bit pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for DynSizedStructure { type Header = H; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2-common/src/test_utils.rs b/multiboot2-common/src/test_utils.rs index 5cd9d4c1..238cf234 100644 --- a/multiboot2-common/src/test_utils.rs +++ b/multiboot2-common/src/test_utils.rs @@ -70,7 +70,9 @@ impl DummyTestHeader { } } -impl Header for DummyTestHeader { +// SAFETY: The header is a padding-free repr(C) struct of raw integers, and +// any bit pattern is valid for it. +unsafe impl Header for DummyTestHeader { fn total_size(&self) -> usize { self.size as usize } @@ -101,7 +103,9 @@ impl DummyDstTag { } } -impl MaybeDynSized for DummyDstTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for DummyDstTag { type Header = DummyTestHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2-header/CHANGELOG.md b/multiboot2-header/CHANGELOG.md index df07893a..bde97dd7 100644 --- a/multiboot2-header/CHANGELOG.md +++ b/multiboot2-header/CHANGELOG.md @@ -28,6 +28,10 @@ `2` (was `1`), matching the example C code of the specification; the enum gained a `Custom` variant and the tag stores the new `ConsoleHeaderTagFlagsRaw` newtype. +- Fixed a possible size underflow in `InformationRequestHeaderTag` parsing + when the tag reports a size smaller than the tag header. +- **Breaking:** The re-exported `MaybeDynSized` trait is now an `unsafe` + trait; implementations for custom tag types must now use `unsafe impl`. ## v0.10.0 (2026-08-24) diff --git a/multiboot2-header/src/address.rs b/multiboot2-header/src/address.rs index 543423cb..4e420b66 100644 --- a/multiboot2-header/src/address.rs +++ b/multiboot2-header/src/address.rs @@ -107,7 +107,9 @@ impl AddressHeaderTag { } } -impl MaybeDynSized for AddressHeaderTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for AddressHeaderTag { type Header = HeaderTagHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2-header/src/console.rs b/multiboot2-header/src/console.rs index 6b773978..0dc0cdda 100644 --- a/multiboot2-header/src/console.rs +++ b/multiboot2-header/src/console.rs @@ -67,7 +67,9 @@ impl ConsoleHeaderTag { } } -impl MaybeDynSized for ConsoleHeaderTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for ConsoleHeaderTag { type Header = HeaderTagHeader; const BASE_SIZE: usize = size_of::() + size_of::(); diff --git a/multiboot2-header/src/end.rs b/multiboot2-header/src/end.rs index a868d0d2..ff618ba8 100644 --- a/multiboot2-header/src/end.rs +++ b/multiboot2-header/src/end.rs @@ -47,7 +47,9 @@ impl EndHeaderTag { } } -impl MaybeDynSized for EndHeaderTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for EndHeaderTag { type Header = HeaderTagHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2-header/src/entry_address.rs b/multiboot2-header/src/entry_address.rs index a8e3f2ba..8737ea3d 100644 --- a/multiboot2-header/src/entry_address.rs +++ b/multiboot2-header/src/entry_address.rs @@ -57,7 +57,9 @@ impl Debug for EntryAddressHeaderTag { } } -impl MaybeDynSized for EntryAddressHeaderTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for EntryAddressHeaderTag { type Header = HeaderTagHeader; const BASE_SIZE: usize = size_of::() + size_of::(); diff --git a/multiboot2-header/src/entry_efi_32.rs b/multiboot2-header/src/entry_efi_32.rs index e808179a..9eba83f3 100644 --- a/multiboot2-header/src/entry_efi_32.rs +++ b/multiboot2-header/src/entry_efi_32.rs @@ -66,7 +66,9 @@ impl Debug for EntryEfi32HeaderTag { } } -impl MaybeDynSized for EntryEfi32HeaderTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for EntryEfi32HeaderTag { type Header = HeaderTagHeader; const BASE_SIZE: usize = size_of::() + size_of::(); diff --git a/multiboot2-header/src/entry_efi_64.rs b/multiboot2-header/src/entry_efi_64.rs index 856a5d48..be051c51 100644 --- a/multiboot2-header/src/entry_efi_64.rs +++ b/multiboot2-header/src/entry_efi_64.rs @@ -66,7 +66,9 @@ impl Debug for EntryEfi64HeaderTag { } } -impl MaybeDynSized for EntryEfi64HeaderTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for EntryEfi64HeaderTag { type Header = HeaderTagHeader; const BASE_SIZE: usize = size_of::() + size_of::(); diff --git a/multiboot2-header/src/framebuffer.rs b/multiboot2-header/src/framebuffer.rs index b9091947..50633e62 100644 --- a/multiboot2-header/src/framebuffer.rs +++ b/multiboot2-header/src/framebuffer.rs @@ -65,7 +65,9 @@ impl FramebufferHeaderTag { } } -impl MaybeDynSized for FramebufferHeaderTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for FramebufferHeaderTag { type Header = HeaderTagHeader; const BASE_SIZE: usize = size_of::() + 3 * size_of::(); diff --git a/multiboot2-header/src/header.rs b/multiboot2-header/src/header.rs index 7739ee45..a79162ed 100644 --- a/multiboot2-header/src/header.rs +++ b/multiboot2-header/src/header.rs @@ -427,7 +427,9 @@ impl Multiboot2BasicHeader { } } -impl DynSizedHeader for Multiboot2BasicHeader { +// SAFETY: The header is a padding-free repr(C) struct of raw integers, and +// any bit pattern is valid for it. +unsafe impl DynSizedHeader for Multiboot2BasicHeader { fn total_size(&self) -> usize { self.length as usize } diff --git a/multiboot2-header/src/information_request.rs b/multiboot2-header/src/information_request.rs index 7404a72e..e1064ae3 100644 --- a/multiboot2-header/src/information_request.rs +++ b/multiboot2-header/src/information_request.rs @@ -75,12 +75,15 @@ impl Debug for InformationRequestHeaderTag { } } -impl MaybeDynSized for InformationRequestHeaderTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for InformationRequestHeaderTag { type Header = HeaderTagHeader; const BASE_SIZE: usize = size_of::(); fn dst_len(header: &Self::Header) -> Self::Metadata { + assert!(header.size() as usize >= Self::BASE_SIZE); let dst_size = header.size() as usize - Self::BASE_SIZE; assert_eq!(dst_size % size_of::(), 0); dst_size / size_of::() @@ -97,6 +100,17 @@ impl Tag for InformationRequestHeaderTag { mod tests { use super::*; + #[test] + #[should_panic] + fn dst_len_rejects_undersized_header() { + let header = HeaderTagHeader::new( + HeaderTagType::InformationRequest, + HeaderTagFlag::Optional, + 4, + ); + let _ = ::dst_len(&header); + } + #[test] fn creation() { let requests = [ diff --git a/multiboot2-header/src/module_align.rs b/multiboot2-header/src/module_align.rs index ce0c43c6..d156f9e6 100644 --- a/multiboot2-header/src/module_align.rs +++ b/multiboot2-header/src/module_align.rs @@ -38,7 +38,9 @@ impl ModuleAlignHeaderTag { } } -impl MaybeDynSized for ModuleAlignHeaderTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for ModuleAlignHeaderTag { type Header = HeaderTagHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2-header/src/relocatable.rs b/multiboot2-header/src/relocatable.rs index 9c7b7eb8..734cfc26 100644 --- a/multiboot2-header/src/relocatable.rs +++ b/multiboot2-header/src/relocatable.rs @@ -125,7 +125,9 @@ impl Debug for RelocatableHeaderTag { } } -impl MaybeDynSized for RelocatableHeaderTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for RelocatableHeaderTag { type Header = HeaderTagHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2-header/src/tags.rs b/multiboot2-header/src/tags.rs index 87fdab8c..88cdb5b7 100644 --- a/multiboot2-header/src/tags.rs +++ b/multiboot2-header/src/tags.rs @@ -126,7 +126,9 @@ impl HeaderTagHeader { } } -impl Header for HeaderTagHeader { +// SAFETY: The header is a padding-free repr(C) struct of raw integers, and +// any bit pattern is valid for it. +unsafe impl Header for HeaderTagHeader { fn total_size(&self) -> usize { self.size as usize } diff --git a/multiboot2-header/src/uefi_bs.rs b/multiboot2-header/src/uefi_bs.rs index 29a88787..f3cb1b4d 100644 --- a/multiboot2-header/src/uefi_bs.rs +++ b/multiboot2-header/src/uefi_bs.rs @@ -38,7 +38,9 @@ impl EfiBootServiceHeaderTag { } } -impl MaybeDynSized for EfiBootServiceHeaderTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for EfiBootServiceHeaderTag { type Header = HeaderTagHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2/CHANGELOG.md b/multiboot2/CHANGELOG.md index 818f24c9..2c195afb 100644 --- a/multiboot2/CHANGELOG.md +++ b/multiboot2/CHANGELOG.md @@ -19,6 +19,13 @@ - **Breaking:** `EFIMemoryDesc` (re-exported from `uefi-raw` crate) has a new padding field which is required for correct type layout on some non-UEFI 32-bit targets. +- Fixed a possible size underflow in `NetworkTag` parsing when the tag reports + a size smaller than the tag header. +- **Breaking:** `ElfSectionsTag::string_table()` is now an `unsafe fn`. The + string table is not part of the tag itself; the caller must ensure that the + referenced section memory is still loaded and valid. +- **Breaking:** The re-exported `MaybeDynSized` trait is now an `unsafe` + trait; implementations for custom tag types must now use `unsafe impl`. ## v0.26.0 / v0.26.1 (2026-08-24) diff --git a/multiboot2/src/apm.rs b/multiboot2/src/apm.rs index fe2f158e..8edebf0f 100644 --- a/multiboot2/src/apm.rs +++ b/multiboot2/src/apm.rs @@ -109,7 +109,9 @@ impl ApmTag { } } -impl MaybeDynSized for ApmTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for ApmTag { type Header = TagHeader; // Spec size (28), excluding the trailing padding that `size_of::()` diff --git a/multiboot2/src/boot_information.rs b/multiboot2/src/boot_information.rs index 3d7ab125..11a7eaaf 100644 --- a/multiboot2/src/boot_information.rs +++ b/multiboot2/src/boot_information.rs @@ -55,7 +55,9 @@ impl BootInformationHeader { } } -impl Header for BootInformationHeader { +// SAFETY: The header is a padding-free repr(C) struct of raw integers, and +// any bit pattern is valid for it. +unsafe impl Header for BootInformationHeader { fn total_size(&self) -> usize { self.total_size as usize } @@ -391,7 +393,9 @@ impl<'a> BootInformation<'a> { /// } /// /// // Give the library hints how big this tag is. - /// impl MaybeDynSized for CustomTag { + /// // SAFETY: The tag is repr(C) with the header as first field, any bit + /// // pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. + /// unsafe impl MaybeDynSized for CustomTag { /// type Header = TagHeader; /// const BASE_SIZE: usize = size_of::() + size_of::(); /// diff --git a/multiboot2/src/boot_loader_name.rs b/multiboot2/src/boot_loader_name.rs index 0cc61f08..ccd90dff 100644 --- a/multiboot2/src/boot_loader_name.rs +++ b/multiboot2/src/boot_loader_name.rs @@ -74,7 +74,9 @@ impl Debug for BootLoaderNameTag { } } -impl MaybeDynSized for BootLoaderNameTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for BootLoaderNameTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2/src/bootdev.rs b/multiboot2/src/bootdev.rs index 92aa651a..d7e6235b 100644 --- a/multiboot2/src/bootdev.rs +++ b/multiboot2/src/bootdev.rs @@ -51,7 +51,9 @@ impl BootdevTag { } } -impl MaybeDynSized for BootdevTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for BootdevTag { type Header = TagHeader; // Spec size (20), excluding the trailing padding that `size_of::()` diff --git a/multiboot2/src/command_line.rs b/multiboot2/src/command_line.rs index e54d4f0c..724e4482 100644 --- a/multiboot2/src/command_line.rs +++ b/multiboot2/src/command_line.rs @@ -68,7 +68,9 @@ impl Debug for CommandLineTag { } } -impl MaybeDynSized for CommandLineTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for CommandLineTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2/src/efi.rs b/multiboot2/src/efi.rs index ace84e5a..c573dde2 100644 --- a/multiboot2/src/efi.rs +++ b/multiboot2/src/efi.rs @@ -37,7 +37,9 @@ impl EFISdt32Tag { } } -impl MaybeDynSized for EFISdt32Tag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for EFISdt32Tag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); @@ -74,7 +76,9 @@ impl EFISdt64Tag { } } -impl MaybeDynSized for EFISdt64Tag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for EFISdt64Tag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); @@ -114,7 +118,9 @@ impl EFIImageHandle32Tag { } } -impl MaybeDynSized for EFIImageHandle32Tag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for EFIImageHandle32Tag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); @@ -152,7 +158,9 @@ impl EFIImageHandle64Tag { } } -impl MaybeDynSized for EFIImageHandle64Tag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for EFIImageHandle64Tag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); @@ -188,7 +196,9 @@ impl Default for EFIBootServicesNotExitedTag { } } -impl MaybeDynSized for EFIBootServicesNotExitedTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for EFIBootServicesNotExitedTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2/src/elf_sections.rs b/multiboot2/src/elf_sections.rs index 392bbd6d..d6c93a0b 100644 --- a/multiboot2/src/elf_sections.rs +++ b/multiboot2/src/elf_sections.rs @@ -61,8 +61,17 @@ impl ElfSectionsTag { } /// Returns the string table data, if it's present. + /// + /// The string table is not part of the tag: the tag only stores the + /// physical address (`sh_addr`) and size of the section holding it. + /// + /// # Safety + /// + /// The caller must ensure that the string table section is still loaded + /// at its reported `sh_addr`, identity-mapped, readable for `sh_size` + /// bytes, and not modified for the lifetime of the returned slice. #[must_use] - pub fn string_table(&self) -> Option<&[u8]> { + pub unsafe fn string_table(&self) -> Option<&[u8]> { let shdr_table = SectionHeaderTable::new(NativeEndian, self.class(), &self.sections); // Info for this here @@ -76,8 +85,8 @@ impl ElfSectionsTag { let strtab_hdr = shdr_table.get(strtab_index).ok()?; // todo: Should this check that `strtab_hdr.sh_type == elf::abi::SHT_STRTAB`? - // SAFETY: The multiboot2 spec defines that sections are always loaded at `sh_addr`. - // Casting through `usize` will not truncate data on 32bit systems because the multiboot2 loads all sections below u32::MAX + // SAFETY: The caller guarantees that the section is loaded at + // `sh_addr` and readable for `sh_size` bytes. Some(unsafe { core::slice::from_raw_parts( core::ptr::with_exposed_provenance(strtab_hdr.sh_addr as usize), @@ -105,7 +114,9 @@ impl ElfSectionsTag { } } -impl MaybeDynSized for ElfSectionsTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for ElfSectionsTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::() + 3 * size_of::(); @@ -189,6 +200,18 @@ impl ElfSectionExt for SectionHeader { } } +#[cfg(all(test, feature = "builder"))] +mod tests { + use super::*; + + #[test] + fn string_table_returns_none_for_undef_shndx() { + let tag = ElfSectionsTag::new(0, size_of::() as u32, 0, &[]); + // SAFETY: With `shndx == SHN_UNDEF`, no memory is dereferenced. + assert_eq!(unsafe { tag.string_table() }, None); + } +} + /// An enum abstraction over raw ELF section types. #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] #[repr(u32)] diff --git a/multiboot2/src/end.rs b/multiboot2/src/end.rs index a9bb7faa..8eb8c9b4 100644 --- a/multiboot2/src/end.rs +++ b/multiboot2/src/end.rs @@ -18,7 +18,9 @@ impl Default for EndTag { } } -impl MaybeDynSized for EndTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for EndTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2/src/framebuffer.rs b/multiboot2/src/framebuffer.rs index dc86a491..1d5f1ac7 100644 --- a/multiboot2/src/framebuffer.rs +++ b/multiboot2/src/framebuffer.rs @@ -216,7 +216,9 @@ impl FramebufferTag { } } -impl MaybeDynSized for FramebufferTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for FramebufferTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::() diff --git a/multiboot2/src/image_load_addr.rs b/multiboot2/src/image_load_addr.rs index 67fda150..d2d4d66c 100644 --- a/multiboot2/src/image_load_addr.rs +++ b/multiboot2/src/image_load_addr.rs @@ -32,7 +32,9 @@ impl ImageLoadPhysAddrTag { self.load_base_addr } } -impl MaybeDynSized for ImageLoadPhysAddrTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for ImageLoadPhysAddrTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index c3db6997..d24d6033 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -876,7 +876,9 @@ mod tests { assert_eq!(addr, bi.start_address()); assert_eq!(addr + bytes.len(), bi.end_address()); assert_eq!(bytes.len(), bi.total_size()); - let strtab = bi.elf_sections_tag().unwrap().string_table().unwrap(); + // SAFETY: The test patched `sh_addr` of the string table section to + // point at a live, initialized buffer. + let strtab = unsafe { bi.elf_sections_tag().unwrap().string_table() }.unwrap(); let mut es = bi.elf_sections_tag().unwrap().sections(); let _s0 = es.next().expect("Should have one more section"); @@ -1119,7 +1121,9 @@ mod tests { assert_eq!(addr, bi.start_address()); assert_eq!(addr + bytes.0.len(), bi.end_address()); assert_eq!(bytes.0.len(), bi.total_size()); - let strtab = bi.elf_sections_tag().unwrap().string_table().unwrap(); + // SAFETY: The test patched `sh_addr` of the string table section to + // point at a live, initialized buffer. + let strtab = unsafe { bi.elf_sections_tag().unwrap().string_table() }.unwrap(); let mut es = bi.elf_sections_tag().unwrap().sections(); let s0 = es.next().expect("Should have one more sections"); @@ -1241,7 +1245,9 @@ mod tests { foo: u32, } - impl MaybeDynSized for CustomTag { + // SAFETY: The tag is repr(C) with the header as first field, any + // bit pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. + unsafe impl MaybeDynSized for CustomTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); @@ -1343,7 +1349,9 @@ mod tests { } } - impl MaybeDynSized for CustomTag { + // SAFETY: The tag is repr(C) with the header as first field, any + // bit pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. + unsafe impl MaybeDynSized for CustomTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::() + size_of::(); diff --git a/multiboot2/src/memory_map.rs b/multiboot2/src/memory_map.rs index 66c1eba4..d99d3920 100644 --- a/multiboot2/src/memory_map.rs +++ b/multiboot2/src/memory_map.rs @@ -74,7 +74,9 @@ impl MemoryMapTag { } } -impl MaybeDynSized for MemoryMapTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for MemoryMapTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::() + 2 * size_of::(); @@ -229,7 +231,9 @@ impl BasicMemoryInfoTag { } } -impl MaybeDynSized for BasicMemoryInfoTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for BasicMemoryInfoTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); @@ -342,7 +346,9 @@ impl Debug for EFIMemoryMapTag { } } -impl MaybeDynSized for EFIMemoryMapTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for EFIMemoryMapTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::() + 3 * size_of::(); diff --git a/multiboot2/src/module.rs b/multiboot2/src/module.rs index 8c57f498..dca1061f 100644 --- a/multiboot2/src/module.rs +++ b/multiboot2/src/module.rs @@ -70,7 +70,9 @@ impl ModuleTag { } } -impl MaybeDynSized for ModuleTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for ModuleTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::() + 2 * size_of::(); diff --git a/multiboot2/src/network.rs b/multiboot2/src/network.rs index e264f292..772dfbd1 100644 --- a/multiboot2/src/network.rs +++ b/multiboot2/src/network.rs @@ -25,12 +25,15 @@ impl NetworkTag { } } -impl MaybeDynSized for NetworkTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for NetworkTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); fn dst_len(header: &TagHeader) -> usize { + assert!(header.size as usize >= Self::BASE_SIZE); header.size as usize - Self::BASE_SIZE } } @@ -40,3 +43,15 @@ impl Tag for NetworkTag { const ID: TagType = TagType::Network; } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + #[should_panic] + fn dst_len_rejects_undersized_header() { + let header = TagHeader::new(TagType::Network, 4); + let _ = ::dst_len(&header); + } +} diff --git a/multiboot2/src/rsdp.rs b/multiboot2/src/rsdp.rs index 81b907e7..59121d0f 100644 --- a/multiboot2/src/rsdp.rs +++ b/multiboot2/src/rsdp.rs @@ -112,7 +112,9 @@ impl RsdpV1Tag { } } -impl MaybeDynSized for RsdpV1Tag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for RsdpV1Tag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); @@ -248,7 +250,9 @@ impl RsdpV2Tag { } } -impl MaybeDynSized for RsdpV2Tag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for RsdpV2Tag { type Header = TagHeader; const BASE_SIZE: usize = size_of::(); diff --git a/multiboot2/src/smbios.rs b/multiboot2/src/smbios.rs index ec85f63c..fb73dbc9 100644 --- a/multiboot2/src/smbios.rs +++ b/multiboot2/src/smbios.rs @@ -47,7 +47,9 @@ impl SmbiosTag { } } -impl MaybeDynSized for SmbiosTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for SmbiosTag { type Header = TagHeader; #[expect(clippy::manual_bits)] // false positive const BASE_SIZE: usize = size_of::() + size_of::() * 8; diff --git a/multiboot2/src/tag.rs b/multiboot2/src/tag.rs index 9f489718..4dfc9fbe 100644 --- a/multiboot2/src/tag.rs +++ b/multiboot2/src/tag.rs @@ -32,7 +32,9 @@ impl TagHeader { } } -impl Header for TagHeader { +// SAFETY: The header is a padding-free repr(C) struct of raw integers, and +// any bit pattern is valid for it. +unsafe impl Header for TagHeader { fn total_size(&self) -> usize { self.size as usize } diff --git a/multiboot2/src/vbe_info.rs b/multiboot2/src/vbe_info.rs index 678f1e6e..fefc659a 100644 --- a/multiboot2/src/vbe_info.rs +++ b/multiboot2/src/vbe_info.rs @@ -82,7 +82,9 @@ impl VBEInfoTag { } } -impl MaybeDynSized for VBEInfoTag { +// SAFETY: The tag is repr(C) with the header as first field, any bit +// pattern is valid, and `BASE_SIZE`/`dst_len` match the ABI. +unsafe impl MaybeDynSized for VBEInfoTag { type Header = TagHeader; const BASE_SIZE: usize = size_of::();