diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d848768..ea54e61d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,3 +3,10 @@ Please see: - [multiboot2/CHANGELOG.md](multiboot2/CHANGELOG.md) - [multiboot2-header/CHANGELOG.md](multiboot2-header/CHANGELOG.md) - [multiboot2-common/CHANGELOG.md](multiboot2-common/CHANGELOG.md) + +Integration tests: + +- Allowed the shared integration utility crate to compile under the host test + harness by using the harness allocator and panic implementation for tests. +- Fixed chainloader BSS initialization to zero the full segment instead of + repeatedly writing the first byte. diff --git a/integration-test/bins/multiboot2_chainloader/src/loader.rs b/integration-test/bins/multiboot2_chainloader/src/loader.rs index e543867c..71d3b910 100644 --- a/integration-test/bins/multiboot2_chainloader/src/loader.rs +++ b/integration-test/bins/multiboot2_chainloader/src/loader.rs @@ -20,11 +20,8 @@ pub fn load_module(mut modules: multiboot::information::ModuleIter) -> ! { // Check if a header is present. { - let hdr = multiboot2_header::Multiboot2Header::find_header(elf_bytes) - .unwrap() + let (hdr, _) = multiboot2_header::Multiboot2Header::find_header(elf_bytes) .expect("Should have Multiboot2 header"); - let hdr = - unsafe { multiboot2_header::Multiboot2Header::load(hdr.0.as_ptr().cast()) }.unwrap(); log::info!("Multiboot2 header:\n{hdr:#?}"); } @@ -91,9 +88,6 @@ fn map_memory(ph: ProgramHeaderEntry) { let dest_ptr = unsafe { dest_ptr.add(ph.filesz() as usize) }; // Zero .bss memory - for _ in 0..(ph.memsz() - ph.filesz()) { - unsafe { - core::ptr::write(dest_ptr, 0); - } - } + assert!(ph.memsz() >= ph.filesz()); + unsafe { core::ptr::write_bytes(dest_ptr, 0, (ph.memsz() - ph.filesz()) as usize) }; } diff --git a/integration-test/bins/util/src/allocator.rs b/integration-test/bins/util/src/allocator.rs index a398dc1c..5db01257 100644 --- a/integration-test/bins/util/src/allocator.rs +++ b/integration-test/bins/util/src/allocator.rs @@ -1,18 +1,27 @@ +#[cfg(not(test))] use good_memory_allocator::SpinLockedAllocator; +#[cfg(not(test))] #[repr(align(0x4000))] struct Align16K(T); /// 16 KiB naturally aligned backing storage for heap. +#[cfg(not(test))] static mut HEAP: Align16K<[u8; 0x4000]> = Align16K([0; 0x4000]); +#[cfg(not(test))] #[global_allocator] static ALLOCATOR: SpinLockedAllocator = SpinLockedAllocator::empty(); /// Initializes the allocator. Call only once. +#[cfg(not(test))] pub fn init() { #[expect(static_mut_refs)] unsafe { ALLOCATOR.init(HEAP.0.as_ptr().cast::() as _, HEAP.0.len()); } } + +/// Initializes the allocator. Host tests use the test harness allocator. +#[cfg(test)] +pub fn init() {} diff --git a/integration-test/bins/util/src/lib.rs b/integration-test/bins/util/src/lib.rs index 779dd244..cb387728 100644 --- a/integration-test/bins/util/src/lib.rs +++ b/integration-test/bins/util/src/lib.rs @@ -3,7 +3,9 @@ #[macro_use] extern crate alloc; +#[cfg(not(test))] use core::panic::PanicInfo; +#[cfg(not(test))] use log::error; use qemu_exit::QEMUExit; @@ -26,6 +28,7 @@ pub fn init_environment() { log::info!("Allocator initialized! {:?}", vec![1, 2, 3]); } +#[cfg(not(test))] #[panic_handler] fn panic_handler(info: &PanicInfo) -> ! { error!("PANIC! {}", info); diff --git a/multiboot2-common/CHANGELOG.md b/multiboot2-common/CHANGELOG.md index f2b5579b..b894bfd1 100644 --- a/multiboot2-common/CHANGELOG.md +++ b/multiboot2-common/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Added size details to memory validation errors. +- Fixed validation for dynamically sized structures whose reported total size + exceeds the available buffer. - Small code improvements diff --git a/multiboot2-common/src/boxed.rs b/multiboot2-common/src/boxed.rs index 6e79ba67..9f3b0595 100644 --- a/multiboot2-common/src/boxed.rs +++ b/multiboot2-common/src/boxed.rs @@ -3,7 +3,6 @@ use crate::{ALIGNMENT, Header, MaybeDynSized, increase_to_alignment}; use alloc::boxed::Box; use core::alloc::Layout; -use core::mem; use core::ops::Deref; use core::ptr; @@ -31,7 +30,7 @@ pub fn new_boxed + ?Sized>( .map(|b| b.len()) .sum::(); - let tag_size = mem::size_of::() + additional_size; + let tag_size = size_of::() + additional_size; header.set_size(tag_size); // Allocation size is multiple of alignment. @@ -43,7 +42,7 @@ pub fn new_boxed + ?Sized>( // write header { - let len = mem::size_of::(); + let len = size_of::(); let ptr = core::ptr::addr_of!(header); unsafe { ptr::copy_nonoverlapping(ptr.cast::(), heap_ptr, len); @@ -52,7 +51,7 @@ pub fn new_boxed + ?Sized>( // write body { - let mut write_offset = mem::size_of::(); + let mut write_offset = size_of::(); for &bytes in additional_bytes_slices { let len = bytes.len(); let src = bytes.as_ptr(); @@ -71,7 +70,7 @@ pub fn new_boxed + ?Sized>( // If this panic triggers, there is a fundamental flaw in my logic. This is // not the fault of an API user. assert_eq!( - mem::size_of_val(reference.deref()), + size_of_val(reference.deref()), alloc_size, "Allocation should match Rusts expectation" ); diff --git a/multiboot2-common/src/bytes_ref.rs b/multiboot2-common/src/bytes_ref.rs index 13b4e389..dd6e0c63 100644 --- a/multiboot2-common/src/bytes_ref.rs +++ b/multiboot2-common/src/bytes_ref.rs @@ -2,7 +2,6 @@ use crate::{ALIGNMENT, Header, MemoryError}; use core::marker::PhantomData; -use core::mem; use core::ops::Deref; /// Wraps a byte slice representing a Multiboot2 structure including an optional @@ -25,7 +24,7 @@ impl<'a, H: Header> TryFrom<&'a [u8]> for BytesRef<'a, H> { type Error = MemoryError; fn try_from(bytes: &'a [u8]) -> Result { - if bytes.len() < mem::size_of::() { + if bytes.len() < size_of::() { return Err(MemoryError::ShorterThanHeader); } // Doesn't work as expected: if align_of_val(&value[0]) < ALIGNMENT { diff --git a/multiboot2-common/src/iter.rs b/multiboot2-common/src/iter.rs index 75e5c78c..7b029c59 100644 --- a/multiboot2-common/src/iter.rs +++ b/multiboot2-common/src/iter.rs @@ -4,7 +4,6 @@ use crate::{ALIGNMENT, DynSizedStructure, Header, increase_to_alignment}; use core::marker::PhantomData; -use core::mem; /// Iterates over the tags (modelled by [`DynSizedStructure`]) of the underlying /// byte slice. Each tag is expected to have the same common [`Header`] with @@ -64,7 +63,7 @@ impl<'a, H: Header + 'a> Iterator for TagIter<'a, H> { // See . let slice = { let from = self.next_tag_offset; - let len = mem::size_of::() + tag_hdr.payload_len(); + let len = size_of::() + tag_hdr.payload_len(); let to = from + len; // The size of (the allocation for) a value is always a multiple of diff --git a/multiboot2-common/src/lib.rs b/multiboot2-common/src/lib.rs index 558bd827..f01a6dfa 100644 --- a/multiboot2-common/src/lib.rs +++ b/multiboot2-common/src/lib.rs @@ -256,7 +256,6 @@ pub use iter::TagIter; pub use tag::{MaybeDynSized, Tag}; use core::fmt::Debug; -use core::mem; use core::ptr; use core::ptr::NonNull; use core::slice; @@ -284,7 +283,7 @@ pub trait Header: Clone + Sized + PartialEq + Eq + Debug { /// plus [`Header::payload_len`]. #[must_use] fn total_size(&self) -> usize { - mem::size_of::() + self.payload_len() + size_of::() + self.payload_len() } /// Updates the header with the given `total_size`. @@ -334,15 +333,20 @@ impl DynSizedStructure { let ptr = bytes.as_ptr().cast::(); let hdr = unsafe { &*ptr }; - if hdr.payload_len() > bytes.len() { - return Err(MemoryError::InvalidReportedTotalSize); + let payload_len = hdr.payload_len(); + let total_size = size_of::() + payload_len; + if total_size > bytes.len() { + return Err(MemoryError::InvalidReportedTotalSize( + total_size, + bytes.len(), + )); } // At this point we know that the memory slice fulfills the base // assumptions and requirements. Now, we safety can create the fat // pointer. - let dst_size = hdr.payload_len(); + let dst_size = payload_len; // Create fat pointer for the DST. let ptr = ptr_meta::from_raw_parts(ptr.cast(), dst_size); let reference = unsafe { &*ptr }; @@ -401,8 +405,6 @@ impl DynSizedStructure { /// # Panics /// This panics if there is a size mismatch. However, this should never be /// the case if all types follow their documented requirements. - /// - /// [`size_of_val`]: mem::size_of_val pub fn cast + ?Sized>(&self) -> &T { // Thin or fat pointer, depending on type. // However, only thin ptr is needed. @@ -410,14 +412,14 @@ impl DynSizedStructure { // This should be a compile-time assertion. However, this is the best // location to place it for now. - assert!(T::BASE_SIZE >= mem::size_of::()); + assert!(T::BASE_SIZE >= size_of::()); let t_dst_size = T::dst_len(self.header()); // Creates thin or fat pointer, depending on type. let t_ptr = ptr_meta::from_raw_parts(base_ptr.cast(), t_dst_size); let t_ref = unsafe { &*t_ptr }; - assert_eq!(mem::size_of_val(self), mem::size_of_val(t_ref)); + assert_eq!(size_of_val(self), size_of_val(t_ref)); t_ref } @@ -436,6 +438,9 @@ pub enum MemoryError { /// type. #[error("memory range is shorter than the size of the header structure")] ShorterThanHeader, + /// The size is insufficient to contain at least a valid minimal structure. + #[error("memory range is shorter than the size of the header structure")] + SizeInsufficient(usize /* actual */, usize /* expected */), /// The buffer misses the terminating padding to the next alignment /// boundary. The padding is relevant to satisfy Rustc/Miri, but also the /// spec mandates that the padding is added. @@ -443,8 +448,10 @@ pub enum MemoryError { MissingPadding, /// The size-property has an illegal value that can't be fulfilled with the /// given bytes. - #[error("the header reports an invalid total size")] - InvalidReportedTotalSize, + #[error( + "header reports an invalid total size of 0x{0:x} while only 0x{1:x} bytes are available" + )] + InvalidReportedTotalSize(usize /* actual */, usize /* expected */), } /// Increases the given size to the next alignment boundary, if it is not a @@ -487,7 +494,7 @@ mod tests { impl MaybeDynSized for CustomSizedTag { type Header = DummyTestHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(_header: &DummyTestHeader) -> Self::Metadata {} } @@ -502,7 +509,7 @@ mod tests { let tag = DynSizedStructure::ref_from_slice(bytes.borrow()).unwrap(); let custom_tag = tag.cast::(); - assert_eq!(mem::size_of_val(custom_tag), 16); + assert_eq!(size_of_val(custom_tag), 16); assert_eq!(custom_tag.a, 0xdead_beef); assert_eq!(custom_tag.b, 0x1337_1337); } @@ -530,4 +537,24 @@ mod tests { assert_eq!(tag.header().typ(), 0x1337); assert_eq!(tag.header().size(), 18); } + + #[test] + fn test_ref_from_slice_rejects_oversized_header() { + #[rustfmt::skip] + let bytes = AlignedBytes::new( + [ + 0x37, 0x13, 0, 0, + /* Tag size */ + 24, 0, 0, 0, + /* Only 8 bytes payload plus padding are available. */ + 0, 1, 2, 3, + 4, 5, 6, 7, + ], + ); + + assert_eq!( + DynSizedStructure::::ref_from_slice(bytes.borrow()), + Err(MemoryError::InvalidReportedTotalSize(24, 16)) + ); + } } diff --git a/multiboot2-common/src/tag.rs b/multiboot2-common/src/tag.rs index 8ba6b19c..6e3947da 100644 --- a/multiboot2-common/src/tag.rs +++ b/multiboot2-common/src/tag.rs @@ -1,7 +1,6 @@ //! Module for the traits [`MaybeDynSized`] and [`Tag`]. use crate::{BytesRef, DynSizedStructure, Header}; -use core::mem; use core::slice; use ptr_meta::Pointee; @@ -55,7 +54,7 @@ pub trait MaybeDynSized: Pointee { /// Returns the payload, i.e., all memory that is not occupied by the /// [`Header`] of the type. fn payload(&self) -> &[u8] { - let from = mem::size_of::(); + let from = size_of::(); &self.as_bytes()[from..] } @@ -65,7 +64,7 @@ pub trait MaybeDynSized: Pointee { fn as_bytes(&self) -> BytesRef<'_, Self::Header> { let ptr = core::ptr::addr_of!(*self); // Actual tag size with optional terminating padding. - let size = mem::size_of_val(self); + let size = size_of_val(self); let slice = unsafe { slice::from_raw_parts(ptr.cast::(), size) }; // Unwrap is fine as this type can't exist without the underlying memory // guarantees. @@ -95,7 +94,7 @@ pub trait Tag: MaybeDynSized { impl MaybeDynSized for DynSizedStructure { type Header = H; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(header: &Self::Header) -> Self::Metadata { header.payload_len() diff --git a/multiboot2-common/src/test_utils.rs b/multiboot2-common/src/test_utils.rs index 1a4cfc1e..4f0f74ca 100644 --- a/multiboot2-common/src/test_utils.rs +++ b/multiboot2-common/src/test_utils.rs @@ -4,7 +4,6 @@ use crate::{Header, MaybeDynSized, Tag}; use core::borrow::Borrow; -use core::mem; use core::ops::Deref; /// Helper to 8-byte align the underlying bytes, as mandated in the Multiboot2 @@ -71,7 +70,7 @@ impl DummyTestHeader { impl Header for DummyTestHeader { fn payload_len(&self) -> usize { - self.size as usize - mem::size_of::() + self.size as usize - size_of::() } fn set_size(&mut self, total_size: usize) { @@ -87,7 +86,7 @@ pub struct DummyDstTag { } impl DummyDstTag { - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); #[must_use] pub const fn header(&self) -> &DummyTestHeader { @@ -103,7 +102,7 @@ impl DummyDstTag { impl MaybeDynSized for DummyDstTag { type Header = DummyTestHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(header: &Self::Header) -> Self::Metadata { header.size as usize - Self::BASE_SIZE @@ -117,7 +116,6 @@ impl Tag for DummyDstTag { #[cfg(test)] mod tests { - use core::mem; use core::ptr::addr_of; use crate::ALIGNMENT; @@ -126,7 +124,7 @@ mod tests { #[test] fn abi() { - assert_eq!(mem::align_of::>(), ALIGNMENT); + assert_eq!(align_of::>(), ALIGNMENT); let bytes = AlignedBytes([0]); assert_eq!(bytes.as_ptr().align_offset(8), 0); diff --git a/multiboot2-header/CHANGELOG.md b/multiboot2-header/CHANGELOG.md index dfa5dbcf..6a4b90b2 100644 --- a/multiboot2-header/CHANGELOG.md +++ b/multiboot2-header/CHANGELOG.md @@ -2,6 +2,13 @@ ## Unreleased +- Changed `Multiboot2Header::find_header` to scan the full 32 KiB search + window, validate candidate headers, and return the parsed header plus offset. +- Added validation that loaded headers end with the mandatory end tag. +- Changed checksum validation errors to include the actual and expected + checksum values. +- Fixed `EndHeaderTag::new` and `Builder::build` so generated headers contain + the mandatory end tag. - Small code improvements diff --git a/multiboot2-header/src/address.rs b/multiboot2-header/src/address.rs index 1bfe7fd8..826860e9 100644 --- a/multiboot2-header/src/address.rs +++ b/multiboot2-header/src/address.rs @@ -1,5 +1,4 @@ use crate::{HeaderTagFlag, HeaderTagHeader, HeaderTagType}; -use core::mem::size_of; use multiboot2_common::{MaybeDynSized, Tag}; /// Binary address information for non-ELF images. @@ -107,7 +106,7 @@ mod tests { #[test] fn test_assert_size() { assert_eq!( - core::mem::size_of::(), + core::size_of::(), 2 + 2 + 4 + 4 + 4 + 4 + 4 ); } diff --git a/multiboot2-header/src/builder.rs b/multiboot2-header/src/builder.rs index 1c9598d8..0037c700 100644 --- a/multiboot2-header/src/builder.rs +++ b/multiboot2-header/src/builder.rs @@ -1,9 +1,10 @@ //! Exports a builder [`Builder`]. use crate::{ - AddressHeaderTag, ConsoleHeaderTag, EfiBootServiceHeaderTag, EntryAddressHeaderTag, - EntryEfi32HeaderTag, EntryEfi64HeaderTag, FramebufferHeaderTag, HeaderTagISA, - InformationRequestHeaderTag, ModuleAlignHeaderTag, Multiboot2BasicHeader, RelocatableHeaderTag, + AddressHeaderTag, ConsoleHeaderTag, EfiBootServiceHeaderTag, EndHeaderTag, + EntryAddressHeaderTag, EntryEfi32HeaderTag, EntryEfi64HeaderTag, FramebufferHeaderTag, + HeaderTagISA, InformationRequestHeaderTag, ModuleAlignHeaderTag, Multiboot2BasicHeader, + RelocatableHeaderTag, }; use alloc::boxed::Box; use alloc::vec::Vec; @@ -155,6 +156,8 @@ impl Builder { byte_refs.push(tag.as_bytes().as_ref()); } // TODO add support for custom tags once someone requests it. + let end_tag = EndHeaderTag::new(); + byte_refs.push(end_tag.as_bytes().as_ref()); new_boxed(header, byte_refs.as_slice()) } } @@ -216,7 +219,11 @@ mod tests { unsafe { Multiboot2Header::load(structure.as_bytes().as_ref().as_ptr().cast()) } .unwrap(); - assert!(header.verify_checksum()); + assert_eq!(header.verify_checksum(), Ok(())); + assert_eq!( + header.iter().last().unwrap().header().typ(), + crate::HeaderTagType::End + ); for tag in header.iter() { dbg!(tag); diff --git a/multiboot2-header/src/console.rs b/multiboot2-header/src/console.rs index 107bda5a..df65d846 100644 --- a/multiboot2-header/src/console.rs +++ b/multiboot2-header/src/console.rs @@ -1,5 +1,4 @@ use crate::{HeaderTagFlag, HeaderTagHeader, HeaderTagType}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// Possible flags for [`ConsoleHeaderTag`]. @@ -61,7 +60,7 @@ impl ConsoleHeaderTag { impl MaybeDynSized for ConsoleHeaderTag { type Header = HeaderTagHeader; - const BASE_SIZE: usize = mem::size_of::() + mem::size_of::(); + const BASE_SIZE: usize = size_of::() + size_of::(); fn dst_len(_header: &Self::Header) -> Self::Metadata {} } diff --git a/multiboot2-header/src/end.rs b/multiboot2-header/src/end.rs index 6c118863..6e83b0d2 100644 --- a/multiboot2-header/src/end.rs +++ b/multiboot2-header/src/end.rs @@ -1,5 +1,4 @@ use crate::{HeaderTagFlag, HeaderTagHeader, HeaderTagType}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// Terminates a list of optional tags in a Multiboot2 header. @@ -20,9 +19,9 @@ impl EndHeaderTag { #[must_use] pub const fn new() -> Self { let header = HeaderTagHeader::new( - HeaderTagType::EntryAddress, + HeaderTagType::End, HeaderTagFlag::Required, - mem::size_of::() as u32, + size_of::() as u32, ); Self { header } } @@ -49,7 +48,7 @@ impl EndHeaderTag { impl MaybeDynSized for EndHeaderTag { type Header = HeaderTagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(_header: &Self::Header) -> Self::Metadata {} } @@ -65,6 +64,11 @@ mod tests { #[test] fn test_assert_size() { - assert_eq!(core::mem::size_of::(), 2 + 2 + 4); + assert_eq!(core::size_of::(), 2 + 2 + 4); + } + + #[test] + fn test_end_tag_type() { + assert_eq!(EndHeaderTag::new().typ(), crate::HeaderTagType::End); } } diff --git a/multiboot2-header/src/entry_address.rs b/multiboot2-header/src/entry_address.rs index 852d481f..a45a2c03 100644 --- a/multiboot2-header/src/entry_address.rs +++ b/multiboot2-header/src/entry_address.rs @@ -1,7 +1,6 @@ use crate::{HeaderTagFlag, HeaderTagHeader, HeaderTagType}; use core::fmt; use core::fmt::{Debug, Formatter}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// Specifies the physical address to which the boot loader should jump in @@ -61,7 +60,7 @@ impl Debug for EntryAddressHeaderTag { impl MaybeDynSized for EntryAddressHeaderTag { type Header = HeaderTagHeader; - const BASE_SIZE: usize = mem::size_of::() + mem::size_of::(); + const BASE_SIZE: usize = size_of::() + size_of::(); fn dst_len(_header: &Self::Header) -> Self::Metadata {} } diff --git a/multiboot2-header/src/entry_efi_32.rs b/multiboot2-header/src/entry_efi_32.rs index 1a40d559..66ddb8d1 100644 --- a/multiboot2-header/src/entry_efi_32.rs +++ b/multiboot2-header/src/entry_efi_32.rs @@ -1,7 +1,6 @@ use crate::{HeaderTagFlag, HeaderTagHeader, HeaderTagType}; use core::fmt; use core::fmt::{Debug, Formatter}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// Contains the entry address for EFI i386 machine state. @@ -70,7 +69,7 @@ impl Debug for EntryEfi32HeaderTag { impl MaybeDynSized for EntryEfi32HeaderTag { type Header = HeaderTagHeader; - const BASE_SIZE: usize = mem::size_of::() + mem::size_of::(); + const BASE_SIZE: usize = size_of::() + size_of::(); fn dst_len(_header: &Self::Header) -> Self::Metadata {} } diff --git a/multiboot2-header/src/entry_efi_64.rs b/multiboot2-header/src/entry_efi_64.rs index 28787254..9c70b555 100644 --- a/multiboot2-header/src/entry_efi_64.rs +++ b/multiboot2-header/src/entry_efi_64.rs @@ -1,7 +1,6 @@ use crate::{HeaderTagFlag, HeaderTagHeader, HeaderTagType}; use core::fmt; use core::fmt::{Debug, Formatter}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// Contains the entry address for EFI amd64 machine state. @@ -70,7 +69,7 @@ impl Debug for EntryEfi64HeaderTag { impl MaybeDynSized for EntryEfi64HeaderTag { type Header = HeaderTagHeader; - const BASE_SIZE: usize = mem::size_of::() + mem::size_of::(); + const BASE_SIZE: usize = size_of::() + size_of::(); fn dst_len(_header: &Self::Header) -> Self::Metadata {} } diff --git a/multiboot2-header/src/framebuffer.rs b/multiboot2-header/src/framebuffer.rs index e635fb24..6a1af645 100644 --- a/multiboot2-header/src/framebuffer.rs +++ b/multiboot2-header/src/framebuffer.rs @@ -1,5 +1,4 @@ use crate::{HeaderTagFlag, HeaderTagHeader, HeaderTagType}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// Specifies the preferred graphics mode. If this tag @@ -69,7 +68,7 @@ impl FramebufferHeaderTag { impl MaybeDynSized for FramebufferHeaderTag { type Header = HeaderTagHeader; - const BASE_SIZE: usize = mem::size_of::() + 3 * mem::size_of::(); + const BASE_SIZE: usize = size_of::() + 3 * size_of::(); fn dst_len(_header: &Self::Header) -> Self::Metadata {} } diff --git a/multiboot2-header/src/header.rs b/multiboot2-header/src/header.rs index 12260bfd..41f3871d 100644 --- a/multiboot2-header/src/header.rs +++ b/multiboot2-header/src/header.rs @@ -5,13 +5,15 @@ use crate::{ TagIter, }; use core::fmt::{Debug, Formatter}; -use core::mem::size_of; use core::ptr::NonNull; use multiboot2_common::{ALIGNMENT, DynSizedStructure, Header, MemoryError, Tag}; use thiserror::Error; /// Magic value for a [`Multiboot2Header`], as defined by the spec. pub const MAGIC: u32 = 0xe85250d6; +/// Range from the beginning of an image in that bootloaders will search for a +/// multiboot2 header. +pub const HEADER_SEARCH_LIMIT: usize = 32768; /// Wrapper type around a pointer to the Multiboot2 header. /// @@ -21,6 +23,7 @@ pub const MAGIC: u32 = 0xe85250d6; /// to parse it. If you want to construct the type by yourself, /// please look at `HeaderBuilder` (requires the `builder` feature). #[repr(transparent)] +#[derive(PartialEq, Eq)] pub struct Multiboot2Header<'a>(&'a DynSizedStructure); impl<'a> Multiboot2Header<'a> { @@ -45,60 +48,119 @@ impl<'a> Multiboot2Header<'a> { if header.header_magic != MAGIC { return Err(LoadError::MagicNotFound); } - if !header.verify_checksum() { - return Err(LoadError::ChecksumMismatch); + header + .verify_checksum() + .map_err(|x| LoadError::ChecksumMismatch(x.0, x.1))?; + if !this.has_valid_end_tag() { + return Err(LoadError::NoEndTag); } Ok(this) } - /// Find the header in a given slice. + /// Checks whether the header ends with the mandatory end tag. + fn has_valid_end_tag(&self) -> bool { + let payload = self.0.payload(); + // Check we have an end tag. + if payload.len() < size_of::() { + return false; + } + + let end_tag = &payload[payload.len() - size_of::()..]; + let typ = u16::from_le_bytes(end_tag[0..2].try_into().unwrap()); + let flags = u16::from_le_bytes(end_tag[2..4].try_into().unwrap()); + let size = u32::from_le_bytes(end_tag[4..8].try_into().unwrap()); + + typ == HeaderTagType::End as u16 + && flags == crate::HeaderTagFlag::Required as u16 + && size as usize == size_of::() + } + + /// Tries finding a Multiboot2 header in a given slice of binary data. + /// + /// Performs basic checks, such as length checks and a checksum match. /// - /// If it succeeds, it returns a tuple consisting of the subslice containing - /// just the header and the index of the header in the given slice. - /// If it fails (either because the header is not properly 64-bit aligned - /// or because it is truncated), it returns a [`LoadError`]. - /// If there is no header, it returns `None`. - pub fn find_header(buffer: &[u8]) -> Result, LoadError> { + /// The Multiboot2 header must be contained completely within the first + /// [`HEADER_SEARCH_LIMIT`] bytes of the OS image, and must be + /// [64-bit aligned](ALIGNMENT). + /// + /// On success, it returns the parsed header and an index into the original + /// buffer pointing to where the header starts. + /// + /// # Parameter + /// - `buffer`: [64-bit aligned](ALIGNMENT) buffer describing the first + /// [`HEADER_SEARCH_LIMIT`] bytes of a potential Multiboot2 kernel image. + pub fn find_header(buffer: &[u8]) -> Result<(Self, usize /* index in buffer */), LoadError> { + if buffer.len() < size_of::() { + return Err(LoadError::Memory(MemoryError::ShorterThanHeader)); + } if buffer.as_ptr().align_offset(ALIGNMENT) != 0 { return Err(LoadError::Memory(MemoryError::WrongAlignment)); } - let mut windows = buffer[0..8192].windows(4); - let magic_index = match windows.position(|vals| { - u32::from_le_bytes(vals.try_into().unwrap()) // yes, there's 4 bytes here - == MAGIC - }) { - Some(idx) => { - if idx % 8 == 0 { - idx - } else { - return Err(LoadError::Memory(MemoryError::WrongAlignment)); - } - } - None => return Ok(None), - }; - // skip over rest of magic - windows.next(); - windows.next(); - windows.next(); - // arch - windows.next(); - windows.next(); - windows.next(); - windows.next(); - let header_length: usize = u32::from_le_bytes( - windows - .next() - .ok_or(LoadError::Memory(MemoryError::MissingPadding))? - .try_into() - .unwrap(), // 4 bytes are a u32 - ) - .try_into() - .unwrap(); - Ok(Some(( - &buffer[magic_index..magic_index + header_length], - magic_index as u32, - ))) + let search_len = buffer.len().min(HEADER_SEARCH_LIMIT); + let buffer = &buffer[0..search_len]; + + let mut u32_iter = buffer + .chunks(size_of::()) + .enumerate() + .take_while(|(_, chunk)| chunk.len() == size_of::()) + // Index now points into the original byte buffer. + .map(|(idx, chunk)| { + ( + idx * size_of::(), + u32::from_le_bytes([chunk[0], chunk[1], chunk[2], chunk[3]]), + ) + }); + + // After that, `u32_iter` continues at the next index after the magic. + let (magic_begin_idx, _) = u32_iter + // The 64-bit-aligned header starts with magic, so magic must be + // aligned too. + .find(|(idx, value)| { + let is_64bit_aligned = idx % ALIGNMENT == 0; + let magic_matches = *value == MAGIC; + is_64bit_aligned && magic_matches + }) + .ok_or(LoadError::MagicNotFound)?; + + // After that, `u32_iter` continues at the next index after the size. + let (_, header_size) = u32_iter + // skip the arch field + .nth(1) + .ok_or(LoadError::Memory(MemoryError::ShorterThanHeader))?; + + let header_size = usize::try_from(header_size) + .map_err(|_| LoadError::Memory(MemoryError::ShorterThanHeader))?; + + if header_size < size_of::() { + return Err(LoadError::Memory(MemoryError::ShorterThanHeader)); + } + + let min_size = size_of::() + size_of::(); + if header_size < min_size { + return Err(LoadError::Memory(MemoryError::SizeInsufficient( + header_size, + min_size, + ))); + } + + // Check if the remaining length of the buffer contains the expected + // memory. + let remaining = buffer[magic_begin_idx..].len(); + if remaining < header_size { + return Err(LoadError::Memory(MemoryError::InvalidReportedTotalSize( + header_size, + remaining, + ))); + } + + let ptr = buffer + .as_ptr() + .wrapping_add(magic_begin_idx) + .cast::(); + + let header = unsafe { Self::load(ptr)? }; + Ok((header, magic_begin_idx)) } /// Returns a [`TagIter`]. @@ -108,8 +170,15 @@ impl<'a> Multiboot2Header<'a> { } /// Wrapper around [`Multiboot2BasicHeader::verify_checksum`]. - #[must_use] - pub const fn verify_checksum(&self) -> bool { + pub const fn verify_checksum( + &self, + ) -> Result< + (), + ( + u32, /* actual checksum */ + u32, /* expected checksum */ + ), + > { self.0.header().verify_checksum() } /// Wrapper around [`Multiboot2BasicHeader::header_magic`]. @@ -228,11 +297,14 @@ impl Debug for Multiboot2Header<'_> { #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Error)] pub enum LoadError { /// The provided checksum does not match the expected value. - #[error("checksum does not match expected value")] - ChecksumMismatch, + #[error("checksum 0x{0:X} does not match expected value 0x{1:x}")] + ChecksumMismatch(u32 /* is */, u32 /* expected */), /// The header does not contain the correct magic number. #[error("header does not contain expected magic value")] MagicNotFound, + /// Missing mandatory end tag. + #[error("missing mandatory end tag")] + NoEndTag, /// The provided memory can't be parsed as [`Multiboot2Header`]. /// See [`MemoryError`]. #[error("memory can't be parsed as multiboot2 header")] @@ -267,11 +339,22 @@ impl Multiboot2BasicHeader { } } - /// Verifies that a Multiboot2 header is valid. - #[must_use] - pub const fn verify_checksum(&self) -> bool { + /// Verifies if a Multiboot2 header is valid. + pub const fn verify_checksum( + &self, + ) -> Result< + (), + ( + u32, /* actual checksum */ + u32, /* expected checksum */ + ), + > { let check = Self::calc_checksum(self.header_magic, self.arch, self.length); - check == self.checksum + if check == self.checksum { + Ok(()) + } else { + Err((self.checksum, check)) + } } /// Calculates the checksum as described in the spec. @@ -307,6 +390,7 @@ impl Multiboot2BasicHeader { impl Header for Multiboot2BasicHeader { fn payload_len(&self) -> usize { + assert!(self.length as usize >= size_of::()); self.length as usize - size_of::() } @@ -330,10 +414,99 @@ impl Debug for Multiboot2BasicHeader { #[cfg(test)] mod tests { - use crate::Multiboot2BasicHeader; + use crate::{HeaderTagISA, LoadError, MAGIC, Multiboot2BasicHeader, Multiboot2Header}; + use core::borrow::Borrow; + use multiboot2_common::MemoryError; + use multiboot2_common::test_utils::AlignedBytes; + + /// Writes a minimal valid Multiboot2 header into the buffer, consisting + /// only of the basic header and an end tag. + fn write_minimal_valid_header_tag(buffer: &mut [u8]) { + // Aligned magic + buffer[0..4].copy_from_slice(&MAGIC.to_le_bytes()); + // Architecture + buffer[4..8].copy_from_slice(&(HeaderTagISA::I386 as u32).to_le_bytes()); + // Total size + buffer[8..12].copy_from_slice(&24_u32.to_le_bytes()); + // Checksum + buffer[12..16].copy_from_slice(&0x17adaf12_u32.to_le_bytes()); + // End tag: ID + buffer[16..18].copy_from_slice(&0_u16.to_le_bytes()); + // End tag: Flags + buffer[18..20].copy_from_slice(&0_u16.to_le_bytes()); + // End tag: Size + buffer[20..24].copy_from_slice(&8_u32.to_le_bytes()); + } #[test] fn test_assert_size() { - assert_eq!(core::mem::size_of::(), 4 + 4 + 4 + 4); + assert_eq!(core::size_of::(), 4 + 4 + 4 + 4); + } + + #[test] + fn find_header_handles_short_buffers() { + let bytes = AlignedBytes::new([0; 16]); + + assert_eq!( + Multiboot2Header::find_header(bytes.borrow()), + Err(LoadError::MagicNotFound) + ); + } + + #[test] + fn find_header_rejects_truncated_header() { + let mut bytes = AlignedBytes::new([0; 16]); + bytes.0[0..4].copy_from_slice(&MAGIC.to_le_bytes()); + bytes.0[8..12].copy_from_slice(&32_u32.to_le_bytes()); + + assert_eq!( + Multiboot2Header::find_header(bytes.borrow()), + Err(LoadError::Memory(MemoryError::InvalidReportedTotalSize( + 32, 16 + ))) + ); + } + + #[test] + fn find_header_searches_full_multiboot2_range() { + let mut bytes = AlignedBytes::new([0; 9000]); + write_minimal_valid_header_tag(&mut bytes.0[8192..]); + + let (_header, offset) = Multiboot2Header::find_header(bytes.borrow()).unwrap(); + assert_eq!(offset, 8192); + } + + #[test] + fn find_header_skips_unaligned_magic_candidates() { + let mut bytes = AlignedBytes::new([0; 40]); + // Unaligned magic + bytes.0[4..8].copy_from_slice(&MAGIC.to_le_bytes()); + write_minimal_valid_header_tag(&mut bytes.0[8..]); + + let (_header, offset) = Multiboot2Header::find_header(bytes.borrow()).unwrap(); + assert_eq!(offset, 8); + } + + #[test] + fn load_accepts_minimal_header_with_end_tag() { + let mut bytes = AlignedBytes::new([0; 24]); + write_minimal_valid_header_tag(&mut bytes.0); + + let header = unsafe { Multiboot2Header::load(bytes.as_ptr().cast()) }; + + assert!(header.is_ok()); + } + + #[test] + fn load_rejects_missing_end_tag() { + let mut bytes = AlignedBytes::new([0; 16]); + let checksum = Multiboot2BasicHeader::calc_checksum(MAGIC, HeaderTagISA::I386, 16); + bytes.0[0..4].copy_from_slice(&MAGIC.to_le_bytes()); + bytes.0[8..12].copy_from_slice(&16_u32.to_le_bytes()); + bytes.0[12..16].copy_from_slice(&checksum.to_le_bytes()); + + let header = unsafe { Multiboot2Header::load(bytes.as_ptr().cast()) }; + + assert!(matches!(header, Err(LoadError::NoEndTag))); } } diff --git a/multiboot2-header/src/information_request.rs b/multiboot2-header/src/information_request.rs index 0db71efb..2e5f9e20 100644 --- a/multiboot2-header/src/information_request.rs +++ b/multiboot2-header/src/information_request.rs @@ -2,7 +2,6 @@ use crate::{HeaderTagFlag, HeaderTagHeader}; use crate::{HeaderTagType, MbiTagTypeId}; use core::fmt; use core::fmt::{Debug, Formatter}; -use core::mem; #[cfg(feature = "builder")] use multiboot2_common::new_boxed; use multiboot2_common::{MaybeDynSized, Tag}; @@ -29,7 +28,7 @@ impl InformationRequestHeaderTag { let header = HeaderTagHeader::new(HeaderTagType::InformationRequest, flags, 0); let requests = unsafe { let ptr = ptr::addr_of!(*requests); - slice::from_raw_parts(ptr.cast::(), mem::size_of_val(requests)) + slice::from_raw_parts(ptr.cast::(), size_of_val(requests)) }; new_boxed(header, &[requests]) } @@ -73,12 +72,12 @@ impl Debug for InformationRequestHeaderTag { impl MaybeDynSized for InformationRequestHeaderTag { type Header = HeaderTagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(header: &Self::Header) -> Self::Metadata { let dst_size = header.size() as usize - Self::BASE_SIZE; - assert_eq!(dst_size % mem::size_of::(), 0); - dst_size / mem::size_of::() + assert_eq!(dst_size % size_of::(), 0); + dst_size / size_of::() } } diff --git a/multiboot2-header/src/module_align.rs b/multiboot2-header/src/module_align.rs index f15dc611..d63cb0f0 100644 --- a/multiboot2-header/src/module_align.rs +++ b/multiboot2-header/src/module_align.rs @@ -1,5 +1,4 @@ use crate::{HeaderTagFlag, HeaderTagHeader, HeaderTagType}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// If this tag is present, provided boot modules must be page aligned. @@ -13,11 +12,8 @@ impl ModuleAlignHeaderTag { /// Constructs a new tag. #[must_use] pub const fn new(flags: HeaderTagFlag) -> Self { - let header = HeaderTagHeader::new( - HeaderTagType::ModuleAlign, - flags, - mem::size_of::() as u32, - ); + let header = + HeaderTagHeader::new(HeaderTagType::ModuleAlign, flags, size_of::() as u32); Self { header } } @@ -43,7 +39,7 @@ impl ModuleAlignHeaderTag { impl MaybeDynSized for ModuleAlignHeaderTag { type Header = HeaderTagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(_header: &Self::Header) -> Self::Metadata {} } @@ -59,6 +55,6 @@ mod tests { #[test] fn test_assert_size() { - assert_eq!(core::mem::size_of::(), 2 + 2 + 4); + assert_eq!(core::size_of::(), 2 + 2 + 4); } } diff --git a/multiboot2-header/src/relocatable.rs b/multiboot2-header/src/relocatable.rs index 9575f592..3db0df6b 100644 --- a/multiboot2-header/src/relocatable.rs +++ b/multiboot2-header/src/relocatable.rs @@ -1,7 +1,6 @@ use crate::{HeaderTagFlag, HeaderTagHeader, HeaderTagType}; use core::fmt; use core::fmt::{Debug, Formatter}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// It contains load address placement suggestion for bootloader. @@ -44,11 +43,8 @@ impl RelocatableHeaderTag { align: u32, preference: RelocatableHeaderTagPreference, ) -> Self { - let header = HeaderTagHeader::new( - HeaderTagType::Relocatable, - flags, - mem::size_of::() as u32, - ); + let header = + HeaderTagHeader::new(HeaderTagType::Relocatable, flags, size_of::() as u32); Self { header, min_addr, @@ -119,7 +115,7 @@ impl Debug for RelocatableHeaderTag { impl MaybeDynSized for RelocatableHeaderTag { type Header = HeaderTagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(_header: &Self::Header) -> Self::Metadata {} } @@ -136,7 +132,7 @@ mod tests { #[test] fn test_assert_size() { assert_eq!( - core::mem::size_of::(), + core::size_of::(), 2 + 2 + 4 + 4 + 4 + 4 + 4 ); } diff --git a/multiboot2-header/src/tags.rs b/multiboot2-header/src/tags.rs index 862eea45..3006fa3b 100644 --- a/multiboot2-header/src/tags.rs +++ b/multiboot2-header/src/tags.rs @@ -2,7 +2,6 @@ //! code at the end of the official Multiboot2 spec. These tags follow in memory right after //! [`crate::Multiboot2BasicHeader`]. -use core::mem; use multiboot2_common::Header; /// ISA/ARCH in Multiboot2 header. @@ -109,7 +108,7 @@ impl HeaderTagHeader { impl Header for HeaderTagHeader { fn payload_len(&self) -> usize { - self.size as usize - mem::size_of::() + self.size as usize - size_of::() } fn set_size(&mut self, total_size: usize) { @@ -123,6 +122,6 @@ mod tests { #[test] fn test_assert_size() { - assert_eq!(core::mem::size_of::(), 2 + 2 + 4); + assert_eq!(size_of::(), 2 + 2 + 4); } } diff --git a/multiboot2-header/src/uefi_bs.rs b/multiboot2-header/src/uefi_bs.rs index dbbf63b5..c69b9f10 100644 --- a/multiboot2-header/src/uefi_bs.rs +++ b/multiboot2-header/src/uefi_bs.rs @@ -1,5 +1,4 @@ use crate::{HeaderTagFlag, HeaderTagHeader, HeaderTagType}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// This tag indicates that payload supports starting without terminating UEFI boot services. @@ -14,8 +13,7 @@ impl EfiBootServiceHeaderTag { /// Constructs a new tag. #[must_use] pub const fn new(flags: HeaderTagFlag) -> Self { - let header = - HeaderTagHeader::new(HeaderTagType::EfiBS, flags, mem::size_of::() as u32); + let header = HeaderTagHeader::new(HeaderTagType::EfiBS, flags, size_of::() as u32); Self { header } } @@ -41,7 +39,7 @@ impl EfiBootServiceHeaderTag { impl MaybeDynSized for EfiBootServiceHeaderTag { type Header = HeaderTagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(_header: &Self::Header) -> Self::Metadata {} } @@ -57,6 +55,6 @@ mod tests { #[test] fn test_assert_size() { - assert_eq!(core::mem::size_of::(), 2 + 2 + 4); + assert_eq!(size_of::(), 2 + 2 + 4); } } diff --git a/multiboot2/CHANGELOG.md b/multiboot2/CHANGELOG.md index be2f5edb..7235b0aa 100644 --- a/multiboot2/CHANGELOG.md +++ b/multiboot2/CHANGELOG.md @@ -11,6 +11,11 @@ - Added `ElfSectionsTag::string_table()`. - Added some flags to `ElfSectionFlags`. - Added UserDefined section to `ElfSectionType`. +- Added equality implementations for `BootInformation`. +- Fixed indexed framebuffer parsing to reject palette metadata that exceeds + the tag payload. +- Fixed EFI memory map parsing to reject descriptor sizes that cannot safely + describe EFI memory descriptors. - Fixed some bugs. ## v0.24.1 (2025-11-21) diff --git a/multiboot2/src/apm.rs b/multiboot2/src/apm.rs index 4f6a74fc..bcc73dcc 100644 --- a/multiboot2/src/apm.rs +++ b/multiboot2/src/apm.rs @@ -1,7 +1,6 @@ //! Module for [`ApmTag`]. use crate::{TagHeader, TagType}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// The Advanced Power Management (APM) tag. @@ -36,7 +35,7 @@ impl ApmTag { dseg_len: u16, ) -> Self { Self { - header: TagHeader::new(Self::ID, mem::size_of::() as u32), + header: TagHeader::new(Self::ID, size_of::() as u32), version, cseg, offset, @@ -114,7 +113,7 @@ impl ApmTag { impl MaybeDynSized for ApmTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(_: &TagHeader) {} } diff --git a/multiboot2/src/boot_information.rs b/multiboot2/src/boot_information.rs index 8771fffe..f059ef8c 100644 --- a/multiboot2/src/boot_information.rs +++ b/multiboot2/src/boot_information.rs @@ -10,7 +10,6 @@ use crate::{ TagIter, TagType, VBEInfoTag, module, }; use core::fmt; -use core::mem; use core::ptr::NonNull; use multiboot2_common::{DynSizedStructure, Header, MaybeDynSized, MemoryError, Tag}; use thiserror::Error; @@ -56,7 +55,8 @@ impl BootInformationHeader { impl Header for BootInformationHeader { fn payload_len(&self) -> usize { - self.total_size as usize - mem::size_of::() + assert!(self.total_size as usize >= size_of::()); + self.total_size as usize - size_of::() } fn set_size(&mut self, total_size: usize) { @@ -66,6 +66,7 @@ impl Header for BootInformationHeader { /// A Multiboot 2 Boot Information (MBI) accessor. #[repr(transparent)] +#[derive(PartialEq, Eq)] pub struct BootInformation<'a>(&'a DynSizedStructure); impl<'a> BootInformation<'a> { @@ -107,17 +108,22 @@ impl<'a> BootInformation<'a> { /// bytes. fn has_valid_end_tag(&self) -> bool { let header = self.0.header(); + // Check we have an end tag + if header.payload_len() < size_of::() { + return false; + } + let end_tag_ptr = unsafe { self.0 .payload() .as_ptr() .add(header.payload_len()) - .sub(mem::size_of::()) + .sub(size_of::()) .cast::() }; let end_tag = unsafe { &*end_tag_ptr }; - end_tag.typ == EndTag::ID && end_tag.size as usize == mem::size_of::() + end_tag.typ == EndTag::ID && end_tag.size as usize == size_of::() } /// Get the start address of the boot info. @@ -370,7 +376,7 @@ impl<'a> BootInformation<'a> { /// // Give the library hints how big this tag is. /// impl MaybeDynSized for CustomTag { /// type Header = TagHeader; - /// const BASE_SIZE: usize = mem::size_of::() + mem::size_of::(); + /// const BASE_SIZE: usize = size_of::() + size_of::(); /// /// // This differs for DSTs and normal structs. See function /// // documentation. diff --git a/multiboot2/src/boot_loader_name.rs b/multiboot2/src/boot_loader_name.rs index 636bd97d..a4601490 100644 --- a/multiboot2/src/boot_loader_name.rs +++ b/multiboot2/src/boot_loader_name.rs @@ -3,7 +3,6 @@ use crate::tag::TagHeader; use crate::{StringError, TagType, parse_slice_as_string}; use core::fmt::{Debug, Formatter}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; #[cfg(feature = "builder")] use {alloc::boxed::Box, multiboot2_common::new_boxed}; @@ -78,7 +77,7 @@ impl Debug for BootLoaderNameTag { impl MaybeDynSized for BootLoaderNameTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(header: &TagHeader) -> usize { assert!(header.size as usize >= Self::BASE_SIZE); diff --git a/multiboot2/src/bootdev.rs b/multiboot2/src/bootdev.rs index 6ed43bfc..75a705a6 100644 --- a/multiboot2/src/bootdev.rs +++ b/multiboot2/src/bootdev.rs @@ -1,7 +1,6 @@ //! Module for [`BootdevTag`]. use crate::{TagHeader, TagType}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// The end tag ends the information struct. @@ -19,7 +18,7 @@ impl BootdevTag { #[must_use] pub fn new(biosdev: u32, slice: u32, part: u32) -> Self { Self { - header: TagHeader::new(Self::ID, mem::size_of::() as u32), + header: TagHeader::new(Self::ID, size_of::() as u32), biosdev, slice, part, @@ -55,7 +54,7 @@ impl BootdevTag { impl MaybeDynSized for BootdevTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(_: &TagHeader) {} } diff --git a/multiboot2/src/command_line.rs b/multiboot2/src/command_line.rs index 405cfb25..5aa15659 100644 --- a/multiboot2/src/command_line.rs +++ b/multiboot2/src/command_line.rs @@ -3,7 +3,6 @@ use crate::tag::TagHeader; use crate::{StringError, TagType, parse_slice_as_string}; use core::fmt::{Debug, Formatter}; -use core::mem; use core::str; use multiboot2_common::{MaybeDynSized, Tag}; #[cfg(feature = "builder")] @@ -72,7 +71,7 @@ impl Debug for CommandLineTag { impl MaybeDynSized for CommandLineTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(header: &TagHeader) -> usize { assert!(header.size as usize >= Self::BASE_SIZE); diff --git a/multiboot2/src/efi.rs b/multiboot2/src/efi.rs index a3a14093..5320534d 100644 --- a/multiboot2/src/efi.rs +++ b/multiboot2/src/efi.rs @@ -8,7 +8,6 @@ use crate::TagType; use crate::tag::TagHeader; -use core::mem::size_of; use multiboot2_common::{MaybeDynSized, Tag}; /// EFI system table in 32 bit mode tag. diff --git a/multiboot2/src/elf_sections.rs b/multiboot2/src/elf_sections.rs index 8ce5a62b..414e1dfe 100644 --- a/multiboot2/src/elf_sections.rs +++ b/multiboot2/src/elf_sections.rs @@ -3,7 +3,6 @@ use crate::{TagHeader, TagType}; use core::ffi::{CStr, FromBytesUntilNulError}; use core::fmt::{Debug, Formatter}; -use core::mem; use elf::endian::NativeEndian; use elf::section::{SectionHeader, SectionHeaderTable}; use multiboot2_common::{MaybeDynSized, Tag}; @@ -115,7 +114,7 @@ impl<'a> From<&'a ElfSectionsTag> for SectionHeaderTable<'a, NativeEndian> { impl MaybeDynSized for ElfSectionsTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::() + 3 * mem::size_of::(); + const BASE_SIZE: usize = size_of::() + 3 * size_of::(); fn dst_len(header: &TagHeader) -> usize { assert!(header.size as usize >= Self::BASE_SIZE); diff --git a/multiboot2/src/end.rs b/multiboot2/src/end.rs index c2d69b73..e4bdd5cb 100644 --- a/multiboot2/src/end.rs +++ b/multiboot2/src/end.rs @@ -1,7 +1,6 @@ //! Module for [`EndTag`]. use crate::{TagHeader, TagType}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// The end tag ends the information struct. @@ -14,7 +13,7 @@ pub struct EndTag { impl Default for EndTag { fn default() -> Self { Self { - header: TagHeader::new(Self::ID, mem::size_of::() as u32), + header: TagHeader::new(Self::ID, size_of::() as u32), } } } @@ -22,7 +21,7 @@ impl Default for EndTag { impl MaybeDynSized for EndTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(_: &TagHeader) {} } @@ -36,12 +35,13 @@ impl Tag for EndTag { #[cfg(test)] mod tests { use super::*; + use core::mem::transmute; #[test] /// Compile time test for [`EndTag`]. fn test_end_tag_size() { unsafe { - core::mem::transmute::<[u8; 8], EndTag>([0u8; 8]); + transmute::<[u8; 8], EndTag>([0u8; 8]); } } } diff --git a/multiboot2/src/framebuffer.rs b/multiboot2/src/framebuffer.rs index 7e745b27..4d5eca46 100644 --- a/multiboot2/src/framebuffer.rs +++ b/multiboot2/src/framebuffer.rs @@ -3,7 +3,6 @@ use crate::TagType; use crate::tag::TagHeader; use core::fmt::Debug; -use core::mem; use core::slice; use multiboot2_common::{MaybeDynSized, Tag}; use thiserror::Error; @@ -182,7 +181,12 @@ impl FramebufferTag { let palette = { // Ensure the slice can be created without causing UB - assert_eq!(mem::size_of::(), 3); + assert_eq!(size_of::(), 3); + let palette_len = num_colors as usize * size_of::(); + assert!( + self.buffer.len() - reader.off >= palette_len, + "indexed framebuffer palette must fit in the tag" + ); unsafe { slice::from_raw_parts( @@ -223,11 +227,11 @@ impl FramebufferTag { impl MaybeDynSized for FramebufferTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::() - + mem::size_of::() - + 3 * mem::size_of::() - + 2 * mem::size_of::() - + mem::size_of::(); + const BASE_SIZE: usize = size_of::() + + size_of::() + + 3 * size_of::() + + 2 * size_of::() + + size_of::(); fn dst_len(header: &TagHeader) -> usize { assert!(header.size as usize >= Self::BASE_SIZE); @@ -408,11 +412,14 @@ pub struct UnknownFramebufferType(u8); #[cfg(test)] mod tests { use super::*; + use crate::GenericInfoTag; + use core::borrow::Borrow; + use multiboot2_common::test_utils::AlignedBytes; // Compile time test #[test] fn test_size() { - assert_eq!(mem::size_of::(), 3) + assert_eq!(size_of::(), 3) } #[test] @@ -470,4 +477,35 @@ mod tests { // Good test for Miri dbg!(tag); } + + #[test] + #[should_panic(expected = "indexed framebuffer palette must fit in the tag")] + fn indexed_palette_must_fit_in_tag() { + #[rustfmt::skip] + let bytes = AlignedBytes::new([ + /* typ = framebuffer */ + 8, 0, 0, 0, + /* size = base size + num_colors + one palette entry */ + 37, 0, 0, 0, + /* address */ + 0, 0, 0, 0, 0, 0, 0, 0, + /* pitch, width, height */ + 0, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0, + /* bpp, type = indexed, padding */ + 0, 0, 0, 0, + /* num_colors = 2 */ + 2, 0, + /* only one 3-byte palette entry follows */ + 1, 2, 3, + /* padding */ + 0, 0, 0, + ]); + let tag = GenericInfoTag::ref_from_slice(bytes.borrow()) + .unwrap() + .cast::(); + + let _ = tag.buffer_type(); + } } diff --git a/multiboot2/src/image_load_addr.rs b/multiboot2/src/image_load_addr.rs index 560671a1..5f464428 100644 --- a/multiboot2/src/image_load_addr.rs +++ b/multiboot2/src/image_load_addr.rs @@ -2,8 +2,6 @@ use crate::TagType; use crate::tag::TagHeader; -#[cfg(feature = "builder")] -use core::mem::size_of; use multiboot2_common::{MaybeDynSized, Tag}; /// The physical load address tag. Typically, this is only available if the diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index 1c83e0e6..bde325c7 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -547,7 +547,7 @@ mod tests { fn vbe_info_tag_size() { unsafe { // 16 for the start + 512 from `VBEControlInfo` + 256 from `VBEModeInfo`. - core::mem::transmute::<[u8; 784], VBEInfoTag>([0u8; 784]); + transmute::<[u8; 784], VBEInfoTag>([0u8; 784]); } } @@ -1195,7 +1195,7 @@ mod tests { impl MaybeDynSized for CustomTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(_: &TagHeader) -> Self::Metadata {} } @@ -1273,7 +1273,7 @@ mod tests { impl MaybeDynSized for CustomTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::() + mem::size_of::(); + const BASE_SIZE: usize = size_of::() + size_of::(); fn dst_len(header: &TagHeader) -> usize { // The size of the sized portion of the command line tag. diff --git a/multiboot2/src/memory_map.rs b/multiboot2/src/memory_map.rs index 0aca78c7..a407c7e2 100644 --- a/multiboot2/src/memory_map.rs +++ b/multiboot2/src/memory_map.rs @@ -9,7 +9,6 @@ use crate::tag::TagHeader; use crate::{TagType, TagTypeId}; use core::fmt::{Debug, Formatter}; use core::marker::PhantomData; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; #[cfg(feature = "builder")] use {alloc::boxed::Box, core::slice, multiboot2_common::new_boxed}; @@ -39,11 +38,11 @@ impl MemoryMapTag { #[must_use] pub fn new(areas: &[MemoryArea]) -> Box { let header = TagHeader::new(Self::ID, 0); - let entry_size = (mem::size_of::() as u32).to_ne_bytes(); + let entry_size = (size_of::() as u32).to_ne_bytes(); let entry_version = 0_u32.to_ne_bytes(); let areas = { let ptr = areas.as_ptr().cast::(); - let len = mem::size_of_val(areas); + let len = size_of_val(areas); unsafe { slice::from_raw_parts(ptr, len) } }; new_boxed(header, &[&entry_size, &entry_version, areas]) @@ -68,7 +67,7 @@ impl MemoryMapTag { #[must_use] pub fn memory_areas(&self) -> &[MemoryArea] { // If this ever fails, we need to model this differently in this crate. - assert_eq!(self.entry_size as usize, mem::size_of::()); + assert_eq!(self.entry_size as usize, size_of::()); &self.areas } } @@ -76,13 +75,13 @@ impl MemoryMapTag { impl MaybeDynSized for MemoryMapTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::() + 2 * mem::size_of::(); + const BASE_SIZE: usize = size_of::() + 2 * size_of::(); fn dst_len(header: &TagHeader) -> usize { assert!(header.size as usize >= Self::BASE_SIZE); let size = header.size as usize - Self::BASE_SIZE; - assert_eq!(size % mem::size_of::(), 0); - size / mem::size_of::() + assert_eq!(size % size_of::(), 0); + size / size_of::() } } @@ -271,7 +270,7 @@ impl BasicMemoryInfoTag { #[must_use] pub fn new(memory_lower: u32, memory_upper: u32) -> Self { Self { - header: TagHeader::new(Self::ID, mem::size_of::().try_into().unwrap()), + header: TagHeader::new(Self::ID, size_of::().try_into().unwrap()), memory_lower, memory_upper, } @@ -293,7 +292,7 @@ impl BasicMemoryInfoTag { impl MaybeDynSized for BasicMemoryInfoTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(_: &TagHeader) {} } @@ -336,12 +335,12 @@ impl EFIMemoryMapTag { pub fn new_from_descs(descs: &[EFIMemoryDesc]) -> Box { let efi_mmap = { let ptr = descs.as_ptr().cast::(); - let len = mem::size_of_val(descs); + let len = size_of_val(descs); unsafe { slice::from_raw_parts(ptr, len) } }; Self::new_from_map( - mem::size_of::() as u32, + size_of::() as u32, EFIMemoryDesc::VERSION, efi_mmap, ) @@ -367,10 +366,21 @@ impl EFIMemoryMapTag { // If this ever fails, this needs to be refactored in a joint-effort // with the uefi-rs project to have all corresponding typings. assert_eq!(self.desc_version, EFIMemoryDesc::VERSION); + let desc_size = self.desc_size as usize; + assert_ne!(desc_size, 0, "EFI descriptor size must not be zero"); + assert!( + desc_size >= size_of::(), + "EFI descriptor size must cover an EFI memory descriptor" + ); + assert_eq!( + desc_size % align_of::(), + 0, + "EFI descriptor size must preserve EFI memory descriptor alignment" + ); assert_eq!( self.memory_map .as_ptr() - .align_offset(mem::align_of::()), + .align_offset(align_of::()), 0 ); @@ -395,7 +405,7 @@ impl Debug for EFIMemoryMapTag { impl MaybeDynSized for EFIMemoryMapTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::() + 3 * mem::size_of::(); + const BASE_SIZE: usize = size_of::() + 3 * size_of::(); fn dst_len(header: &TagHeader) -> usize { assert!(header.size as usize >= Self::BASE_SIZE); @@ -479,7 +489,7 @@ impl Debug for EFIMemoryAreaIter<'_> { #[cfg(all(test, feature = "builder"))] mod tests { use super::*; - use std::mem::size_of; + use std::size_of; #[test] fn test_create_old_mmap() { @@ -519,6 +529,25 @@ mod tests { assert_eq!(iter.next(), None); } + #[test] + #[should_panic(expected = "EFI descriptor size must cover an EFI memory descriptor")] + fn efi_rejects_too_small_desc_size() { + let map = [0; size_of::()]; + let tag = EFIMemoryMapTag::new_from_map(1, EFIMemoryDesc::VERSION, &map); + + let _ = tag.memory_areas(); + } + + #[test] + #[should_panic(expected = "EFI descriptor size must preserve EFI memory descriptor alignment")] + fn efi_rejects_misaligned_desc_size() { + let desc_size = size_of::() + 1; + let map = alloc::vec![0; desc_size]; + let tag = EFIMemoryMapTag::new_from_map(desc_size as u32, EFIMemoryDesc::VERSION, &map); + + let _ = tag.memory_areas(); + } + /// Tests the EFI memory map parsing using a real world efi memory map. /// This is taken from the uefi-rs repository. See /// for more info. diff --git a/multiboot2/src/module.rs b/multiboot2/src/module.rs index d87ab6f9..84b50bc7 100644 --- a/multiboot2/src/module.rs +++ b/multiboot2/src/module.rs @@ -3,7 +3,6 @@ use crate::tag::TagHeader; use crate::{StringError, TagIter, TagType, parse_slice_as_string}; use core::fmt::{Debug, Formatter}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; #[cfg(feature = "builder")] use {alloc::boxed::Box, multiboot2_common::new_boxed}; @@ -75,7 +74,7 @@ impl ModuleTag { impl MaybeDynSized for ModuleTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::() + 2 * mem::size_of::(); + const BASE_SIZE: usize = size_of::() + 2 * size_of::(); fn dst_len(header: &TagHeader) -> usize { assert!(header.size as usize >= Self::BASE_SIZE); diff --git a/multiboot2/src/network.rs b/multiboot2/src/network.rs index 1cea90b7..eb72bddd 100644 --- a/multiboot2/src/network.rs +++ b/multiboot2/src/network.rs @@ -1,7 +1,6 @@ //! Module for [`NetworkTag`]. use crate::{TagHeader, TagType, TagTypeId}; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; use ptr_meta::Pointee; #[cfg(feature = "builder")] @@ -29,7 +28,7 @@ impl NetworkTag { impl MaybeDynSized for NetworkTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(header: &TagHeader) -> usize { header.size as usize - Self::BASE_SIZE diff --git a/multiboot2/src/rsdp.rs b/multiboot2/src/rsdp.rs index 9414f433..2542bc51 100644 --- a/multiboot2/src/rsdp.rs +++ b/multiboot2/src/rsdp.rs @@ -14,8 +14,6 @@ use crate::TagType; use crate::tag::TagHeader; -#[cfg(feature = "builder")] -use core::mem::size_of; use core::slice; use core::str; use core::str::Utf8Error; diff --git a/multiboot2/src/smbios.rs b/multiboot2/src/smbios.rs index 0eef33d8..c2f9f515 100644 --- a/multiboot2/src/smbios.rs +++ b/multiboot2/src/smbios.rs @@ -3,7 +3,6 @@ use crate::TagType; use crate::tag::TagHeader; use core::fmt::Debug; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; #[cfg(feature = "builder")] use {alloc::boxed::Box, multiboot2_common::new_boxed}; @@ -51,7 +50,7 @@ impl SmbiosTag { impl MaybeDynSized for SmbiosTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::() + mem::size_of::() * 8; + const BASE_SIZE: usize = size_of::() + size_of::() * 8; fn dst_len(header: &TagHeader) -> usize { assert!(header.size as usize >= Self::BASE_SIZE); diff --git a/multiboot2/src/tag.rs b/multiboot2/src/tag.rs index ebc11e14..239b8564 100644 --- a/multiboot2/src/tag.rs +++ b/multiboot2/src/tag.rs @@ -2,7 +2,6 @@ use crate::TagTypeId; use core::fmt::Debug; -use core::mem; use multiboot2_common::Header; /// The common header that all tags have in common. This type is ABI compatible. @@ -35,8 +34,8 @@ impl TagHeader { impl Header for TagHeader { fn payload_len(&self) -> usize { - assert!(self.size as usize >= mem::size_of::()); - self.size as usize - mem::size_of::() + assert!(self.size as usize >= size_of::()); + self.size as usize - size_of::() } fn set_size(&mut self, total_size: usize) { diff --git a/multiboot2/src/tag_type.rs b/multiboot2/src/tag_type.rs index bb16aabb..ad606ada 100644 --- a/multiboot2/src/tag_type.rs +++ b/multiboot2/src/tag_type.rs @@ -149,11 +149,12 @@ impl TagType { /// and [´TagType´]. mod primitive_conversion_impls { use super::*; + use core::mem::transmute; impl From for TagTypeId { fn from(value: u32) -> Self { // SAFETY: the type has repr(transparent) - unsafe { core::mem::transmute(value) } + unsafe { transmute(value) } } } diff --git a/multiboot2/src/vbe_info.rs b/multiboot2/src/vbe_info.rs index 79b841c2..1677382c 100644 --- a/multiboot2/src/vbe_info.rs +++ b/multiboot2/src/vbe_info.rs @@ -2,7 +2,6 @@ use crate::{TagHeader, TagType}; use core::fmt; -use core::mem; use multiboot2_common::{MaybeDynSized, Tag}; /// This tag contains VBE metadata, VBE controller information returned by the @@ -31,7 +30,7 @@ impl VBEInfoTag { mode_info: VBEModeInfo, ) -> Self { Self { - header: TagHeader::new(Self::ID, mem::size_of::().try_into().unwrap()), + header: TagHeader::new(Self::ID, size_of::().try_into().unwrap()), mode, interface_segment, interface_offset, @@ -86,7 +85,7 @@ impl VBEInfoTag { impl MaybeDynSized for VBEInfoTag { type Header = TagHeader; - const BASE_SIZE: usize = mem::size_of::(); + const BASE_SIZE: usize = size_of::(); fn dst_len(_: &TagHeader) {} }