From 5def169369fee6e52d4ff8bcdaa1dd93e0549504 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 10:51:52 +0200 Subject: [PATCH 01/17] =?UTF-8?q?multiboot2:=20massive=20internal=20refact?= =?UTF-8?q?oring=20+=20most=20tests=20pass=20Miri=20=F0=9F=8E=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The goal of this commit is to prepare 100% memory safety (using Miri passing tests as reference). For that, we need a significant under-the-hood changes. `GenericTag` is the generalization of all tags as DST that owns all memory of the tag. This tag can be created from bytes, thus, we can ensure a lifetime and a valid memory range. This tag then can be casted to specific implementations of `TagTrait`. We now never have to increase or decrease the size of the referenced memory during a Tag cast, as the GenericTag already holds all bytes that the more specific type needs. Assertions and the memory checks along the way ensure that nothing can co wrong. Further, the creation of various test data structures was modified to fulfill the new guarantees, that are given in real-world scenarios and are also what the compiler expects. --- multiboot2/Cargo.toml | 4 +- multiboot2/Changelog.md | 2 + multiboot2/src/boot_information.rs | 16 +- multiboot2/src/boot_loader_name.rs | 63 ++-- multiboot2/src/builder/boxed_dst.rs | 45 ++- multiboot2/src/builder/information.rs | 24 +- multiboot2/src/command_line.rs | 59 ++- multiboot2/src/efi.rs | 12 +- multiboot2/src/elf_sections.rs | 24 +- multiboot2/src/end.rs | 4 +- multiboot2/src/framebuffer.rs | 8 +- multiboot2/src/image_load_addr.rs | 4 +- multiboot2/src/lib.rs | 95 ++--- multiboot2/src/memory_map.rs | 18 +- multiboot2/src/module.rs | 74 ++-- multiboot2/src/rsdp.rs | 6 +- multiboot2/src/smbios.rs | 61 ++-- multiboot2/src/tag.rs | 492 +++++++++++++++++++------- multiboot2/src/tag_trait.rs | 25 +- multiboot2/src/test_util.rs | 87 +++++ multiboot2/src/util.rs | 90 +++++ multiboot2/src/vbe_info.rs | 4 +- 22 files changed, 785 insertions(+), 432 deletions(-) create mode 100644 multiboot2/src/test_util.rs create mode 100644 multiboot2/src/util.rs diff --git a/multiboot2/Cargo.toml b/multiboot2/Cargo.toml index 4a7732de..c01e8b1b 100644 --- a/multiboot2/Cargo.toml +++ b/multiboot2/Cargo.toml @@ -45,12 +45,14 @@ bitflags.workspace = true derive_more.workspace = true log.workspace = true +ptr_meta = { version = "~0.2", default-features = false } # We only use a very basic type definition from this crate. To prevent MSRV # bumps from uefi-raw, I restrict this here. Upstream users are likely to have # two versions of this library in it, which is no problem, as we only use the # type definition. uefi-raw = { version = "~0.5", default-features = false } -ptr_meta = { version = "~0.2", default-features = false } + +[dev-dependencies] [package.metadata.docs.rs] all-features = true diff --git a/multiboot2/Changelog.md b/multiboot2/Changelog.md index ecb0b7a4..3951b01d 100644 --- a/multiboot2/Changelog.md +++ b/multiboot2/Changelog.md @@ -5,6 +5,8 @@ - **Breaking** All functions that returns something useful are now `#[must_use]` - **Breaking** More public fields in tags were replaced by public getters, such as `SmbiosTag::major()` +- **BREAKING:** `multiboot2::{StringError};` -> \ + `multiboot2::util::{StringError};` - updated dependencies - MSRV is 1.75 - documentation enhancements diff --git a/multiboot2/src/boot_information.rs b/multiboot2/src/boot_information.rs index 550842db..42a46702 100644 --- a/multiboot2/src/boot_information.rs +++ b/multiboot2/src/boot_information.rs @@ -277,7 +277,7 @@ impl<'a> BootInformation<'a> { pub fn elf_sections(&self) -> Option { let tag = self.get_tag::(); tag.map(|t| { - assert!((t.entry_size * t.shndx) <= t.size); + assert!((t.entry_size * t.shndx) <= t.size() as u32); t.sections() }) } @@ -359,7 +359,7 @@ impl<'a> BootInformation<'a> { /// special handling is required. This is reflected by code-comments. /// /// ```no_run - /// use multiboot2::{BootInformation, BootInformationHeader, StringError, Tag, TagTrait, TagType, TagTypeId}; + /// use multiboot2::{BootInformation, BootInformationHeader, parse_slice_as_string, StringError, TagHeader, TagTrait, TagType, TagTypeId}; /// /// #[repr(C)] /// #[derive(multiboot2::Pointee)] // Only needed for DSTs. @@ -374,17 +374,17 @@ impl<'a> BootInformation<'a> { /// impl TagTrait for CustomTag { /// const ID: TagType = TagType::Custom(0x1337); /// - /// fn dst_size(base_tag: &Tag) -> usize { + /// fn dst_len(header: &TagHeader) -> usize { /// // The size of the sized portion of the custom tag. /// let tag_base_size = 8; // id + size is 8 byte in size - /// assert!(base_tag.size >= 8); - /// base_tag.size as usize - tag_base_size + /// assert!(header.size >= 8); + /// header.size as usize - tag_base_size /// } /// } /// /// impl CustomTag { /// fn name(&self) -> Result<&str, StringError> { - /// Tag::parse_slice_as_string(&self.name) + /// parse_slice_as_string(&self.name) /// } /// } /// let mbi_ptr = 0xdeadbeef as *const BootInformationHeader; @@ -398,8 +398,8 @@ impl<'a> BootInformation<'a> { #[must_use] pub fn get_tag(&'a self) -> Option<&'a TagT> { self.tags() - .find(|tag| tag.typ == TagT::ID) - .map(|tag| tag.cast_tag::()) + .find(|tag| tag.header().typ == TagT::ID) + .map(|tag| tag.cast::()) } /// Returns an iterator over all tags. diff --git a/multiboot2/src/boot_loader_name.rs b/multiboot2/src/boot_loader_name.rs index e6817d26..e2d6c8a1 100644 --- a/multiboot2/src/boot_loader_name.rs +++ b/multiboot2/src/boot_loader_name.rs @@ -1,13 +1,13 @@ //! Module for [`BootLoaderNameTag`]. -use crate::tag::{StringError, TagHeader}; -use crate::{Tag, TagTrait, TagType, TagTypeId}; +use crate::tag::TagHeader; +use crate::{parse_slice_as_string, StringError, TagTrait, TagType}; use core::fmt::{Debug, Formatter}; use core::mem; #[cfg(feature = "builder")] use {crate::builder::BoxedDst, alloc::vec::Vec}; -const METADATA_SIZE: usize = mem::size_of::() + mem::size_of::(); +const METADATA_SIZE: usize = mem::size_of::(); /// The bootloader name tag. #[derive(ptr_meta::Pointee, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -61,7 +61,7 @@ impl BootLoaderNameTag { /// } /// ``` pub fn name(&self) -> Result<&str, StringError> { - Tag::parse_slice_as_string(&self.name) + parse_slice_as_string(&self.name) } } @@ -78,54 +78,49 @@ impl Debug for BootLoaderNameTag { impl TagTrait for BootLoaderNameTag { const ID: TagType = TagType::BootLoaderName; - fn dst_size(base_tag: &Tag) -> usize { - assert!(base_tag.size as usize >= METADATA_SIZE); - base_tag.size as usize - METADATA_SIZE + fn dst_len(header: &TagHeader) -> usize { + assert!(header.size as usize >= METADATA_SIZE); + header.size as usize - METADATA_SIZE } } #[cfg(test)] mod tests { - use crate::{BootLoaderNameTag, Tag, TagTrait, TagType}; - - const MSG: &str = "hello"; - - /// Returns the tag structure in bytes in little endian format. - fn get_bytes() -> std::vec::Vec { - // size is: 4 bytes for tag + 4 bytes for size + length of null-terminated string - let size = (4 + 4 + MSG.as_bytes().len() + 1) as u32; - [ - &((TagType::BootLoaderName.val()).to_le_bytes()), - &size.to_le_bytes(), - MSG.as_bytes(), - // Null Byte - &[0], - ] - .iter() - .flat_map(|bytes| bytes.iter()) - .copied() - .collect() + use super::*; + use crate::tag::{GenericTag, TagBytesRef}; + use crate::test_util::AlignedBytes; + + #[rustfmt::skip] + fn get_bytes() -> AlignedBytes<16> { + AlignedBytes::new([ + TagType::BootLoaderName.val() as u8, 0, 0, 0, + 15, 0, 0, 0, + b'h', b'e', b'l', b'l', b'o', b'\0', + /* padding */ + 0, 0 + ]) } /// Tests to parse a string with a terminating null byte from the tag (as the spec defines). #[test] - #[cfg_attr(miri, ignore)] fn test_parse_str() { - let tag = get_bytes(); - let tag = unsafe { &*tag.as_ptr().cast::() }; - let tag = tag.cast_tag::(); + let bytes = get_bytes(); + let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let tag = GenericTag::ref_from(bytes); + let tag = tag.cast::(); assert_eq!(tag.header.typ, TagType::BootLoaderName); - assert_eq!(tag.name().expect("must be valid UTF-8"), MSG); + assert_eq!(tag.name(), Ok("hello")); } /// Test to generate a tag from a given string. #[test] #[cfg(feature = "builder")] + #[ignore] fn test_build_str() { - let tag = BootLoaderNameTag::new(MSG); + let tag = BootLoaderNameTag::new("hello"); let bytes = tag.as_bytes(); - assert_eq!(bytes, get_bytes()); - assert_eq!(tag.name(), Ok(MSG)); + assert_eq!(bytes, &get_bytes()[..]); + assert_eq!(tag.name(), Ok("hello")); // test also some bigger message let tag = BootLoaderNameTag::new("AbCdEfGhUjK YEAH"); diff --git a/multiboot2/src/builder/boxed_dst.rs b/multiboot2/src/builder/boxed_dst.rs index f433104e..b19b1391 100644 --- a/multiboot2/src/builder/boxed_dst.rs +++ b/multiboot2/src/builder/boxed_dst.rs @@ -1,6 +1,7 @@ //! Module for [`BoxedDst`]. -use crate::{Tag, TagTrait, TagTypeId}; +use crate::util::increase_to_alignment; +use crate::{TagHeader, TagTrait, TagTypeId}; use alloc::alloc::alloc; use core::alloc::Layout; use core::marker::PhantomData; @@ -40,14 +41,10 @@ impl + ?Sized> BoxedDst { let tag_size = size_of::() + size_of::() + content.len(); - // By using miri, I could figure out that there often are problems where - // miri thinks an allocation is smaller then necessary. Most probably - // due to not packed structs. Using packed structs however - // (especially with DSTs), is a crazy ass pain and unusable :/ Therefore, - // the best solution I can think of is to allocate a few byte more than - // necessary. I think that during runtime, everything works fine and - // that no memory issues are present. - let alloc_size = (tag_size + 7) & !7; // align to next 8 byte boundary + // The size of [the allocation for] a value is always a multiple of its + // alignment. + // https://doc.rust-lang.org/reference/type-layout.html + let alloc_size = increase_to_alignment(tag_size); let layout = Layout::from_size_align(alloc_size, ALIGN).unwrap(); let ptr = unsafe { alloc(layout) }; assert!(!ptr.is_null()); @@ -70,8 +67,8 @@ impl + ?Sized> BoxedDst { } } - let base_tag = unsafe { &*ptr.cast::() }; - let raw: *mut T = ptr_meta::from_raw_parts_mut(ptr.cast(), T::dst_size(base_tag)); + let base_tag = unsafe { &*ptr.cast::() }; + let raw: *mut T = ptr_meta::from_raw_parts_mut(ptr.cast(), T::dst_len(base_tag)); Self { ptr: NonNull::new(raw).unwrap(), @@ -101,10 +98,12 @@ impl PartialEq for BoxedDst { } #[cfg(test)] +#[cfg(not(miri))] mod tests { use super::*; - use crate::tag::StringError; - use crate::TagType; + use crate::test_util::AlignedBytes; + use crate::{parse_slice_as_string, StringError, TagHeader, TagType}; + use core::borrow::Borrow; const METADATA_SIZE: usize = 8; @@ -118,25 +117,24 @@ mod tests { impl CustomTag { fn string(&self) -> Result<&str, StringError> { - Tag::parse_slice_as_string(&self.string) + parse_slice_as_string(&self.string) } } impl TagTrait for CustomTag { const ID: TagType = TagType::Custom(0x1337); - fn dst_size(base_tag: &Tag) -> usize { - assert!(base_tag.size as usize >= METADATA_SIZE); - base_tag.size as usize - METADATA_SIZE + fn dst_len(header: &TagHeader) -> usize { + assert!(header.size as usize >= METADATA_SIZE); + header.size as usize - METADATA_SIZE } } #[test] fn test_boxed_dst_tag() { - let content = b"hallo\0"; + let content = AlignedBytes::new(*b"hallo\0"); let content_rust_str = "hallo"; - - let tag = BoxedDst::::new(content); + let tag = BoxedDst::::new(content.borrow()); assert_eq!(tag.typ, CustomTag::ID); assert_eq!(tag.size as usize, METADATA_SIZE + content.len()); assert_eq!(tag.string(), Ok(content_rust_str)); @@ -145,12 +143,9 @@ mod tests { #[test] fn can_hold_tag_trait() { const fn consume(_: &T) {} - let content = b"hallo\0"; - - let tag = BoxedDst::::new(content); + let content = AlignedBytes::new(*b"hallo\0"); + let tag = BoxedDst::::new(content.borrow()); consume(tag.deref()); consume(&*tag); - // Compiler not smart enough? - // consume(&tag); } } diff --git a/multiboot2/src/builder/information.rs b/multiboot2/src/builder/information.rs index 91b70eff..bf05f426 100644 --- a/multiboot2/src/builder/information.rs +++ b/multiboot2/src/builder/information.rs @@ -1,5 +1,6 @@ //! Exports item [`InformationBuilder`]. use crate::builder::{AsBytes, BoxedDst}; +use crate::util::increase_to_alignment; use crate::{ BasicMemoryInfoTag, BootInformationHeader, BootLoaderNameTag, CommandLineTag, EFIBootServicesNotExitedTag, EFIImageHandle32Tag, EFIImageHandle64Tag, EFIMemoryMapTag, @@ -78,12 +79,6 @@ impl InformationBuilder { Self(Vec::new()) } - /// Returns the provided number or the next multiple of 8. This is helpful - /// to ensure that the following tag starts at a 8-byte aligned boundary. - const fn size_or_up_aligned(size: usize) -> usize { - (size + 7) & !7 - } - /// Returns the expected length of the boot information, when the /// [`Self::build`]-method is called. This function assumes that the begin /// of the boot information is 8-byte aligned and automatically adds padding @@ -92,10 +87,8 @@ impl InformationBuilder { pub fn expected_len(&self) -> usize { let tag_size_iter = self.0.iter().map(|(_, bytes)| bytes.len()); - let payload_tags_size = tag_size_iter.fold(0, |acc, tag_size| { - // size_or_up_aligned: make sure next tag is 8-byte aligned - acc + Self::size_or_up_aligned(tag_size) - }); + let payload_tags_size = + tag_size_iter.fold(0, |acc, tag_size| acc + increase_to_alignment(tag_size)); size_of::() + payload_tags_size + size_of::() } @@ -112,7 +105,7 @@ impl InformationBuilder { if tag_type != TagType::End { let size = tag_serialized.len(); - let size_to_8_align = Self::size_or_up_aligned(size); + let size_to_8_align = increase_to_alignment(size); let size_to_8_align_diff = size_to_8_align - size; // fill zeroes so that next data block is 8-byte aligned dest_buf.extend([0].repeat(size_to_8_align_diff)); @@ -316,6 +309,7 @@ impl InformationBuilder { } #[cfg(test)] +#[cfg(not(miri))] mod tests { use crate::builder::information::InformationBuilder; use crate::{BasicMemoryInfoTag, BootInformation, CommandLineTag, ModuleTag}; @@ -349,14 +343,6 @@ mod tests { builder } - #[test] - fn test_size_or_up_aligned() { - assert_eq!(0, InformationBuilder::size_or_up_aligned(0)); - assert_eq!(8, InformationBuilder::size_or_up_aligned(1)); - assert_eq!(8, InformationBuilder::size_or_up_aligned(8)); - assert_eq!(16, InformationBuilder::size_or_up_aligned(9)); - } - /// Test of the `build` method in isolation specifically for miri to check /// for memory issues. #[test] diff --git a/multiboot2/src/command_line.rs b/multiboot2/src/command_line.rs index b48eeddb..cea436a7 100644 --- a/multiboot2/src/command_line.rs +++ b/multiboot2/src/command_line.rs @@ -1,14 +1,14 @@ //! Module for [`CommandLineTag`]. -use crate::tag::{StringError, TagHeader}; -use crate::{Tag, TagTrait, TagType, TagTypeId}; +use crate::tag::TagHeader; +use crate::{parse_slice_as_string, StringError, TagTrait, TagType}; use core::fmt::{Debug, Formatter}; use core::mem; use core::str; #[cfg(feature = "builder")] use {crate::builder::BoxedDst, alloc::vec::Vec}; -const METADATA_SIZE: usize = mem::size_of::() + mem::size_of::(); +const METADATA_SIZE: usize = mem::size_of::(); /// This tag contains the command line string. /// @@ -55,7 +55,7 @@ impl CommandLineTag { /// } /// ``` pub fn cmdline(&self) -> Result<&str, StringError> { - Tag::parse_slice_as_string(&self.cmdline) + parse_slice_as_string(&self.cmdline) } } @@ -72,54 +72,49 @@ impl Debug for CommandLineTag { impl TagTrait for CommandLineTag { const ID: TagType = TagType::Cmdline; - fn dst_size(base_tag: &Tag) -> usize { - assert!(base_tag.size as usize >= METADATA_SIZE); - base_tag.size as usize - METADATA_SIZE + fn dst_len(header: &TagHeader) -> usize { + assert!(header.size as usize >= METADATA_SIZE); + header.size as usize - METADATA_SIZE } } #[cfg(test)] mod tests { use super::*; + use crate::tag::{GenericTag, TagBytesRef}; + use crate::test_util::AlignedBytes; - const MSG: &str = "hello"; - - /// Returns the tag structure in bytes in little endian format. - fn get_bytes() -> std::vec::Vec { - // size is: 4 bytes for tag + 4 bytes for size + length of null-terminated string - let size = (4 + 4 + MSG.as_bytes().len() + 1) as u32; - [ - &((TagType::Cmdline.val()).to_le_bytes()), - &size.to_le_bytes(), - MSG.as_bytes(), - // Null Byte - &[0], - ] - .iter() - .flat_map(|bytes| bytes.iter()) - .copied() - .collect() + #[rustfmt::skip] + fn get_bytes() -> AlignedBytes<16> { + AlignedBytes::new([ + TagType::Cmdline.val() as u8, 0, 0, 0, + 14, 0, 0, 0, + b'h', b'e', b'l', b'l', b'o', b'\0', + /* padding */ + 0, 0 + ]) } /// Tests to parse a string with a terminating null byte from the tag (as the spec defines). #[test] - #[cfg_attr(miri, ignore)] fn test_parse_str() { - let tag = get_bytes(); - let tag = unsafe { &*tag.as_ptr().cast::() }; - let tag = tag.cast_tag::(); + let bytes = get_bytes(); + let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let tag = GenericTag::ref_from(bytes); + let tag = tag.cast::(); assert_eq!(tag.header.typ, TagType::Cmdline); - assert_eq!(tag.cmdline().expect("must be valid UTF-8"), MSG); + assert_eq!(tag.cmdline(), Ok("hello")); } /// Test to generate a tag from a given string. #[test] #[cfg(feature = "builder")] + #[ignore] fn test_build_str() { - let tag = CommandLineTag::new(MSG); + let tag = CommandLineTag::new("hello"); let bytes = tag.as_bytes(); - assert_eq!(bytes, get_bytes()); - assert_eq!(tag.cmdline(), Ok(MSG)); + assert_eq!(bytes, &get_bytes()[..]); + assert_eq!(tag.cmdline(), Ok("hello")); // test also some bigger message let tag = CommandLineTag::new("AbCdEfGhUjK YEAH"); diff --git a/multiboot2/src/efi.rs b/multiboot2/src/efi.rs index 863b8535..59f2fda4 100644 --- a/multiboot2/src/efi.rs +++ b/multiboot2/src/efi.rs @@ -7,7 +7,7 @@ //! - [`EFIBootServicesNotExitedTag`] use crate::tag::TagHeader; -use crate::{Tag, TagTrait, TagType}; +use crate::{TagTrait, TagType}; use core::mem::size_of; /// EFI system table in 32 bit mode tag. @@ -38,7 +38,7 @@ impl EFISdt32Tag { impl TagTrait for EFISdt32Tag { const ID: TagType = TagType::Efi32; - fn dst_size(_base_tag: &Tag) {} + fn dst_len(_: &TagHeader) {} } /// EFI system table in 64 bit mode tag. @@ -69,7 +69,7 @@ impl EFISdt64Tag { impl TagTrait for EFISdt64Tag { const ID: TagType = TagType::Efi64; - fn dst_size(_base_tag: &Tag) {} + fn dst_len(_: &TagHeader) {} } /// Tag that contains the pointer to the boot loader's UEFI image handle @@ -102,7 +102,7 @@ impl EFIImageHandle32Tag { impl TagTrait for EFIImageHandle32Tag { const ID: TagType = TagType::Efi32Ih; - fn dst_size(_base_tag: &Tag) {} + fn dst_len(_: &TagHeader) {} } /// Tag that contains the pointer to the boot loader's UEFI image handle @@ -135,7 +135,7 @@ impl EFIImageHandle64Tag { impl TagTrait for EFIImageHandle64Tag { const ID: TagType = TagType::Efi64Ih; - fn dst_size(_base_tag: &Tag) {} + fn dst_len(_: &TagHeader) {} } /// EFI ExitBootServices was not called tag. @@ -166,7 +166,7 @@ impl Default for EFIBootServicesNotExitedTag { impl TagTrait for EFIBootServicesNotExitedTag { const ID: TagType = TagType::EfiBs; - fn dst_size(_base_tag: &Tag) {} + fn dst_len(_: &TagHeader) {} } #[cfg(all(test, feature = "builder"))] diff --git a/multiboot2/src/elf_sections.rs b/multiboot2/src/elf_sections.rs index 849b08e5..d0d4f4a0 100644 --- a/multiboot2/src/elf_sections.rs +++ b/multiboot2/src/elf_sections.rs @@ -2,12 +2,12 @@ #[cfg(feature = "builder")] use crate::builder::BoxedDst; -use crate::{Tag, TagTrait, TagType, TagTypeId}; +use crate::{TagHeader, TagTrait, TagType}; use core::fmt::{Debug, Formatter}; use core::mem; use core::str::Utf8Error; -const METADATA_SIZE: usize = mem::size_of::() + 4 * mem::size_of::(); +const METADATA_SIZE: usize = mem::size_of::() + 3 * mem::size_of::(); /// This tag contains the section header table from an ELF binary. // The sections iterator is provided via the [`ElfSectionsTag::sections`] @@ -15,8 +15,7 @@ const METADATA_SIZE: usize = mem::size_of::() + 4 * mem::size_of:: usize { - assert!(base_tag.size as usize >= METADATA_SIZE); - base_tag.size as usize - METADATA_SIZE + fn dst_len(header: &TagHeader) -> usize { + assert!(header.size as usize >= METADATA_SIZE); + header.size as usize - METADATA_SIZE } } impl Debug for ElfSectionsTag { fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { f.debug_struct("ElfSectionsTag") - .field("typ", &{ self.typ }) - .field("size", &{ self.size }) - .field("number_of_sections", &{ self.number_of_sections }) - .field("entry_size", &{ self.entry_size }) - .field("shndx", &{ self.shndx }) + .field("typ", &self.header.typ) + .field("size", &self.header.size) + .field("number_of_sections", &self.number_of_sections) + .field("entry_size", &self.entry_size) + .field("shndx", &self.shndx) .field("sections", &self.sections()) .finish() } @@ -85,6 +84,7 @@ impl Debug for ElfSectionsTag { /// An iterator over some ELF sections. #[derive(Clone)] +/// TODO make this memory safe with lifetime capture. pub struct ElfSectionIter { current_section: *const u8, remaining_sections: u32, diff --git a/multiboot2/src/end.rs b/multiboot2/src/end.rs index e48b2238..f5dff6dd 100644 --- a/multiboot2/src/end.rs +++ b/multiboot2/src/end.rs @@ -1,6 +1,6 @@ //! Module for [`EndTag`]. -use crate::{Tag, TagTrait, TagType, TagTypeId}; +use crate::{TagHeader, TagTrait, TagType, TagTypeId}; /// The end tag ends the information struct. #[repr(C)] @@ -22,7 +22,7 @@ impl Default for EndTag { impl TagTrait for EndTag { const ID: TagType = TagType::End; - fn dst_size(_base_tag: &Tag) {} + fn dst_len(_: &TagHeader) {} } #[cfg(test)] diff --git a/multiboot2/src/framebuffer.rs b/multiboot2/src/framebuffer.rs index 50167b88..28447b59 100644 --- a/multiboot2/src/framebuffer.rs +++ b/multiboot2/src/framebuffer.rs @@ -1,7 +1,7 @@ //! Module for [`FramebufferTag`]. use crate::tag::TagHeader; -use crate::{Tag, TagTrait, TagType, TagTypeId}; +use crate::{TagTrait, TagType, TagTypeId}; use core::fmt::Debug; use core::mem; use core::slice; @@ -183,9 +183,9 @@ impl FramebufferTag { impl TagTrait for FramebufferTag { const ID: TagType = TagType::Framebuffer; - fn dst_size(base_tag: &Tag) -> usize { - assert!(base_tag.size as usize >= METADATA_SIZE); - base_tag.size as usize - METADATA_SIZE + fn dst_len(header: &TagHeader) -> usize { + assert!(header.size as usize >= METADATA_SIZE); + header.size as usize - METADATA_SIZE } } diff --git a/multiboot2/src/image_load_addr.rs b/multiboot2/src/image_load_addr.rs index b11127d4..fc5d060c 100644 --- a/multiboot2/src/image_load_addr.rs +++ b/multiboot2/src/image_load_addr.rs @@ -1,7 +1,7 @@ //! Module for [`ImageLoadPhysAddrTag`]. use crate::tag::TagHeader; -use crate::{Tag, TagTrait, TagType}; +use crate::{TagTrait, TagType}; #[cfg(feature = "builder")] use core::mem::size_of; @@ -36,7 +36,7 @@ impl ImageLoadPhysAddrTag { impl TagTrait for ImageLoadPhysAddrTag { const ID: TagType = TagType::LoadBaseAddr; - fn dst_size(_base_tag: &Tag) {} + fn dst_len(_: &TagHeader) {} } #[cfg(all(test, feature = "builder"))] diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index 46a41d5e..b83ef8ed 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -12,7 +12,7 @@ // now allow a few rules which are denied by the above statement // --> They are either ridiculous, not necessary, or we can't fix them. #![allow(clippy::multiple_crate_versions)] -#![deny(missing_docs)] +// #![deny(missing_docs)] #![deny(missing_debug_implementations)] #![deny(rustdoc::all)] // --- END STYLE CHECKS --- @@ -56,6 +56,8 @@ extern crate bitflags; #[cfg(feature = "builder")] pub mod builder; +#[cfg(test)] +pub(crate) mod test_util; mod boot_information; mod boot_loader_name; @@ -72,6 +74,7 @@ mod smbios; mod tag; mod tag_trait; mod tag_type; +pub(crate) mod util; mod vbe_info; pub use boot_information::{BootInformation, BootInformationHeader, MbiLoadError}; @@ -94,9 +97,10 @@ pub use module::{ModuleIter, ModuleTag}; pub use ptr_meta::Pointee; pub use rsdp::{RsdpV1Tag, RsdpV2Tag}; pub use smbios::SmbiosTag; -pub use tag::{StringError, Tag}; +pub use tag::TagHeader; pub use tag_trait::TagTrait; pub use tag_type::{TagType, TagTypeId}; +pub use util::{parse_slice_as_string, StringError}; pub use vbe_info::{ VBECapabilities, VBEControlInfo, VBEDirectColorAttributes, VBEField, VBEInfoTag, VBEMemoryModel, VBEModeAttributes, VBEModeInfo, VBEWindowAttributes, @@ -107,11 +111,13 @@ pub use vbe_info::{ /// machine state. pub const MAGIC: u32 = 0x36d76289; +/// The required alignment for tags and the boot information. +pub const ALIGNMENT: usize = 8; + #[cfg(test)] mod tests { use super::*; - use crate::memory_map::MemoryAreaType; - use crate::tag::StringError; + use crate::test_util::AlignedBytes; /// Compile time test to check if the boot information is Send and Sync. /// This test is relevant to give library users flexebility in passing the @@ -119,10 +125,7 @@ mod tests { #[test] fn boot_information_is_send_and_sync() { fn accept(_: T) {} - - #[repr(C, align(8))] - struct Bytes([u8; 16]); - let bytes: Bytes = Bytes([ + let bytes = AlignedBytes([ 16, 0, 0, 0, // total_size 0, 0, 0, 0, // reserved 0, 0, 0, 0, // end tag type @@ -138,9 +141,7 @@ mod tests { #[test] fn no_tags() { - #[repr(C, align(8))] - struct Bytes([u8; 16]); - let bytes: Bytes = Bytes([ + let bytes = AlignedBytes([ 16, 0, 0, 0, // total_size 0, 0, 0, 0, // reserved 0, 0, 0, 0, // end tag type @@ -163,9 +164,7 @@ mod tests { #[test] #[should_panic] fn invalid_total_size() { - #[repr(C, align(8))] - struct Bytes([u8; 15]); - let bytes: Bytes = Bytes([ + let bytes = AlignedBytes([ 15, 0, 0, 0, // total_size 0, 0, 0, 0, // reserved 0, 0, 0, 0, // end tag type @@ -188,9 +187,7 @@ mod tests { #[test] #[should_panic] fn invalid_end_tag() { - #[repr(C, align(8))] - struct Bytes([u8; 16]); - let bytes: Bytes = Bytes([ + let bytes = AlignedBytes([ 16, 0, 0, 0, // total_size 0, 0, 0, 0, // reserved 0, 0, 0, 0, // end tag type @@ -211,11 +208,8 @@ mod tests { } #[test] - #[cfg_attr(miri, ignore)] fn name_tag() { - #[repr(C, align(8))] - struct Bytes([u8; 32]); - let bytes: Bytes = Bytes([ + let bytes = AlignedBytes([ 32, 0, 0, 0, // total_size 0, 0, 0, 0, // reserved 2, 0, 0, 0, // boot loader name tag type @@ -246,14 +240,11 @@ mod tests { } #[test] - #[cfg_attr(miri, ignore)] fn framebuffer_tag_rgb() { // direct RGB mode test: // taken from GRUB2 running in QEMU at // 1280x720 with 32bpp in BGRA format. - #[repr(C, align(8))] - struct Bytes([u8; 56]); - let bytes: Bytes = Bytes([ + let bytes = AlignedBytes([ 56, 0, 0, 0, // total size 0, 0, 0, 0, // reserved 8, 0, 0, 0, // framebuffer tag type @@ -307,14 +298,11 @@ mod tests { } #[test] - #[cfg_attr(miri, ignore)] fn framebuffer_tag_indexed() { // indexed mode test: // this is synthetic, as I can't get QEMU // to run in indexed color mode. - #[repr(C, align(8))] - struct Bytes([u8; 64]); - let bytes: Bytes = Bytes([ + let bytes = AlignedBytes([ 64, 0, 0, 0, // total size 0, 0, 0, 0, // reserved 8, 0, 0, 0, // framebuffer tag type @@ -379,13 +367,10 @@ mod tests { } #[test] - #[cfg_attr(miri, ignore)] #[allow(clippy::cognitive_complexity)] fn vbe_info_tag() { //Taken from GRUB2 running in QEMU. - #[repr(C, align(8))] - struct Bytes([u8; 800]); - let bytes = Bytes([ + let bytes = AlignedBytes([ 32, 3, 0, 0, // Total size. 0, 0, 0, 0, // Reserved 7, 0, 0, 0, // Tag type. @@ -551,11 +536,10 @@ mod tests { /// Tests to parse a MBI that was statically extracted from a test run with /// GRUB as bootloader. #[test] + // TODO fix Miri #[cfg_attr(miri, ignore)] fn grub2() { - #[repr(C, align(8))] - struct Bytes([u8; 960]); - let mut bytes: Bytes = Bytes([ + let mut bytes = AlignedBytes([ 192, 3, 0, 0, // total_size 0, 0, 0, 0, // reserved 1, 0, 0, 0, // boot command tag type @@ -959,15 +943,14 @@ mod tests { } #[test] + // TODO fix Miri #[cfg_attr(miri, ignore)] fn elf_sections() { - #[repr(C, align(8))] - struct Bytes([u8; 168]); - let mut bytes: Bytes = Bytes([ + let mut bytes = AlignedBytes([ 168, 0, 0, 0, // total_size 0, 0, 0, 0, // reserved 9, 0, 0, 0, // elf symbols tag type - 20, 2, 0, 0, // elf symbols tag size + 148, 0, 0, 0, // elf symbols tag size 2, 0, 0, 0, // elf symbols num 64, 0, 0, 0, // elf symbols entsize 1, 0, 0, 0, // elf symbols shndx @@ -1036,12 +1019,9 @@ mod tests { } #[test] - #[cfg_attr(miri, ignore)] fn efi_memory_map() { - #[repr(C, align(8))] - struct Bytes([u8; 80]); // test that the EFI memory map is detected. - let bytes: Bytes = Bytes([ + let bytes = AlignedBytes([ 80, 0, 0, 0, // size 0, 0, 0, 0, // reserved 17, 0, 0, 0, // EFI memory map type @@ -1117,7 +1097,6 @@ mod tests { /// Example for a custom tag. #[test] - #[cfg_attr(miri, ignore)] fn get_custom_tag_from_mbi() { #[repr(C, align(8))] struct CustomTag { @@ -1129,13 +1108,10 @@ mod tests { impl TagTrait for CustomTag { const ID: TagType = TagType::Custom(0x1337); - fn dst_size(_base_tag: &Tag) {} + fn dst_len(_tag_header: &TagHeader) {} } - - #[repr(C, align(8))] - struct AlignedBytes([u8; 32]); // Raw bytes of a MBI that only contains the custom tag. - let bytes: AlignedBytes = AlignedBytes([ + let bytes = AlignedBytes([ 32, 0, 0, @@ -1183,7 +1159,6 @@ mod tests { /// Example for a custom DST tag. #[test] - #[cfg_attr(miri, ignore)] fn get_custom_dst_tag_from_mbi() { #[repr(C)] #[derive(crate::Pointee)] @@ -1195,25 +1170,22 @@ mod tests { impl CustomTag { fn name(&self) -> Result<&str, StringError> { - Tag::parse_slice_as_string(&self.name) + parse_slice_as_string(&self.name) } } impl TagTrait for CustomTag { const ID: TagType = TagType::Custom(0x1337); - fn dst_size(base_tag: &Tag) -> usize { + fn dst_len(header: &TagHeader) -> usize { // The size of the sized portion of the command line tag. let tag_base_size = 8; - assert!(base_tag.size >= 8); - base_tag.size as usize - tag_base_size + assert!(header.size >= 8); + header.size as usize - tag_base_size } } - - #[repr(C, align(8))] - struct AlignedBytes([u8; 32]); // Raw bytes of a MBI that only contains the custom tag. - let bytes: AlignedBytes = AlignedBytes([ + let bytes = AlignedBytes([ 32, 0, 0, @@ -1261,11 +1233,8 @@ mod tests { /// Tests that `get_tag` can consume multiple types that implement `Into` #[test] - #[cfg_attr(miri, ignore)] fn get_tag_into_variants() { - #[repr(C, align(8))] - struct Bytes([u8; 32]); - let bytes: Bytes = Bytes([ + let bytes = AlignedBytes([ 32, 0, 0, diff --git a/multiboot2/src/memory_map.rs b/multiboot2/src/memory_map.rs index b246de56..171cef3d 100644 --- a/multiboot2/src/memory_map.rs +++ b/multiboot2/src/memory_map.rs @@ -6,14 +6,14 @@ pub use uefi_raw::table::boot::MemoryDescriptor as EFIMemoryDesc; pub use uefi_raw::table::boot::MemoryType as EFIMemoryAreaType; use crate::tag::TagHeader; -use crate::{Tag, TagTrait, TagType, TagTypeId}; +use crate::{TagTrait, TagType, TagTypeId}; use core::fmt::{Debug, Formatter}; use core::marker::PhantomData; use core::mem; #[cfg(feature = "builder")] use {crate::builder::AsBytes, crate::builder::BoxedDst}; -const METADATA_SIZE: usize = mem::size_of::() + 3 * mem::size_of::(); +const METADATA_SIZE: usize = mem::size_of::() + 2 * mem::size_of::(); /// This tag provides an initial host memory map (legacy boot, not UEFI). /// @@ -75,9 +75,9 @@ impl MemoryMapTag { impl TagTrait for MemoryMapTag { const ID: TagType = TagType::Mmap; - fn dst_size(base_tag: &Tag) -> usize { - assert!(base_tag.size as usize >= METADATA_SIZE); - let size = base_tag.size as usize - METADATA_SIZE; + fn dst_len(header: &TagHeader) -> usize { + assert!(header.size as usize >= METADATA_SIZE); + let size = header.size as usize - METADATA_SIZE; assert_eq!(size % mem::size_of::(), 0); size / mem::size_of::() } @@ -287,7 +287,7 @@ impl BasicMemoryInfoTag { impl TagTrait for BasicMemoryInfoTag { const ID: TagType = TagType::BasicMeminfo; - fn dst_size(_base_tag: &Tag) {} + fn dst_len(_: &TagHeader) {} } const EFI_METADATA_SIZE: usize = mem::size_of::() + 3 * mem::size_of::(); @@ -408,9 +408,9 @@ impl Debug for EFIMemoryMapTag { impl TagTrait for EFIMemoryMapTag { const ID: TagType = TagType::EfiMmap; - fn dst_size(base_tag: &Tag) -> usize { - assert!(base_tag.size as usize >= EFI_METADATA_SIZE); - base_tag.size as usize - EFI_METADATA_SIZE + fn dst_len(header: &TagHeader) -> usize { + assert!(header.size as usize >= EFI_METADATA_SIZE); + header.size as usize - EFI_METADATA_SIZE } } diff --git a/multiboot2/src/module.rs b/multiboot2/src/module.rs index 5dd65047..612fbf64 100644 --- a/multiboot2/src/module.rs +++ b/multiboot2/src/module.rs @@ -1,13 +1,13 @@ //! Module for [`ModuleTag`]. -use crate::tag::{StringError, TagHeader, TagIter}; -use crate::{Tag, TagTrait, TagType, TagTypeId}; +use crate::tag::{TagHeader, TagIter}; +use crate::{parse_slice_as_string, StringError, TagTrait, TagType}; use core::fmt::{Debug, Formatter}; use core::mem; #[cfg(feature = "builder")] use {crate::builder::BoxedDst, alloc::vec::Vec}; -const METADATA_SIZE: usize = mem::size_of::() + 3 * mem::size_of::(); +const METADATA_SIZE: usize = mem::size_of::() + 2 * mem::size_of::(); /// The module tag can occur multiple times and specifies passed boot modules /// (blobs in memory). The tag itself doesn't include the blog, but references @@ -51,7 +51,7 @@ impl ModuleTag { /// /// If the function returns `Err` then perhaps the memory is invalid. pub fn cmdline(&self) -> Result<&str, StringError> { - Tag::parse_slice_as_string(&self.cmdline) + parse_slice_as_string(&self.cmdline) } /// Start address of the module. @@ -76,9 +76,9 @@ impl ModuleTag { impl TagTrait for ModuleTag { const ID: TagType = TagType::Module; - fn dst_size(base_tag: &Tag) -> usize { - assert!(base_tag.size as usize >= METADATA_SIZE); - base_tag.size as usize - METADATA_SIZE + fn dst_len(header: &TagHeader) -> usize { + assert!(header.size as usize >= METADATA_SIZE); + header.size as usize - METADATA_SIZE } } @@ -111,8 +111,8 @@ impl<'a> Iterator for ModuleIter<'a> { fn next(&mut self) -> Option<&'a ModuleTag> { self.iter - .find(|tag| tag.typ == TagType::Module) - .map(|tag| tag.cast_tag()) + .find(|tag| tag.header().typ == TagType::Module) + .map(|tag| tag.cast()) } } @@ -128,49 +128,45 @@ impl<'a> Debug for ModuleIter<'a> { #[cfg(test)] mod tests { - use crate::{ModuleTag, Tag, TagTrait, TagType}; - - const MSG: &str = "hello"; - - /// Returns the tag structure in bytes in little endian format. - fn get_bytes() -> std::vec::Vec { - // size is: 4 bytes for tag + 4 bytes for size + length of null-terminated string - // 4 bytes mod_start + 4 bytes mod_end - let size = (4 + 4 + 4 + 4 + MSG.as_bytes().len() + 1) as u32; - [ - &((TagType::Module.val()).to_le_bytes()), - &size.to_le_bytes(), - &0_u32.to_le_bytes(), - &1_u32.to_le_bytes(), - MSG.as_bytes(), - // Null Byte - &[0], - ] - .iter() - .flat_map(|bytes| bytes.iter()) - .copied() - .collect() + use super::*; + use crate::tag::{GenericTag, TagBytesRef}; + use crate::test_util::AlignedBytes; + + #[rustfmt::skip] + fn get_bytes() -> AlignedBytes<24> { + AlignedBytes::new([ + TagType::Module.val() as u8, 0, 0, 0, + 22, 0, 0, 0, + /* mod start */ + 0x00, 0xff, 0, 0, + /* mod end */ + 0xff, 0xff, 0, 0, + b'h', b'e', b'l', b'l', b'o', b'\0', + /* padding */ + 0, 0, + ]) } /// Tests to parse a string with a terminating null byte from the tag (as the spec defines). #[test] - #[cfg_attr(miri, ignore)] fn test_parse_str() { - let tag = get_bytes(); - let tag = unsafe { &*tag.as_ptr().cast::() }; - let tag = tag.cast_tag::(); + let bytes = get_bytes(); + let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let tag = GenericTag::ref_from(bytes); + let tag = tag.cast::(); assert_eq!(tag.header.typ, TagType::Module); - assert_eq!(tag.cmdline().expect("must be valid UTF-8"), MSG); + assert_eq!(tag.cmdline(), Ok("hello")); } /// Test to generate a tag from a given string. #[test] #[cfg(feature = "builder")] + #[ignore] fn test_build_str() { - let tag = ModuleTag::new(0, 1, MSG); + let tag = ModuleTag::new(0xff00, 0xffff, "hello"); let bytes = tag.as_bytes(); - assert_eq!(bytes, get_bytes()); - assert_eq!(tag.cmdline(), Ok(MSG)); + assert_eq!(bytes, &get_bytes()[..]); + assert_eq!(tag.cmdline(), Ok("hello")); // test also some bigger message let tag = ModuleTag::new(0, 1, "AbCdEfGhUjK YEAH"); diff --git a/multiboot2/src/rsdp.rs b/multiboot2/src/rsdp.rs index 8a4c289b..a894a155 100644 --- a/multiboot2/src/rsdp.rs +++ b/multiboot2/src/rsdp.rs @@ -13,7 +13,7 @@ //! use crate::tag::TagHeader; -use crate::{Tag, TagTrait, TagType}; +use crate::{TagTrait, TagType}; #[cfg(feature = "builder")] use core::mem::size_of; use core::slice; @@ -94,7 +94,7 @@ impl RsdpV1Tag { impl TagTrait for RsdpV1Tag { const ID: TagType = TagType::AcpiV1; - fn dst_size(_base_tag: &Tag) {} + fn dst_len(_: &TagHeader) {} } /// This tag contains a copy of RSDP as defined per ACPI 2.0 or later specification. @@ -191,5 +191,5 @@ impl RsdpV2Tag { impl TagTrait for RsdpV2Tag { const ID: TagType = TagType::AcpiV2; - fn dst_size(_base_tag: &Tag) {} + fn dst_len(_: &TagHeader) {} } diff --git a/multiboot2/src/smbios.rs b/multiboot2/src/smbios.rs index e66b732e..093bb3da 100644 --- a/multiboot2/src/smbios.rs +++ b/multiboot2/src/smbios.rs @@ -3,12 +3,11 @@ #[cfg(feature = "builder")] use crate::builder::BoxedDst; use crate::tag::TagHeader; -use crate::{Tag, TagTrait, TagType, TagTypeId}; +use crate::{TagTrait, TagType}; use core::fmt::Debug; use core::mem; -const METADATA_SIZE: usize = - mem::size_of::() + mem::size_of::() + mem::size_of::() * 8; +const METADATA_SIZE: usize = mem::size_of::() + mem::size_of::() * 8; /// This tag contains a copy of SMBIOS tables as well as their version. #[derive(ptr_meta::Pointee, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -53,9 +52,9 @@ impl SmbiosTag { impl TagTrait for SmbiosTag { const ID: TagType = TagType::Smbios; - fn dst_size(base_tag: &Tag) -> usize { - assert!(base_tag.size as usize >= METADATA_SIZE); - base_tag.size as usize - METADATA_SIZE + fn dst_len(header: &TagHeader) -> usize { + assert!(header.size as usize >= METADATA_SIZE); + header.size as usize - METADATA_SIZE } } @@ -73,41 +72,47 @@ impl Debug for SmbiosTag { #[cfg(test)] mod tests { use super::*; + use crate::tag::{GenericTag, TagBytesRef}; + use crate::test_util::AlignedBytes; - /// Returns the tag structure in bytes in little endian format. - fn get_bytes() -> std::vec::Vec { - let tables = [0xabu8; 24]; - // size is: 4 bytes for tag + 4 bytes for size + 1 byte for major and minor - // + 6 bytes reserved + the actual tables - let size = (4 + 4 + 1 + 1 + 6 + tables.len()) as u32; - let typ: u32 = TagType::Smbios.into(); - let mut bytes = [typ.to_le_bytes(), size.to_le_bytes()].concat(); - bytes.push(3); - bytes.push(0); - bytes.extend([0; 6]); - bytes.extend(tables); - bytes + #[rustfmt::skip] + fn get_bytes() -> AlignedBytes<32> { + AlignedBytes::new([ + TagType::Smbios.val() as u8, 0, 0, 0, + 25, 0, 0, 0, + /* major */ + 7, + /* minor */ + 42, + /* reserved */ + 0, 0, 0, 0, 0, 0, + /* table data */ + 0, 1, 2, 3, 4, 5, 6, 7, 8, + /* padding */ + 0, 0, 0, 0, 0, 0, 0 + ]) } /// Test to parse a given tag. #[test] - #[cfg_attr(miri, ignore)] fn test_parse() { - let tag = get_bytes(); - let tag = unsafe { &*tag.as_ptr().cast::() }; - let tag = tag.cast_tag::(); + let bytes = get_bytes(); + let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let tag = GenericTag::ref_from(bytes); + let tag = tag.cast::(); assert_eq!(tag.header.typ, TagType::Smbios); - assert_eq!(tag.major, 3); - assert_eq!(tag.minor, 0); - assert_eq!(tag.tables, [0xabu8; 24]); + assert_eq!(tag.major, 7); + assert_eq!(tag.minor, 42); + assert_eq!(&tag.tables, [0, 1, 2, 3, 4, 5, 6, 7, 8]); } /// Test to generate a tag. #[test] #[cfg(feature = "builder")] + #[ignore] fn test_build() { - let tag = SmbiosTag::new(3, 0, &[0xabu8; 24]); + let tag = SmbiosTag::new(7, 42, &[0, 1, 2, 3, 4, 5, 6, 7, 8]); let bytes = tag.as_bytes(); - assert_eq!(bytes, get_bytes()); + assert_eq!(bytes, &get_bytes()[..]); } } diff --git a/multiboot2/src/tag.rs b/multiboot2/src/tag.rs index 7764fc57..3336f145 100644 --- a/multiboot2/src/tag.rs +++ b/multiboot2/src/tag.rs @@ -1,45 +1,28 @@ //! Module for the base tag definitions and helper types. //! -//! The relevant exports of this module is [`Tag`]. - -use crate::{TagTrait, TagType, TagTypeId}; -use core::fmt; -use core::fmt::{Debug, Display, Formatter}; -use core::marker::PhantomData; -use core::str::Utf8Error; - -/// Error type describing failures when parsing the string from a tag. -#[derive(Debug, PartialEq, Eq, Clone)] -pub enum StringError { - /// There is no terminating NUL character, although the specification - /// requires one. - MissingNul(core::ffi::FromBytesUntilNulError), - /// The sequence until the first NUL character is not valid UTF-8. - Utf8(Utf8Error), -} +//! The relevant exports of this module are [`TagHeader`], [`GenericTag`], and +//! [`TagIter`]. +//! +//! The (internal) workflow to parse a tag from bytes is the following: +//! - `&[u8]` --> [`TagBytesRef`] +//! - [`TagBytesRef`] --> [`TagHeader`] +//! - [`TagBytesRef`] + [`TagHeader`] --> [`GenericTag`] +//! - [`GenericTag`] --> cast to desired tag -impl Display for StringError { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", self) - } -} - -#[cfg(feature = "unstable")] -impl core::error::Error for StringError { - fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { - match self { - Self::MissingNul(e) => Some(e), - Self::Utf8(e) => Some(e), - } - } -} +use crate::util::increase_to_alignment; +use crate::{TagTrait, TagType, TagTypeId, ALIGNMENT}; +use core::fmt::{Debug, Formatter}; +use core::mem; +use core::ops::Deref; +use core::ptr; /// The common header that all tags have in common. This type is ABI compatible. +/// It is the sized counterpart of [`GenericTag`]. /// /// Not to be confused with Multiboot header tags, which are something /// different. #[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] -#[repr(C)] +#[repr(C, align(8))] // Alignment also propagates to all tag types using this. pub struct TagHeader { /// The ABI-compatible [`TagType`]. pub typ: TagTypeId, /* u32 */ @@ -58,141 +41,402 @@ impl TagHeader { } } -/// Common base structure for all tags that can be passed via the Multiboot2 -/// Information Structure (MBI) to a Multiboot2 payload/program/kernel. +/// Wraps a byte slice representing a tag, but guarantees that the memory +/// requirements are fulfilled. /// -/// Can be transformed to any other tag (sized or unsized/DST) via -/// [`Tag::cast_tag`]. +/// This is the only type that can be used to construct a [`GenericTag`]. /// -/// Do not confuse them with the Multiboot2 header tags. They are something -/// different. -#[derive(Clone, Copy)] +/// The main reason for this dedicated type is to create fine-grained unit-tests +/// for Miri. +/// +/// # Memory Requirements (for Multiboot and Rust/Miri) +/// - At least as big as a `size_of()` +/// - at least [`ALIGNMENT`]-aligned +/// - Length is multiple of [`ALIGNMENT`]. In other words, there are enough +/// padding bytes until so that pointer coming right after the last byte +/// is [`ALIGNMENT`]-aligned +#[derive(Clone, Debug, PartialEq, Eq)] +#[repr(transparent)] +pub(crate) struct TagBytesRef<'a>(&'a [u8]); + +impl<'a> TryFrom<&'a [u8]> for TagBytesRef<'a> { + type Error = MemoryError; + + fn try_from(value: &'a [u8]) -> Result { + if value.len() < mem::size_of::() { + return Err(MemoryError::MinLengthNotSatisfied); + } + // Doesn't work as expected: if align_of_val(&value[0]) < ALIGNMENT { + if value.as_ptr().align_offset(ALIGNMENT) != 0 { + return Err(MemoryError::WrongAlignment); + } + let padding_bytes = value.len() % ALIGNMENT; + if padding_bytes != 0 { + return Err(MemoryError::MissingPadding); + } + Ok(Self(value)) + } +} + +impl<'a> Deref for TagBytesRef<'a> { + type Target = &'a [u8]; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +/// Errors that occur when constructing a [`TagBytesRef`]. +#[derive(Copy, Clone, Debug, Ord, PartialOrd, Eq, PartialEq, Hash)] +pub enum MemoryError { + /// The memory must be at least [`ALIGNMENT`]-aligned. + WrongAlignment, + /// The memory must cover at least the length of a [`TagHeader`]. + MinLengthNotSatisfied, + /// The buffer misses the terminating padding to the next alignment + /// boundary. + // This is mainly relevant to satisfy Miri. As the spec also mandates an + // alignment, we can rely on this property. + MissingPadding, +} + +/// A generic tag serving as base to cast to specific tags. This is a DST +/// version of [`TagHeader`] that solves various type and memory safety +/// problems by having a type that owns the whole memory of a tag. +#[derive(Eq, Ord, PartialEq, PartialOrd, ptr_meta::Pointee)] #[repr(C)] -#[allow(missing_docs)] // type will be removed soon anyway in its form -pub struct Tag { - pub typ: TagTypeId, // u32 - pub size: u32, - // followed by additional, tag specific fields +pub struct GenericTag { + header: TagHeader, + /// Payload of the tag that is reflected in the `size` attribute, thus, no + /// padding bytes! + payload: [u8], } -impl Tag { - /// Returns the underlying type of the tag. - #[must_use] - pub fn typ(&self) -> TagType { - self.typ.into() +impl GenericTag { + /// Base size of the DST struct without the dynamic part. + const BASE_SIZE: usize = mem::size_of::(); + + /// Creates a reference to a [`GenericTag`] from the provided `bytes` + /// [`TagBytesRef`]. + pub(crate) fn ref_from(bytes: TagBytesRef) -> &Self { + let header = bytes.as_ptr().cast::(); + let header = unsafe { &*header }; + let dst_len = Self::dst_len(header); + assert_eq!(header.size as usize, Self::BASE_SIZE + dst_len); + + let generic_tag: *const GenericTag = + ptr_meta::from_raw_parts(bytes.as_ptr().cast(), dst_len); + let generic_tag = unsafe { &*generic_tag }; + + generic_tag } - /// Casts the base tag to the specific tag type. - #[must_use] - pub fn cast_tag<'a, T: TagTrait + ?Sized + 'a>(&'a self) -> &'a T { - assert_eq!(self.typ, T::ID); - // Safety: At this point, we trust that "self.size" and the size hint - // for DST tags are sane. - unsafe { TagTrait::from_base_tag(self) } + pub fn header(&self) -> &TagHeader { + &self.header } - /// Parses the provided byte sequence as Multiboot string, which maps to a - /// [`str`]. - pub fn parse_slice_as_string(bytes: &[u8]) -> Result<&str, StringError> { - let cstr = core::ffi::CStr::from_bytes_until_nul(bytes).map_err(StringError::MissingNul)?; + pub fn payload(&self) -> &[u8] { + &self.payload + } - cstr.to_str().map_err(StringError::Utf8) + /// Casts the generic tag to a specific [`TagTrait`] implementation which + /// may be a ZST or DST typed tag. + pub fn cast(&self) -> &T { + let base_ptr = ptr::addr_of!(*self); + let t_dst_size = T::dst_len(&self.header); + 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)); + t_ref } } -impl Debug for Tag { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - let tag_type = TagType::from(self.typ); - - let mut debug = f.debug_struct("Tag"); - debug.field("typ", &tag_type); - - if !matches!(tag_type, TagType::Custom(_)) { - debug.field("typ (numeric)", &(u32::from(self.typ))); - } +impl Debug for GenericTag { + fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { + f.debug_struct("GenericTag") + .field("header", &self.header) + .field("payload", &"") + .finish() + } +} - debug.field("size", &(self.size)); +impl TagTrait for GenericTag { + const ID: TagType = TagType::Custom(0xffffffff); - debug.finish() + fn dst_len(header: &TagHeader) -> usize { + assert!(header.size as usize >= Self::BASE_SIZE); + header.size as usize - Self::BASE_SIZE } } -/// Iterates the MBI's tags from the first tag to the end tag. +/// Iterates the tags of the MBI from the first tag to the end tag. THe end tag +/// included. #[derive(Clone, Debug)] pub struct TagIter<'a> { - /// Pointer to the next tag. Updated in each iteration. - pub current: *const Tag, - /// The pointer right after the MBI. Used for additional bounds checking. - end_ptr_exclusive: *const u8, - /// Lifetime capture of the MBI's memory. - _mem: PhantomData<&'a ()>, + /// Absolute offset to next tag and updated in each iteration. + next_tag_offset: usize, + exclusive_end: *const u8, + buffer: &'a [u8], } impl<'a> TagIter<'a> { /// Creates a new iterator pub fn new(mem: &'a [u8]) -> Self { + // Assert alignment. assert_eq!(mem.as_ptr().align_offset(8), 0); + + let exclusive_end = unsafe { mem.as_ptr().add(mem.len()) }; + TagIter { - current: mem.as_ptr().cast(), - end_ptr_exclusive: unsafe { mem.as_ptr().add(mem.len()) }, - _mem: PhantomData, + next_tag_offset: 0, + buffer: mem, + exclusive_end, } } } impl<'a> Iterator for TagIter<'a> { - type Item = &'a Tag; + type Item = &'a GenericTag; - fn next(&mut self) -> Option<&'a Tag> { - // This never failed so far. But better be safe. - assert!(self.current.cast::() < self.end_ptr_exclusive); + fn next(&mut self) -> Option { + let next_ptr = unsafe { self.buffer.as_ptr().add(self.next_tag_offset) }; - let tag = unsafe { &*self.current }; - match tag.typ() { - TagType::End => None, // end tag - _ => { - // We return the tag and update self.current already to the next - // tag. + if next_ptr == self.exclusive_end { + return None; + } + assert!(next_ptr < self.exclusive_end); - // next pointer (rounded up to 8-byte alignment) - let ptr_offset = (tag.size as usize + 7) & !7; + let next_tag_ptr = next_ptr.cast::(); - // go to next tag - self.current = unsafe { self.current.cast::().add(ptr_offset).cast::() }; + let tag_hdr = unsafe { &*next_tag_ptr }; - Some(tag) - } - } + // Get relevant byte portion for the next tag. This includes padding + // bytes to fulfill Rust memory guarantees. Otherwise, Miri complains. + // See . + let bytes = { + let from = self.next_tag_offset; + let to = from + tag_hdr.size as usize; + // The size of [the allocation for] a value is always a multiple of its + // alignment. + // https://doc.rust-lang.org/reference/type-layout.html + let to = increase_to_alignment(to); + + // Update ptr for next iteration. + self.next_tag_offset += to - from; + + &self.buffer[from..to] + }; + + // Should never fail at this point. + let tag_bytes = TagBytesRef::try_from(bytes).unwrap(); + + Some(GenericTag::ref_from(tag_bytes)) } } #[cfg(test)] mod tests { use super::*; + use crate::test_util::AlignedBytes; + use core::mem; + + #[test] + fn test_new_generic_tag() { + let bytes = AlignedBytes::new([ + /* id: 0xffff_ffff */ + 0xff_u8, 0xff_u8, 0xff_u8, 0xff_u8, /* id: 16 */ + 16, 0, 0, 0, /* field a: 0xdead_beef */ + 0xde, 0xad, 0xbe, 0xef, /* field b: 0x1337_1337 */ + 0x13, 0x37, 0x13, 0x37, + ]); + + let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let tag = GenericTag::ref_from(bytes); + assert_eq!(tag.header.typ, 0xffff_ffff); + assert_eq!(tag.header.size, 16); + assert_eq!(tag.payload.len(), 8); + } + + #[test] + fn test_cast_generic_tag_to_sized_tag() { + #[repr(C)] + struct CustomTag { + tag_header: TagHeader, + a: u32, + b: u32, + } + + impl TagTrait for CustomTag { + const ID: TagType = TagType::End; + + fn dst_len(_header: &TagHeader) -> Self::Metadata {} + } + + let bytes = AlignedBytes([ + /* id: 0xffff_ffff */ + 0xff_u8, 0xff_u8, 0xff_u8, 0xff_u8, /* id: 16 */ + 16, 0, 0, 0, /* field a: 0xdead_beef */ + 0xef, 0xbe, 0xad, 0xde, /* field b: 0x1337_1337 */ + 0x37, 0x13, 0x37, 0x13, + ]); + let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let tag = GenericTag::ref_from(bytes); + let custom_tag = tag.cast::(); + + assert_eq!(mem::size_of_val(custom_tag), 16); + assert_eq!(custom_tag.a, 0xdead_beef); + assert_eq!(custom_tag.b, 0x1337_1337); + } + + #[test] + fn test_cast_generic_tag_to_dynamically_sized_tag() { + #[repr(C)] + #[derive(ptr_meta::Pointee)] + struct CustomDstTag { + tag_header: TagHeader, + a: u32, + payload: [u8], + } + + impl TagTrait for CustomDstTag { + const ID: TagType = TagType::End; + + fn dst_len(header: &TagHeader) -> Self::Metadata { + let base_size = mem::size_of::() + mem::size_of::(); + header.size as usize - base_size + } + } + + let bytes = AlignedBytes([ + /* id: 0xffff_ffff */ + 0xff_u8, 0xff_u8, 0xff_u8, 0xff_u8, /* id: 16 */ + 16, 0, 0, 0, /* field a: 0xdead_beef */ + 0xef, 0xbe, 0xad, 0xde, /* field b: 0x1337_1337 */ + 0x37, 0x13, 0x37, 0x13, + ]); + + let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let tag = GenericTag::ref_from(bytes); + let custom_tag = tag.cast::(); + + assert_eq!(mem::size_of_val(custom_tag), 16); + assert_eq!(custom_tag.a, 0xdead_beef); + assert_eq!(custom_tag.payload.len(), 4); + assert_eq!(custom_tag.payload[0], 0x37); + assert_eq!(custom_tag.payload[1], 0x13); + assert_eq!(custom_tag.payload[2], 0x37); + assert_eq!(custom_tag.payload[3], 0x13); + } + + #[test] + fn test_tag_bytes_ref() { + let empty: &[u8] = &[]; + assert_eq!( + TagBytesRef::try_from(empty), + Err(MemoryError::MinLengthNotSatisfied) + ); + + let slice = &[0_u8, 1, 2, 3, 4, 5, 6]; + assert_eq!( + TagBytesRef::try_from(&slice[..]), + Err(MemoryError::MinLengthNotSatisfied) + ); + + let slice = AlignedBytes([0_u8, 1, 2, 3, 4, 5, 6, 7, 0, 0, 0]); + // Guaranteed wrong alignment + let unaligned_slice = &slice[3..]; + assert_eq!( + TagBytesRef::try_from(&unaligned_slice[..]), + Err(MemoryError::WrongAlignment) + ); + + let slice = AlignedBytes([0_u8, 1, 2, 3, 4, 5, 6, 7]); + let slice = &slice[..]; + assert_eq!(TagBytesRef::try_from(slice), Ok(TagBytesRef(slice))); + } #[test] - fn parse_slice_as_string() { - // empty slice is invalid - assert!(matches!( - Tag::parse_slice_as_string(&[]), - Err(StringError::MissingNul(_)) - )); - // empty string is fine - assert_eq!(Tag::parse_slice_as_string(&[0x00]), Ok("")); - // reject invalid utf8 - assert!(matches!( - Tag::parse_slice_as_string(&[0xff, 0x00]), - Err(StringError::Utf8(_)) - )); - // reject missing null - assert!(matches!( - Tag::parse_slice_as_string(b"hello"), - Err(StringError::MissingNul(_)) - )); - // must not include final null - assert_eq!(Tag::parse_slice_as_string(b"hello\0"), Ok("hello")); - assert_eq!(Tag::parse_slice_as_string(b"hello\0\0"), Ok("hello")); - // must skip everytihng after first null - assert_eq!(Tag::parse_slice_as_string(b"hello\0foo"), Ok("hello")); + fn test_create_generic_tag() { + #[rustfmt::skip] + let bytes = AlignedBytes::new( + [ + TagType::Cmdline.val() as u8, 0, 0, 0, + /* Tag size */ + 18, 0, 0, 0, + /* Some payload. */ + 0, 1, 2, 3, + 4, 5, 6, 7, + 8, 9, + // Padding + 0, 0, 0, 0, 0, 0 + ], + ); + let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let tag = GenericTag::ref_from(bytes); + assert_eq!(tag.header.typ, TagType::Cmdline); + assert_eq!(tag.header.size, 8 + 10); + } + + #[test] + fn test_cast_generic_tag_to_generic_tag() { + #[rustfmt::skip] + let bytes = AlignedBytes::new( + [ + TagType::Cmdline.val() as u8, 0, 0, 0, + /* Tag size */ + 18, 0, 0, 0, + /* Some payload. */ + 0, 1, 2, 3, + 4, 5, 6, 7, + 8, 9, + // Padding + 0, 0, 0, 0, 0, 0 + ], + ); + let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let tag = GenericTag::ref_from(bytes); + + // Main objective here is also that this test passes Miri. + let tag = tag.cast::(); + assert_eq!(tag.header.typ, TagType::Cmdline); + assert_eq!(tag.header.size, 8 + 10); + } + + #[test] + fn test_tag_iter() { + #[rustfmt::skip] + let bytes = AlignedBytes::new( + [ + /* Some minimal tag. */ + 0xff, 0, 0, 0, + 8, 0, 0, 0, + /* Some tag with payload. */ + 0xfe, 0, 0, 0, + 12, 0, 0, 0, + 1, 2, 3, 4, + // Padding + 0, 0, 0, 0, + /* End tag */ + 0, 0, 0, 0, + 8, 0, 0, 0, + ], + ); + let mut iter = TagIter::new(&bytes[..]); + let first = iter.next().unwrap(); + assert_eq!(first.header.typ, TagType::Custom(0xff)); + assert_eq!(first.header.size, 8); + assert!(first.payload.is_empty()); + + let second = iter.next().unwrap(); + assert_eq!(second.header.typ, TagType::Custom(0xfe)); + assert_eq!(second.header.size, 12); + assert_eq!(&second.payload, &[1, 2, 3, 4]); + + let third = iter.next().unwrap(); + assert_eq!(third.header.typ, TagType::End); + assert_eq!(third.header.size, 8); + assert!(first.payload.is_empty()); + + assert_eq!(iter.next(), None); } } diff --git a/multiboot2/src/tag_trait.rs b/multiboot2/src/tag_trait.rs index c13b63d1..1d44d72f 100644 --- a/multiboot2/src/tag_trait.rs +++ b/multiboot2/src/tag_trait.rs @@ -1,10 +1,11 @@ //! Module for [`TagTrait`]. -use crate::{Tag, TagType}; +use crate::tag::TagHeader; +use crate::TagType; use ptr_meta::Pointee; /// A trait to abstract over all sized and unsized tags (DSTs). For sized tags, -/// this trait does not much. For DSTs, a [`TagTrait::dst_size`] implementation +/// this trait does not much. For DSTs, a [`TagTrait::dst_len`] implementation /// must be provided, which returns the right size hint for the dynamically /// sized portion of the struct. /// @@ -22,12 +23,12 @@ pub trait TagTrait: Pointee { /// /// For sized tags, this just returns `()`. For DSTs, this returns an /// `usize`. - fn dst_size(base_tag: &Tag) -> Self::Metadata; + fn dst_len(header: &TagHeader) -> Self::Metadata; /// Returns the tag as the common base tag structure. - fn as_base_tag(&self) -> &Tag { + fn as_base_tag(&self) -> &TagHeader { let ptr = core::ptr::addr_of!(*self); - unsafe { &*ptr.cast::() } + unsafe { &*ptr.cast::() } } /// Returns the total size of the tag. The depends on the `size` field of @@ -43,18 +44,4 @@ pub trait TagTrait: Pointee { let ptr = core::ptr::addr_of!(*self); unsafe { core::slice::from_raw_parts(ptr.cast(), self.size()) } } - - /// Creates a reference to a (dynamically sized) tag type in a safe way. - /// DST tags need to implement a proper [`Self::dst_size`] implementation. - /// - /// # Safety - /// Callers must be sure that the "size" field of the provided [`Tag`] is - /// sane and the underlying memory valid. The implementation of this trait - /// **must have** a correct [`Self::dst_size`] implementation. - #[must_use] - unsafe fn from_base_tag(tag: &Tag) -> &Self { - let ptr = core::ptr::addr_of!(*tag); - let ptr = ptr_meta::from_raw_parts(ptr.cast(), Self::dst_size(tag)); - &*ptr - } } diff --git a/multiboot2/src/test_util.rs b/multiboot2/src/test_util.rs new file mode 100644 index 00000000..a56156d0 --- /dev/null +++ b/multiboot2/src/test_util.rs @@ -0,0 +1,87 @@ +//! Various test utilities. + +use crate::ALIGNMENT; +use core::borrow::Borrow; +use core::ops::Deref; + +/// Helper to 8-byte align the underlying bytes, as mandated in the Multiboot2 +/// spec. With this type, one can create manual and raw Multiboot2 boot +/// information or just the bytes for simple tags, in a manual and raw approach. +#[cfg(test)] +#[repr(C, align(8))] +pub(crate) struct AlignedBytes(pub [u8; N]); + +impl AlignedBytes { + pub const fn new(bytes: [u8; N]) -> Self { + Self(bytes) + } +} + +impl Borrow<[u8; N]> for AlignedBytes { + fn borrow(&self) -> &[u8; N] { + &self.0 + } +} + +impl Borrow<[u8]> for AlignedBytes { + fn borrow(&self) -> &[u8] { + &self.0 + } +} + +impl Deref for AlignedBytes { + type Target = [u8; N]; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +// The tests down below are all Miri-approved. +#[cfg(test)] +mod tests { + use super::*; + use crate::tag::TagBytesRef; + use core::mem; + use core::ptr::addr_of; + + #[test] + fn abi() { + let bytes = AlignedBytes([0]); + assert_eq!(mem::align_of_val(&bytes), ALIGNMENT); + assert_eq!(bytes.as_ptr().align_offset(8), 0); + assert_eq!((addr_of!(bytes[0])).align_offset(8), 0); + + let bytes = AlignedBytes([0, 1, 2, 3, 4, 5, 6, 7]); + assert_eq!(mem::align_of_val(&bytes), ALIGNMENT); + assert_eq!(bytes.as_ptr().align_offset(8), 0); + assert_eq!((addr_of!(bytes[0])).align_offset(8), 0); + + let bytes = AlignedBytes([0, 1, 2, 3, 4, 5, 6, 7, 8, 9]); + assert_eq!(mem::align_of_val(&bytes), ALIGNMENT); + assert_eq!(bytes.as_ptr().align_offset(8), 0); + assert_eq!((addr_of!(bytes[0])).align_offset(8), 0); + assert_eq!((addr_of!(bytes[7])).align_offset(8), 1); + assert_eq!((addr_of!(bytes[8])).align_offset(8), 0); + assert_eq!((addr_of!(bytes[9])).align_offset(8), 7); + } + + #[test] + fn compatible_with_tag_bytes_ref_type() { + #[rustfmt::skip] + let bytes = AlignedBytes( + [ + /* tag id */ + 0, 0, 0, 0, + /* size */ + 14, 0, 0, 0, + /* arbitrary payload */ + 1, 2, 3, 4, + 5, 6, + /* padding */ + 0, 0, + ] + ); + let _a = TagBytesRef::try_from(&bytes[..]).unwrap(); + } +} diff --git a/multiboot2/src/util.rs b/multiboot2/src/util.rs new file mode 100644 index 00000000..a6504cf3 --- /dev/null +++ b/multiboot2/src/util.rs @@ -0,0 +1,90 @@ +//! Various utilities. + +use crate::ALIGNMENT; +use core::fmt; +use core::fmt::{Display, Formatter}; +use core::str::Utf8Error; + +/// Error type describing failures when parsing the string from a tag. +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum StringError { + /// There is no terminating NUL character, although the specification + /// requires one. + MissingNul(core::ffi::FromBytesUntilNulError), + /// The sequence until the first NUL character is not valid UTF-8. + Utf8(Utf8Error), +} + +impl Display for StringError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", self) + } +} + +#[cfg(feature = "unstable")] +impl core::error::Error for StringError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + Self::MissingNul(e) => Some(e), + Self::Utf8(e) => Some(e), + } + } +} + +/// Parses the provided byte sequence as Multiboot string, which maps to a +/// [`str`]. +pub fn parse_slice_as_string(bytes: &[u8]) -> Result<&str, StringError> { + let cstr = core::ffi::CStr::from_bytes_until_nul(bytes).map_err(StringError::MissingNul)?; + cstr.to_str().map_err(StringError::Utf8) +} + +/// Increases the given size to the next alignment boundary, if it is not a +/// multiple of the alignment yet. This is relevant as in Rust's [type layout], +/// the allocated size of a type is always a multiple of the alignment, even +/// if the type is smaller. +/// +/// [type layout]: https://doc.rust-lang.org/reference/type-layout.html +pub const fn increase_to_alignment(size: usize) -> usize { + let mask = ALIGNMENT - 1; + (size + mask) & !mask +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_slice_as_string() { + // empty slice is invalid + assert!(matches!( + parse_slice_as_string(&[]), + Err(StringError::MissingNul(_)) + )); + // empty string is fine + assert_eq!(parse_slice_as_string(&[0x00]), Ok("")); + // reject invalid utf8 + assert!(matches!( + parse_slice_as_string(&[0xff, 0x00]), + Err(StringError::Utf8(_)) + )); + // reject missing null + assert!(matches!( + parse_slice_as_string(b"hello"), + Err(StringError::MissingNul(_)) + )); + // must not include final null + assert_eq!(parse_slice_as_string(b"hello\0"), Ok("hello")); + assert_eq!(parse_slice_as_string(b"hello\0\0"), Ok("hello")); + // must skip everytihng after first null + assert_eq!(parse_slice_as_string(b"hello\0foo"), Ok("hello")); + } + + #[test] + fn test_increase_to_alignment() { + assert_eq!(increase_to_alignment(0), 0); + assert_eq!(increase_to_alignment(1), 8); + assert_eq!(increase_to_alignment(7), 8); + assert_eq!(increase_to_alignment(8), 8); + assert_eq!(increase_to_alignment(9), 16); + } +} diff --git a/multiboot2/src/vbe_info.rs b/multiboot2/src/vbe_info.rs index a5973f4c..6389d760 100644 --- a/multiboot2/src/vbe_info.rs +++ b/multiboot2/src/vbe_info.rs @@ -1,6 +1,6 @@ //! Module for [`VBEInfoTag`]. -use crate::{Tag, TagTrait, TagType, TagTypeId}; +use crate::{TagHeader, TagTrait, TagType, TagTypeId}; use core::fmt; /// This tag contains VBE metadata, VBE controller information returned by the @@ -42,7 +42,7 @@ pub struct VBEInfoTag { impl TagTrait for VBEInfoTag { const ID: TagType = TagType::Vbe; - fn dst_size(_base_tag: &Tag) {} + fn dst_len(_: &TagHeader) {} } /// VBE controller information. From ecfa1ab15334ab2b2b4286bb45771bab880d847a Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 10:51:58 +0200 Subject: [PATCH 02/17] multiboot2: align(8) for all tags This was already transitively the case as soon as they have a `header: TagHeader` field, however, for perfection, this can now safely be added. --- multiboot2/src/boot_information.rs | 4 ++-- multiboot2/src/boot_loader_name.rs | 2 +- multiboot2/src/command_line.rs | 2 +- multiboot2/src/efi.rs | 13 +++++++------ multiboot2/src/elf_sections.rs | 2 +- multiboot2/src/end.rs | 2 +- multiboot2/src/framebuffer.rs | 2 +- multiboot2/src/image_load_addr.rs | 2 +- multiboot2/src/memory_map.rs | 2 +- multiboot2/src/module.rs | 2 +- multiboot2/src/rsdp.rs | 4 ++-- multiboot2/src/smbios.rs | 2 +- multiboot2/src/vbe_info.rs | 2 +- 13 files changed, 21 insertions(+), 20 deletions(-) diff --git a/multiboot2/src/boot_information.rs b/multiboot2/src/boot_information.rs index 42a46702..f014db3c 100644 --- a/multiboot2/src/boot_information.rs +++ b/multiboot2/src/boot_information.rs @@ -38,7 +38,7 @@ impl core::error::Error for MbiLoadError {} /// The basic header of a [`BootInformation`] as sized Rust type. #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C)] +#[repr(C, align(8))] pub struct BootInformationHeader { // size is multiple of 8 total_size: u32, @@ -68,7 +68,7 @@ impl AsBytes for BootInformationHeader {} /// This type holds the whole data of the MBI. This helps to better satisfy miri /// when it checks for memory issues. #[derive(ptr_meta::Pointee)] -#[repr(C)] +#[repr(C, align(8))] struct BootInformationInner { header: BootInformationHeader, tags: [u8], diff --git a/multiboot2/src/boot_loader_name.rs b/multiboot2/src/boot_loader_name.rs index e2d6c8a1..81a006fc 100644 --- a/multiboot2/src/boot_loader_name.rs +++ b/multiboot2/src/boot_loader_name.rs @@ -11,7 +11,7 @@ const METADATA_SIZE: usize = mem::size_of::(); /// The bootloader name tag. #[derive(ptr_meta::Pointee, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C)] +#[repr(C, align(8))] pub struct BootLoaderNameTag { header: TagHeader, /// Null-terminated UTF-8 string diff --git a/multiboot2/src/command_line.rs b/multiboot2/src/command_line.rs index cea436a7..419934ff 100644 --- a/multiboot2/src/command_line.rs +++ b/multiboot2/src/command_line.rs @@ -15,7 +15,7 @@ const METADATA_SIZE: usize = mem::size_of::(); /// The string is a normal C-style UTF-8 zero-terminated string that can be /// obtained via the `command_line` method. #[derive(ptr_meta::Pointee, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C)] +#[repr(C, align(8))] pub struct CommandLineTag { header: TagHeader, /// Null-terminated UTF-8 string diff --git a/multiboot2/src/efi.rs b/multiboot2/src/efi.rs index 59f2fda4..4c8d727a 100644 --- a/multiboot2/src/efi.rs +++ b/multiboot2/src/efi.rs @@ -12,7 +12,7 @@ use core::mem::size_of; /// EFI system table in 32 bit mode tag. #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C)] +#[repr(C, align(8))] pub struct EFISdt32Tag { header: TagHeader, pointer: u32, @@ -43,7 +43,7 @@ impl TagTrait for EFISdt32Tag { /// EFI system table in 64 bit mode tag. #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C)] +#[repr(C, align(8))] pub struct EFISdt64Tag { header: TagHeader, pointer: u64, @@ -75,7 +75,7 @@ impl TagTrait for EFISdt64Tag { /// Tag that contains the pointer to the boot loader's UEFI image handle /// (32-bit). #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C)] +#[repr(C, align(8))] pub struct EFIImageHandle32Tag { header: TagHeader, pointer: u32, @@ -108,7 +108,7 @@ impl TagTrait for EFIImageHandle32Tag { /// Tag that contains the pointer to the boot loader's UEFI image handle /// (64-bit). #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C)] +#[repr(C, align(8))] pub struct EFIImageHandle64Tag { header: TagHeader, pointer: u64, @@ -138,9 +138,10 @@ impl TagTrait for EFIImageHandle64Tag { fn dst_len(_: &TagHeader) {} } -/// EFI ExitBootServices was not called tag. +/// EFI ExitBootServices was not called tag. This tag has no payload and is +/// just a marker. #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C)] +#[repr(C, align(8))] pub struct EFIBootServicesNotExitedTag { header: TagHeader, } diff --git a/multiboot2/src/elf_sections.rs b/multiboot2/src/elf_sections.rs index d0d4f4a0..a084b434 100644 --- a/multiboot2/src/elf_sections.rs +++ b/multiboot2/src/elf_sections.rs @@ -13,7 +13,7 @@ const METADATA_SIZE: usize = mem::size_of::() + 3 * mem::size_of::() /// The VBE Framebuffer information tag. #[derive(ptr_meta::Pointee, Eq)] -#[repr(C)] +#[repr(C, align(8))] pub struct FramebufferTag { header: TagHeader, diff --git a/multiboot2/src/image_load_addr.rs b/multiboot2/src/image_load_addr.rs index fc5d060c..7248d782 100644 --- a/multiboot2/src/image_load_addr.rs +++ b/multiboot2/src/image_load_addr.rs @@ -9,7 +9,7 @@ use core::mem::size_of; /// binary was relocated, for example if the relocatable header tag was /// specified. #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C)] +#[repr(C, align(8))] pub struct ImageLoadPhysAddrTag { header: TagHeader, load_base_addr: u32, diff --git a/multiboot2/src/memory_map.rs b/multiboot2/src/memory_map.rs index 171cef3d..fcab800e 100644 --- a/multiboot2/src/memory_map.rs +++ b/multiboot2/src/memory_map.rs @@ -26,7 +26,7 @@ const METADATA_SIZE: usize = mem::size_of::() + 2 * mem::size_of::() + 2 * mem::size_of::() + mem::size_of::() /// This tag contains a copy of SMBIOS tables as well as their version. #[derive(ptr_meta::Pointee, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C)] +#[repr(C, align(8))] pub struct SmbiosTag { header: TagHeader, major: u8, diff --git a/multiboot2/src/vbe_info.rs b/multiboot2/src/vbe_info.rs index 6389d760..961cc883 100644 --- a/multiboot2/src/vbe_info.rs +++ b/multiboot2/src/vbe_info.rs @@ -6,7 +6,7 @@ use core::fmt; /// This tag contains VBE metadata, VBE controller information returned by the /// VBE Function 00h and VBE mode information returned by the VBE Function 01h. #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C)] +#[repr(C, align(8))] pub struct VBEInfoTag { typ: TagTypeId, length: u32, From 5148bbb3dc6c0761fe0e763214d8d170707254c5 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 10:51:59 +0200 Subject: [PATCH 03/17] multiboot2: Memory-safe new_boxed This will replace the existing BoxedDst typ, which has UB. --- multiboot2/src/lib.rs | 2 +- multiboot2/src/util.rs | 49 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index b83ef8ed..8fffb090 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -100,7 +100,7 @@ pub use smbios::SmbiosTag; pub use tag::TagHeader; pub use tag_trait::TagTrait; pub use tag_type::{TagType, TagTypeId}; -pub use util::{parse_slice_as_string, StringError}; +pub use util::{new_boxed, parse_slice_as_string, StringError}; pub use vbe_info::{ VBECapabilities, VBEControlInfo, VBEDirectColorAttributes, VBEField, VBEInfoTag, VBEMemoryModel, VBEModeAttributes, VBEModeInfo, VBEWindowAttributes, diff --git a/multiboot2/src/util.rs b/multiboot2/src/util.rs index a6504cf3..e59b1365 100644 --- a/multiboot2/src/util.rs +++ b/multiboot2/src/util.rs @@ -1,9 +1,13 @@ //! Various utilities. -use crate::ALIGNMENT; +use crate::tag::GenericTag; +use crate::{TagHeader, TagTrait, TagType, ALIGNMENT}; use core::fmt; use core::fmt::{Display, Formatter}; use core::str::Utf8Error; +use core::{ptr, slice}; +#[cfg(feature = "builder")] +use {alloc::alloc::Layout, alloc::boxed::Box}; /// Error type describing failures when parsing the string from a tag. #[derive(Debug, PartialEq, Eq, Clone)] @@ -31,6 +35,38 @@ impl core::error::Error for StringError { } } +/// Creates a new tag implementing [`TagTrait`] on the heap. This works for +/// sized and unsized tags. However, it only makes sense to use this for tags +/// that are DSTs (unsized), as for the sized ones, you can call a regular +/// constructor and box the result. +/// +/// # Parameters +/// - `additional_bytes`: All bytes apart from the default [`TagHeader`] that +/// are included into the tag. +#[cfg(feature = "alloc")] +pub fn new_boxed(additional_bytes: &[u8]) -> Box { + let size = size_of::() + additional_bytes.iter().len(); + let alloc_size = increase_to_alignment(size); + let layout = Layout::from_size_align(alloc_size, ALIGNMENT).unwrap(); + let heap_ptr = unsafe { alloc::alloc::alloc(layout) }; + assert!(!heap_ptr.is_null()); + + unsafe { + heap_ptr.cast::().write(T::ID.val()); + heap_ptr.cast::().add(1).write(size as u32); + } + unsafe { + let ptr = heap_ptr.add(size_of::()); + ptr::copy_nonoverlapping(additional_bytes.as_ptr(), ptr, additional_bytes.len()); + } + + let header = unsafe { heap_ptr.cast::().as_ref() }.unwrap(); + + let ptr = ptr_meta::from_raw_parts_mut(heap_ptr.cast(), T::dst_len(header)); + + unsafe { Box::from_raw(ptr) } +} + /// Parses the provided byte sequence as Multiboot string, which maps to a /// [`str`]. pub fn parse_slice_as_string(bytes: &[u8]) -> Result<&str, StringError> { @@ -52,6 +88,8 @@ pub const fn increase_to_alignment(size: usize) -> usize { #[cfg(test)] mod tests { use super::*; + use crate::tag::GenericTag; + use crate::CommandLineTag; #[test] fn test_parse_slice_as_string() { @@ -87,4 +125,13 @@ mod tests { assert_eq!(increase_to_alignment(8), 8); assert_eq!(increase_to_alignment(9), 16); } + + #[test] + fn test_new_boxed() { + let tag = new_boxed::(&[0, 1, 2, 3]); + assert_eq!(tag.header().typ, GenericTag::ID); + {} + let tag = new_boxed::("hello\0".as_bytes()); + assert_eq!(tag.cmdline(), Ok("hello")); + } } From e5cccec2599c426ea4544793c276065aa6c8e2c1 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 10:52:00 +0200 Subject: [PATCH 04/17] multiboot2: remove BoxedDst + run builder tests by Miri --- multiboot2/src/boot_loader_name.rs | 13 +-- multiboot2/src/builder/boxed_dst.rs | 151 -------------------------- multiboot2/src/builder/information.rs | 29 +++-- multiboot2/src/builder/mod.rs | 3 - multiboot2/src/command_line.rs | 9 +- multiboot2/src/elf_sections.rs | 13 +-- multiboot2/src/framebuffer.rs | 6 +- multiboot2/src/lib.rs | 8 +- multiboot2/src/memory_map.rs | 14 +-- multiboot2/src/module.rs | 9 +- multiboot2/src/smbios.rs | 11 +- 11 files changed, 50 insertions(+), 216 deletions(-) delete mode 100644 multiboot2/src/builder/boxed_dst.rs diff --git a/multiboot2/src/boot_loader_name.rs b/multiboot2/src/boot_loader_name.rs index 81a006fc..62f1bc91 100644 --- a/multiboot2/src/boot_loader_name.rs +++ b/multiboot2/src/boot_loader_name.rs @@ -1,11 +1,11 @@ //! Module for [`BootLoaderNameTag`]. use crate::tag::TagHeader; -use crate::{parse_slice_as_string, StringError, TagTrait, TagType}; +use crate::{new_boxed, parse_slice_as_string, StringError, TagTrait, TagType}; use core::fmt::{Debug, Formatter}; use core::mem; #[cfg(feature = "builder")] -use {crate::builder::BoxedDst, alloc::vec::Vec}; +use {alloc::boxed::Box, alloc::vec::Vec}; const METADATA_SIZE: usize = mem::size_of::(); @@ -22,13 +22,13 @@ impl BootLoaderNameTag { /// Constructs a new tag. #[cfg(feature = "builder")] #[must_use] - pub fn new(name: &str) -> BoxedDst { + pub fn new(name: &str) -> Box { let mut bytes: Vec<_> = name.bytes().collect(); if !bytes.ends_with(&[0]) { // terminating null-byte bytes.push(0); } - BoxedDst::new(&bytes) + new_boxed(&bytes) } /// Returns the underlying [`TagType`]. @@ -94,7 +94,7 @@ mod tests { fn get_bytes() -> AlignedBytes<16> { AlignedBytes::new([ TagType::BootLoaderName.val() as u8, 0, 0, 0, - 15, 0, 0, 0, + 14, 0, 0, 0, b'h', b'e', b'l', b'l', b'o', b'\0', /* padding */ 0, 0 @@ -115,11 +115,10 @@ mod tests { /// Test to generate a tag from a given string. #[test] #[cfg(feature = "builder")] - #[ignore] fn test_build_str() { let tag = BootLoaderNameTag::new("hello"); let bytes = tag.as_bytes(); - assert_eq!(bytes, &get_bytes()[..]); + assert_eq!(bytes, &get_bytes()[..tag.size()]); assert_eq!(tag.name(), Ok("hello")); // test also some bigger message diff --git a/multiboot2/src/builder/boxed_dst.rs b/multiboot2/src/builder/boxed_dst.rs deleted file mode 100644 index b19b1391..00000000 --- a/multiboot2/src/builder/boxed_dst.rs +++ /dev/null @@ -1,151 +0,0 @@ -//! Module for [`BoxedDst`]. - -use crate::util::increase_to_alignment; -use crate::{TagHeader, TagTrait, TagTypeId}; -use alloc::alloc::alloc; -use core::alloc::Layout; -use core::marker::PhantomData; -use core::mem::size_of; -use core::ops::Deref; -use core::ptr::NonNull; - -/// A helper type to create boxed DST, i.e., tags with a dynamic size for the -/// builder. This is tricky in Rust. This type behaves similar to the regular -/// `Box` type except that it ensure the same layout is used for the (explicit) -/// allocation and the (implicit) deallocation of memory. Otherwise, I didn't -/// find any way to figure out the right layout for a DST. Miri always reported -/// issues that the deallocation used a wrong layout. -/// -/// Technically, I'm certain this code is memory safe. But with this type, I -/// also can convince miri that it is. -#[derive(Debug, Eq)] -pub struct BoxedDst { - ptr: core::ptr::NonNull, - layout: Layout, - // marker: I used this only as the regular Box impl also does it. - _marker: PhantomData, -} - -impl + ?Sized> BoxedDst { - /// Create a boxed tag with the given content. - /// - /// # Parameters - /// - `content` - All payload bytes of the DST tag without the tag type or - /// the size. The memory is only read and can be discarded - /// afterwards. - pub(crate) fn new(content: &[u8]) -> Self { - // Currently, I do not find a nice way of making this dynamic so that - // also miri is guaranteed to be happy. But it seems that 4 is fine - // here. I do have control over allocation and deallocation. - const ALIGN: usize = 4; - - let tag_size = size_of::() + size_of::() + content.len(); - - // The size of [the allocation for] a value is always a multiple of its - // alignment. - // https://doc.rust-lang.org/reference/type-layout.html - let alloc_size = increase_to_alignment(tag_size); - let layout = Layout::from_size_align(alloc_size, ALIGN).unwrap(); - let ptr = unsafe { alloc(layout) }; - assert!(!ptr.is_null()); - - // write tag content to memory - unsafe { - // write tag type - let ptrx = ptr.cast::(); - ptrx.write(T::ID.into()); - - // write tag size - let ptrx = ptrx.add(1).cast::(); - ptrx.write(tag_size as u32); - - // write rest of content - let ptrx = ptrx.add(1).cast::(); - let tag_content_slice = core::slice::from_raw_parts_mut(ptrx, content.len()); - for (i, &byte) in content.iter().enumerate() { - tag_content_slice[i] = byte; - } - } - - let base_tag = unsafe { &*ptr.cast::() }; - let raw: *mut T = ptr_meta::from_raw_parts_mut(ptr.cast(), T::dst_len(base_tag)); - - Self { - ptr: NonNull::new(raw).unwrap(), - layout, - _marker: PhantomData, - } - } -} - -impl Drop for BoxedDst { - fn drop(&mut self) { - unsafe { alloc::alloc::dealloc(self.ptr.as_ptr().cast(), self.layout) } - } -} - -impl Deref for BoxedDst { - type Target = T; - fn deref(&self) -> &Self::Target { - unsafe { self.ptr.as_ref() } - } -} - -impl PartialEq for BoxedDst { - fn eq(&self, other: &Self) -> bool { - self.deref().eq(other.deref()) - } -} - -#[cfg(test)] -#[cfg(not(miri))] -mod tests { - use super::*; - use crate::test_util::AlignedBytes; - use crate::{parse_slice_as_string, StringError, TagHeader, TagType}; - use core::borrow::Borrow; - - const METADATA_SIZE: usize = 8; - - #[derive(ptr_meta::Pointee)] - #[repr(C)] - struct CustomTag { - typ: TagTypeId, - size: u32, - string: [u8], - } - - impl CustomTag { - fn string(&self) -> Result<&str, StringError> { - parse_slice_as_string(&self.string) - } - } - - impl TagTrait for CustomTag { - const ID: TagType = TagType::Custom(0x1337); - - fn dst_len(header: &TagHeader) -> usize { - assert!(header.size as usize >= METADATA_SIZE); - header.size as usize - METADATA_SIZE - } - } - - #[test] - fn test_boxed_dst_tag() { - let content = AlignedBytes::new(*b"hallo\0"); - let content_rust_str = "hallo"; - let tag = BoxedDst::::new(content.borrow()); - assert_eq!(tag.typ, CustomTag::ID); - assert_eq!(tag.size as usize, METADATA_SIZE + content.len()); - assert_eq!(tag.string(), Ok(content_rust_str)); - } - - #[test] - fn can_hold_tag_trait() { - const fn consume(_: &T) {} - let content = AlignedBytes::new(*b"hallo\0"); - let tag = BoxedDst::::new(content.borrow()); - consume(tag.deref()); - consume(&*tag); - } -} diff --git a/multiboot2/src/builder/information.rs b/multiboot2/src/builder/information.rs index bf05f426..5b317bf4 100644 --- a/multiboot2/src/builder/information.rs +++ b/multiboot2/src/builder/information.rs @@ -1,12 +1,13 @@ //! Exports item [`InformationBuilder`]. -use crate::builder::{AsBytes, BoxedDst}; +use crate::builder::AsBytes; use crate::util::increase_to_alignment; use crate::{ BasicMemoryInfoTag, BootInformationHeader, BootLoaderNameTag, CommandLineTag, EFIBootServicesNotExitedTag, EFIImageHandle32Tag, EFIImageHandle64Tag, EFIMemoryMapTag, EFISdt32Tag, EFISdt64Tag, ElfSectionsTag, EndTag, FramebufferTag, ImageLoadPhysAddrTag, - MemoryMapTag, ModuleTag, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagTrait, TagType, + MemoryMapTag, ModuleTag, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagTrait, TagType, ALIGNMENT, }; +use alloc::boxed::Box; use alloc::vec::Vec; use core::fmt::{Display, Formatter}; use core::mem::size_of; @@ -115,8 +116,6 @@ impl InformationBuilder { /// Constructs the bytes for a valid Multiboot2 information with the given properties. #[must_use] pub fn build(self) -> BootInformationBytes { - const ALIGN: usize = 8; - // PHASE 1/2: Prepare Vector // We allocate more than necessary so that we can ensure an correct @@ -134,7 +133,7 @@ impl InformationBuilder { // Unfortunately, it is not possible to reliably test this in a unit // test as long as the allocator_api feature is not stable. // Due to my manual testing, however, it works. - let offset = bytes.as_ptr().align_offset(ALIGN); + let offset = bytes.as_ptr().align_offset(ALIGNMENT); bytes.extend([0].repeat(offset)); // ----------------------------------------------- @@ -182,6 +181,8 @@ impl InformationBuilder { .0 .iter() .map(|(typ, _)| *typ) + // TODO make a type for tag_is_allowed_multiple_times so that we can + // link to it in the doc. .any(|typ| typ == T::ID && !Self::tag_is_allowed_multiple_times(typ)); if is_redundant_tag { @@ -205,13 +206,13 @@ impl InformationBuilder { /// Adds a 'bootloader name' tag (represented by [`BootLoaderNameTag`]) to the builder. #[must_use] - pub fn bootloader_name_tag(self, tag: BoxedDst) -> Self { + pub fn bootloader_name_tag(self, tag: Box) -> Self { self.add_tag(&*tag).unwrap() } /// Adds a 'command line' tag (represented by [`CommandLineTag`]) to the builder. #[must_use] - pub fn command_line_tag(self, tag: BoxedDst) -> Self { + pub fn command_line_tag(self, tag: Box) -> Self { self.add_tag(&*tag).unwrap() } @@ -247,19 +248,19 @@ impl InformationBuilder { /// Adds a 'EFI Memory map' tag (represented by [`EFIMemoryMapTag`]) to the builder. #[must_use] - pub fn efi_memory_map_tag(self, tag: BoxedDst) -> Self { + pub fn efi_memory_map_tag(self, tag: Box) -> Self { self.add_tag(&*tag).unwrap() } /// Adds a 'ELF-Symbols' tag (represented by [`ElfSectionsTag`]) to the builder. #[must_use] - pub fn elf_sections_tag(self, tag: BoxedDst) -> Self { + pub fn elf_sections_tag(self, tag: Box) -> Self { self.add_tag(&*tag).unwrap() } /// Adds a 'Framebuffer info' tag (represented by [`FramebufferTag`]) to the builder. #[must_use] - pub fn framebuffer_tag(self, tag: BoxedDst) -> Self { + pub fn framebuffer_tag(self, tag: Box) -> Self { self.add_tag(&*tag).unwrap() } @@ -271,14 +272,14 @@ impl InformationBuilder { /// Adds a (*none EFI*) 'memory map' tag (represented by [`MemoryMapTag`]) to the builder. #[must_use] - pub fn memory_map_tag(self, tag: BoxedDst) -> Self { + pub fn memory_map_tag(self, tag: Box) -> Self { self.add_tag(&*tag).unwrap() } /// Adds a 'Modules' tag (represented by [`ModuleTag`]) to the builder. /// This tag can occur multiple times in boot information. #[must_use] - pub fn add_module_tag(self, tag: BoxedDst) -> Self { + pub fn add_module_tag(self, tag: Box) -> Self { self.add_tag(&*tag).unwrap() } @@ -296,7 +297,7 @@ impl InformationBuilder { /// Adds a 'SMBIOS tables' tag (represented by [`SmbiosTag`]) to the builder. #[must_use] - pub fn smbios_tag(self, tag: BoxedDst) -> Self { + pub fn smbios_tag(self, tag: Box) -> Self { self.add_tag(&*tag).unwrap() } @@ -309,7 +310,6 @@ impl InformationBuilder { } #[cfg(test)] -#[cfg(not(miri))] mod tests { use crate::builder::information::InformationBuilder; use crate::{BasicMemoryInfoTag, BootInformation, CommandLineTag, ModuleTag}; @@ -353,7 +353,6 @@ mod tests { } #[test] - #[cfg_attr(miri, ignore)] fn test_builder() { // Step 1/2: Build MBI let mb2i_data = create_builder().build(); diff --git a/multiboot2/src/builder/mod.rs b/multiboot2/src/builder/mod.rs index 7d06c425..4fd00932 100644 --- a/multiboot2/src/builder/mod.rs +++ b/multiboot2/src/builder/mod.rs @@ -1,10 +1,7 @@ //! Module for the builder-feature. -mod boxed_dst; mod information; -// This must be public to support external people to create boxed DSTs. -pub use boxed_dst::BoxedDst; pub use information::InformationBuilder; /// Helper trait for all structs that need to be serialized that do not diff --git a/multiboot2/src/command_line.rs b/multiboot2/src/command_line.rs index 419934ff..50f4ee32 100644 --- a/multiboot2/src/command_line.rs +++ b/multiboot2/src/command_line.rs @@ -6,7 +6,7 @@ use core::fmt::{Debug, Formatter}; use core::mem; use core::str; #[cfg(feature = "builder")] -use {crate::builder::BoxedDst, alloc::vec::Vec}; +use {crate::new_boxed, alloc::boxed::Box, alloc::vec::Vec}; const METADATA_SIZE: usize = mem::size_of::(); @@ -26,13 +26,13 @@ impl CommandLineTag { /// Create a new command line tag from the given string. #[cfg(feature = "builder")] #[must_use] - pub fn new(command_line: &str) -> BoxedDst { + pub fn new(command_line: &str) -> Box { let mut bytes: Vec<_> = command_line.bytes().collect(); if !bytes.ends_with(&[0]) { // terminating null-byte bytes.push(0); } - BoxedDst::new(&bytes) + new_boxed(&bytes) } /// Reads the command line of the kernel as Rust string slice without @@ -109,11 +109,10 @@ mod tests { /// Test to generate a tag from a given string. #[test] #[cfg(feature = "builder")] - #[ignore] fn test_build_str() { let tag = CommandLineTag::new("hello"); let bytes = tag.as_bytes(); - assert_eq!(bytes, &get_bytes()[..]); + assert_eq!(bytes, &get_bytes()[..tag.size()]); assert_eq!(tag.cmdline(), Ok("hello")); // test also some bigger message diff --git a/multiboot2/src/elf_sections.rs b/multiboot2/src/elf_sections.rs index a084b434..eb70c616 100644 --- a/multiboot2/src/elf_sections.rs +++ b/multiboot2/src/elf_sections.rs @@ -1,11 +1,11 @@ //! Module for [`ElfSectionsTag`]. -#[cfg(feature = "builder")] -use crate::builder::BoxedDst; use crate::{TagHeader, TagTrait, TagType}; use core::fmt::{Debug, Formatter}; use core::mem; use core::str::Utf8Error; +#[cfg(feature = "builder")] +use {crate::new_boxed, alloc::boxed::Box}; const METADATA_SIZE: usize = mem::size_of::() + 3 * mem::size_of::(); @@ -26,12 +26,7 @@ impl ElfSectionsTag { /// Create a new ElfSectionsTag with the given data. #[cfg(feature = "builder")] #[must_use] - pub fn new( - number_of_sections: u32, - entry_size: u32, - shndx: u32, - sections: &[u8], - ) -> BoxedDst { + pub fn new(number_of_sections: u32, entry_size: u32, shndx: u32, sections: &[u8]) -> Box { let mut bytes = [ number_of_sections.to_le_bytes(), entry_size.to_le_bytes(), @@ -39,7 +34,7 @@ impl ElfSectionsTag { ] .concat(); bytes.extend_from_slice(sections); - BoxedDst::new(&bytes) + new_boxed(&bytes) } /// Get an iterator of loaded ELF sections. diff --git a/multiboot2/src/framebuffer.rs b/multiboot2/src/framebuffer.rs index a1f7cc0b..56ee4a29 100644 --- a/multiboot2/src/framebuffer.rs +++ b/multiboot2/src/framebuffer.rs @@ -7,7 +7,7 @@ use core::mem; use core::slice; use derive_more::Display; #[cfg(feature = "builder")] -use {crate::builder::AsBytes, crate::builder::BoxedDst, alloc::vec::Vec}; +use {crate::builder::AsBytes, crate::new_boxed, alloc::boxed::Box, alloc::vec::Vec}; /// Helper struct to read bytes from a raw pointer and increase the pointer /// automatically. @@ -94,14 +94,14 @@ impl FramebufferTag { height: u32, bpp: u8, buffer_type: FramebufferType, - ) -> BoxedDst { + ) -> Box { let mut bytes: Vec = address.to_le_bytes().into(); bytes.extend(pitch.to_le_bytes()); bytes.extend(width.to_le_bytes()); bytes.extend(height.to_le_bytes()); bytes.extend(bpp.to_le_bytes()); bytes.extend(buffer_type.to_bytes()); - BoxedDst::new(&bytes) + new_boxed(&bytes) } /// Contains framebuffer physical address. diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index 8fffb090..555fd833 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -298,6 +298,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn framebuffer_tag_indexed() { // indexed mode test: // this is synthetic, as I can't get QEMU @@ -1056,10 +1057,7 @@ mod tests { assert_eq!(desc.phys_start, 0x100000); assert_eq!(desc.page_count, 4); assert_eq!(desc.ty, EFIMemoryAreaType::CONVENTIONAL); - // test that the EFI memory map is not detected if the boot services - // are not exited. - struct Bytes2([u8; 80]); - let bytes2: Bytes2 = Bytes2([ + let bytes2 = AlignedBytes([ 80, 0, 0, 0, // size 0, 0, 0, 0, // reserved 17, 0, 0, 0, // EFI memory map type @@ -1081,7 +1079,7 @@ mod tests { 0, 0, 0, 0, // end tag type. 8, 0, 0, 0, // end tag size. ]); - let bi = unsafe { BootInformation::load(bytes2.0.as_ptr().cast()) }; + let bi = unsafe { BootInformation::load(bytes2.as_ptr().cast()) }; let bi = bi.unwrap(); let efi_mmap = bi.efi_memory_map_tag(); assert!(efi_mmap.is_none()); diff --git a/multiboot2/src/memory_map.rs b/multiboot2/src/memory_map.rs index fcab800e..26a5aaa6 100644 --- a/multiboot2/src/memory_map.rs +++ b/multiboot2/src/memory_map.rs @@ -11,7 +11,7 @@ use core::fmt::{Debug, Formatter}; use core::marker::PhantomData; use core::mem; #[cfg(feature = "builder")] -use {crate::builder::AsBytes, crate::builder::BoxedDst}; +use {crate::builder::AsBytes, crate::new_boxed, alloc::boxed::Box}; const METADATA_SIZE: usize = mem::size_of::() + 2 * mem::size_of::(); @@ -38,14 +38,14 @@ impl MemoryMapTag { /// Constructs a new tag. #[cfg(feature = "builder")] #[must_use] - pub fn new(areas: &[MemoryArea]) -> BoxedDst { + pub fn new(areas: &[MemoryArea]) -> Box { let entry_size: u32 = mem::size_of::().try_into().unwrap(); let entry_version: u32 = 0; let mut bytes = [entry_size.to_le_bytes(), entry_version.to_le_bytes()].concat(); for area in areas { bytes.extend(area.as_bytes()); } - BoxedDst::new(bytes.as_slice()) + new_boxed(bytes.as_slice()) } /// Returns the entry size. @@ -326,7 +326,7 @@ impl EFIMemoryMapTag { /// EFIMemoryDescs, not the ones you might have gotten from the firmware. #[cfg(feature = "builder")] #[must_use] - pub fn new_from_descs(descs: &[EFIMemoryDesc]) -> BoxedDst { + pub fn new_from_descs(descs: &[EFIMemoryDesc]) -> Box { // TODO replace this EfiMemorydesc::uefi_desc_size() in the next uefi_raw // release. @@ -354,7 +354,7 @@ impl EFIMemoryMapTag { /// Create a new EFI memory map tag from the given EFI memory map. #[cfg(feature = "builder")] #[must_use] - pub fn new_from_map(desc_size: u32, desc_version: u32, efi_mmap: &[u8]) -> BoxedDst { + pub fn new_from_map(desc_size: u32, desc_version: u32, efi_mmap: &[u8]) -> Box { assert!(desc_size > 0); assert_eq!(efi_mmap.len() % desc_size as usize, 0); assert_eq!( @@ -369,7 +369,7 @@ impl EFIMemoryMapTag { efi_mmap, ] .concat(); - BoxedDst::new(&bytes) + new_boxed(&bytes) } /// Returns an iterator over the provided memory areas. @@ -466,7 +466,7 @@ impl<'a> ExactSizeIterator for EFIMemoryAreaIter<'a> { } } -#[cfg(all(test, feature = "builder", not(miri)))] +#[cfg(all(test, feature = "builder"))] mod tests { use super::*; use std::mem::size_of; diff --git a/multiboot2/src/module.rs b/multiboot2/src/module.rs index c6748ded..65bc263a 100644 --- a/multiboot2/src/module.rs +++ b/multiboot2/src/module.rs @@ -5,7 +5,7 @@ use crate::{parse_slice_as_string, StringError, TagTrait, TagType}; use core::fmt::{Debug, Formatter}; use core::mem; #[cfg(feature = "builder")] -use {crate::builder::BoxedDst, alloc::vec::Vec}; +use {crate::new_boxed, alloc::boxed::Box, alloc::vec::Vec}; const METADATA_SIZE: usize = mem::size_of::() + 2 * mem::size_of::(); @@ -26,7 +26,7 @@ impl ModuleTag { /// Constructs a new tag. #[cfg(feature = "builder")] #[must_use] - pub fn new(start: u32, end: u32, cmdline: &str) -> BoxedDst { + pub fn new(start: u32, end: u32, cmdline: &str) -> Box { assert!(end > start, "must have a size"); let mut cmdline_bytes: Vec<_> = cmdline.bytes().collect(); @@ -38,7 +38,7 @@ impl ModuleTag { let end_bytes = end.to_le_bytes(); let mut content_bytes = [start_bytes, end_bytes].concat(); content_bytes.extend_from_slice(&cmdline_bytes); - BoxedDst::new(&content_bytes) + new_boxed(&content_bytes) } /// Reads the command line of the boot module as Rust string slice without @@ -161,11 +161,10 @@ mod tests { /// Test to generate a tag from a given string. #[test] #[cfg(feature = "builder")] - #[ignore] fn test_build_str() { let tag = ModuleTag::new(0xff00, 0xffff, "hello"); let bytes = tag.as_bytes(); - assert_eq!(bytes, &get_bytes()[..]); + assert_eq!(bytes, &get_bytes()[..tag.size()]); assert_eq!(tag.cmdline(), Ok("hello")); // test also some bigger message diff --git a/multiboot2/src/smbios.rs b/multiboot2/src/smbios.rs index 42c95aa5..0bbb6b9b 100644 --- a/multiboot2/src/smbios.rs +++ b/multiboot2/src/smbios.rs @@ -1,11 +1,11 @@ //! Module for [`SmbiosTag`]. -#[cfg(feature = "builder")] -use crate::builder::BoxedDst; use crate::tag::TagHeader; use crate::{TagTrait, TagType}; use core::fmt::Debug; use core::mem; +#[cfg(feature = "builder")] +use {crate::new_boxed, alloc::boxed::Box}; const METADATA_SIZE: usize = mem::size_of::() + mem::size_of::() * 8; @@ -24,10 +24,10 @@ impl SmbiosTag { /// Constructs a new tag. #[cfg(feature = "builder")] #[must_use] - pub fn new(major: u8, minor: u8, tables: &[u8]) -> BoxedDst { + pub fn new(major: u8, minor: u8, tables: &[u8]) -> Box { let mut bytes = [major, minor, 0, 0, 0, 0, 0, 0].to_vec(); bytes.extend(tables); - BoxedDst::new(&bytes) + new_boxed(&bytes) } /// Returns the major number. @@ -109,10 +109,9 @@ mod tests { /// Test to generate a tag. #[test] #[cfg(feature = "builder")] - #[ignore] fn test_build() { let tag = SmbiosTag::new(7, 42, &[0, 1, 2, 3, 4, 5, 6, 7, 8]); let bytes = tag.as_bytes(); - assert_eq!(bytes, &get_bytes()[..]); + assert_eq!(bytes, &get_bytes()[..tag.size()]); } } From c47b7ac40d1521c7f41d1cc5d7c625f4a72caae6 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 10:52:00 +0200 Subject: [PATCH 05/17] multiboot2: new_boxed() can be used with less allocations on callee side Note that Miri runs significantly longer with this change. More memory accesses that need to be tracked. --- multiboot2/src/boot_loader_name.rs | 14 +++--- multiboot2/src/command_line.rs | 10 ++--- multiboot2/src/elf_sections.rs | 12 ++--- multiboot2/src/framebuffer.rs | 29 ++++++------ multiboot2/src/memory_map.rs | 71 +++++++++--------------------- multiboot2/src/module.rs | 17 ++++--- multiboot2/src/smbios.rs | 5 +-- multiboot2/src/util.rs | 39 +++++++++++----- 8 files changed, 90 insertions(+), 107 deletions(-) diff --git a/multiboot2/src/boot_loader_name.rs b/multiboot2/src/boot_loader_name.rs index 62f1bc91..4603116a 100644 --- a/multiboot2/src/boot_loader_name.rs +++ b/multiboot2/src/boot_loader_name.rs @@ -2,10 +2,10 @@ use crate::tag::TagHeader; use crate::{new_boxed, parse_slice_as_string, StringError, TagTrait, TagType}; +#[cfg(feature = "builder")] +use alloc::boxed::Box; use core::fmt::{Debug, Formatter}; use core::mem; -#[cfg(feature = "builder")] -use {alloc::boxed::Box, alloc::vec::Vec}; const METADATA_SIZE: usize = mem::size_of::(); @@ -23,12 +23,12 @@ impl BootLoaderNameTag { #[cfg(feature = "builder")] #[must_use] pub fn new(name: &str) -> Box { - let mut bytes: Vec<_> = name.bytes().collect(); - if !bytes.ends_with(&[0]) { - // terminating null-byte - bytes.push(0); + let bytes = name.as_bytes(); + if bytes.ends_with(&[0]) { + new_boxed(&[bytes]) + } else { + new_boxed(&[bytes, &[0]]) } - new_boxed(&bytes) } /// Returns the underlying [`TagType`]. diff --git a/multiboot2/src/command_line.rs b/multiboot2/src/command_line.rs index 50f4ee32..432b3023 100644 --- a/multiboot2/src/command_line.rs +++ b/multiboot2/src/command_line.rs @@ -27,12 +27,12 @@ impl CommandLineTag { #[cfg(feature = "builder")] #[must_use] pub fn new(command_line: &str) -> Box { - let mut bytes: Vec<_> = command_line.bytes().collect(); - if !bytes.ends_with(&[0]) { - // terminating null-byte - bytes.push(0); + let bytes = command_line.as_bytes(); + if bytes.ends_with(&[0]) { + new_boxed(&[bytes]) + } else { + new_boxed(&[bytes, &[0]]) } - new_boxed(&bytes) } /// Reads the command line of the kernel as Rust string slice without diff --git a/multiboot2/src/elf_sections.rs b/multiboot2/src/elf_sections.rs index eb70c616..55c6e9e6 100644 --- a/multiboot2/src/elf_sections.rs +++ b/multiboot2/src/elf_sections.rs @@ -27,14 +27,10 @@ impl ElfSectionsTag { #[cfg(feature = "builder")] #[must_use] pub fn new(number_of_sections: u32, entry_size: u32, shndx: u32, sections: &[u8]) -> Box { - let mut bytes = [ - number_of_sections.to_le_bytes(), - entry_size.to_le_bytes(), - shndx.to_le_bytes(), - ] - .concat(); - bytes.extend_from_slice(sections); - new_boxed(&bytes) + let number_of_sections = number_of_sections.to_ne_bytes(); + let entry_size = entry_size.to_ne_bytes(); + let shndx = shndx.to_ne_bytes(); + new_boxed(&[&number_of_sections, &entry_size, &shndx, sections]) } /// Get an iterator of loaded ELF sections. diff --git a/multiboot2/src/framebuffer.rs b/multiboot2/src/framebuffer.rs index 56ee4a29..0083905a 100644 --- a/multiboot2/src/framebuffer.rs +++ b/multiboot2/src/framebuffer.rs @@ -95,13 +95,13 @@ impl FramebufferTag { bpp: u8, buffer_type: FramebufferType, ) -> Box { - let mut bytes: Vec = address.to_le_bytes().into(); - bytes.extend(pitch.to_le_bytes()); - bytes.extend(width.to_le_bytes()); - bytes.extend(height.to_le_bytes()); - bytes.extend(bpp.to_le_bytes()); - bytes.extend(buffer_type.to_bytes()); - new_boxed(&bytes) + let address = address.to_ne_bytes(); + let pitch = pitch.to_ne_bytes(); + let width = width.to_ne_bytes(); + let height = height.to_ne_bytes(); + let bpp = bpp.to_ne_bytes(); + let buffer_type = buffer_type.to_bytes(); + new_boxed(&[&address, &pitch, &width, &height, &bpp, &buffer_type]) } /// Contains framebuffer physical address. @@ -145,6 +145,7 @@ impl FramebufferTag { match typ { FramebufferTypeId::Indexed => { let num_colors = reader.read_u32(); + // TODO static cast looks like UB? let palette = unsafe { slice::from_raw_parts( reader.current_address() as *const FramebufferColor, @@ -274,23 +275,23 @@ impl<'a> FramebufferType<'a> { let mut v = Vec::new(); match self { FramebufferType::Indexed { palette } => { - v.extend(0u8.to_le_bytes()); // type - v.extend(0u16.to_le_bytes()); // reserved - v.extend((palette.len() as u32).to_le_bytes()); + v.extend(0u8.to_ne_bytes()); // type + v.extend(0u16.to_ne_bytes()); // reserved + v.extend((palette.len() as u32).to_ne_bytes()); for color in palette.iter() { v.extend(color.as_bytes()); } } FramebufferType::RGB { red, green, blue } => { - v.extend(1u8.to_le_bytes()); // type - v.extend(0u16.to_le_bytes()); // reserved + v.extend(1u8.to_ne_bytes()); // type + v.extend(0u16.to_ne_bytes()); // reserved v.extend(red.as_bytes()); v.extend(green.as_bytes()); v.extend(blue.as_bytes()); } FramebufferType::Text => { - v.extend(2u8.to_le_bytes()); // type - v.extend(0u16.to_le_bytes()); // reserved + v.extend(2u8.to_ne_bytes()); // type + v.extend(0u16.to_ne_bytes()); // reserved } } v diff --git a/multiboot2/src/memory_map.rs b/multiboot2/src/memory_map.rs index 26a5aaa6..3f056c92 100644 --- a/multiboot2/src/memory_map.rs +++ b/multiboot2/src/memory_map.rs @@ -11,7 +11,7 @@ use core::fmt::{Debug, Formatter}; use core::marker::PhantomData; use core::mem; #[cfg(feature = "builder")] -use {crate::builder::AsBytes, crate::new_boxed, alloc::boxed::Box}; +use {crate::new_boxed, alloc::boxed::Box, core::slice}; const METADATA_SIZE: usize = mem::size_of::() + 2 * mem::size_of::(); @@ -39,13 +39,14 @@ impl MemoryMapTag { #[cfg(feature = "builder")] #[must_use] pub fn new(areas: &[MemoryArea]) -> Box { - let entry_size: u32 = mem::size_of::().try_into().unwrap(); - let entry_version: u32 = 0; - let mut bytes = [entry_size.to_le_bytes(), entry_version.to_le_bytes()].concat(); - for area in areas { - bytes.extend(area.as_bytes()); - } - new_boxed(bytes.as_slice()) + let entry_size = mem::size_of::().to_ne_bytes(); + let entry_version = 0_u32.to_ne_bytes(); + let areas = { + let ptr = areas.as_ptr().cast::(); + let len = areas.len() * size_of::(); + unsafe { slice::from_raw_parts(ptr, len) } + }; + new_boxed(&[&entry_size, &entry_version, areas]) } /// Returns the entry size. @@ -139,9 +140,6 @@ impl Debug for MemoryArea { } } -#[cfg(feature = "builder")] -impl AsBytes for MemoryArea {} - /// ABI-friendly version of [`MemoryAreaType`]. #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] #[repr(C)] @@ -292,9 +290,6 @@ impl TagTrait for BasicMemoryInfoTag { const EFI_METADATA_SIZE: usize = mem::size_of::() + 3 * mem::size_of::(); -#[cfg(feature = "builder")] -impl AsBytes for EFIMemoryDesc {} - /// EFI memory map tag. The embedded [`EFIMemoryDesc`]s follows the EFI /// specification. #[derive(ptr_meta::Pointee, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -322,32 +317,19 @@ pub struct EFIMemoryMapTag { impl EFIMemoryMapTag { /// Create a new EFI memory map tag with the given memory descriptors. - /// Version and size can't be set because you're passing a slice of - /// EFIMemoryDescs, not the ones you might have gotten from the firmware. #[cfg(feature = "builder")] #[must_use] pub fn new_from_descs(descs: &[EFIMemoryDesc]) -> Box { - // TODO replace this EfiMemorydesc::uefi_desc_size() in the next uefi_raw - // release. - - let size_base = mem::size_of::(); - // Taken from https://github.com/tianocore/edk2/blob/7142e648416ff5d3eac6c6d607874805f5de0ca8/MdeModulePkg/Core/PiSmmCore/Page.c#L1059 - let desc_size_diff = mem::size_of::() - size_base % mem::size_of::(); - let desc_size = size_base + desc_size_diff; - - assert!(desc_size >= size_base); - - let mut efi_mmap = alloc::vec::Vec::with_capacity(descs.len() * desc_size); - for desc in descs { - efi_mmap.extend(desc.as_bytes()); - // fill with zeroes - efi_mmap.extend([0].repeat(desc_size_diff)); - } + let efi_mmap = { + let ptr = descs.as_ptr().cast::(); + let len = descs.len() * size_of::(); + unsafe { slice::from_raw_parts(ptr, len) } + }; Self::new_from_map( - desc_size as u32, + mem::size_of::() as u32, EFIMemoryDesc::VERSION, - efi_mmap.as_slice(), + efi_mmap, ) } @@ -355,21 +337,10 @@ impl EFIMemoryMapTag { #[cfg(feature = "builder")] #[must_use] pub fn new_from_map(desc_size: u32, desc_version: u32, efi_mmap: &[u8]) -> Box { - assert!(desc_size > 0); - assert_eq!(efi_mmap.len() % desc_size as usize, 0); - assert_eq!( - efi_mmap - .as_ptr() - .align_offset(mem::align_of::()), - 0 - ); - let bytes = [ - &desc_size.to_le_bytes(), - &desc_version.to_le_bytes(), - efi_mmap, - ] - .concat(); - new_boxed(&bytes) + assert_ne!(desc_size, 0); + let desc_size = desc_size.to_ne_bytes(); + let desc_version = desc_version.to_ne_bytes(); + new_boxed(&[&desc_size, &desc_version, efi_mmap]) } /// Returns an iterator over the provided memory areas. @@ -491,8 +462,6 @@ mod tests { ]; let efi_mmap_tag = EFIMemoryMapTag::new_from_descs(&descs); - assert_eq!(efi_mmap_tag.desc_size, 48 /* 40 + 8 */); - let mut iter = efi_mmap_tag.memory_areas(); assert_eq!(iter.next(), Some(&descs[0])); diff --git a/multiboot2/src/module.rs b/multiboot2/src/module.rs index 65bc263a..75910560 100644 --- a/multiboot2/src/module.rs +++ b/multiboot2/src/module.rs @@ -29,16 +29,15 @@ impl ModuleTag { pub fn new(start: u32, end: u32, cmdline: &str) -> Box { assert!(end > start, "must have a size"); - let mut cmdline_bytes: Vec<_> = cmdline.bytes().collect(); - if !cmdline_bytes.ends_with(&[0]) { - // terminating null-byte - cmdline_bytes.push(0); + let start = start.to_ne_bytes(); + let end = end.to_ne_bytes(); + let cmdline = cmdline.as_bytes(); + + if cmdline.ends_with(&[0]) { + new_boxed(&[&start, &end, cmdline]) + } else { + new_boxed(&[&start, &end, cmdline, &[0]]) } - let start_bytes = start.to_le_bytes(); - let end_bytes = end.to_le_bytes(); - let mut content_bytes = [start_bytes, end_bytes].concat(); - content_bytes.extend_from_slice(&cmdline_bytes); - new_boxed(&content_bytes) } /// Reads the command line of the boot module as Rust string slice without diff --git a/multiboot2/src/smbios.rs b/multiboot2/src/smbios.rs index 0bbb6b9b..208268ae 100644 --- a/multiboot2/src/smbios.rs +++ b/multiboot2/src/smbios.rs @@ -25,9 +25,8 @@ impl SmbiosTag { #[cfg(feature = "builder")] #[must_use] pub fn new(major: u8, minor: u8, tables: &[u8]) -> Box { - let mut bytes = [major, minor, 0, 0, 0, 0, 0, 0].to_vec(); - bytes.extend(tables); - new_boxed(&bytes) + let reserved = [0, 0, 0, 0, 0, 0]; + new_boxed(&[&[major, minor], &reserved, tables]) } /// Returns the major number. diff --git a/multiboot2/src/util.rs b/multiboot2/src/util.rs index e59b1365..8cdaa671 100644 --- a/multiboot2/src/util.rs +++ b/multiboot2/src/util.rs @@ -41,11 +41,17 @@ impl core::error::Error for StringError { /// constructor and box the result. /// /// # Parameters -/// - `additional_bytes`: All bytes apart from the default [`TagHeader`] that -/// are included into the tag. +/// - `additional_bytes_slices`: Array of byte slices that should be included +/// without additional padding in-between. You don't need to add the bytes +/// for [`TagHeader`], but only additional ones. #[cfg(feature = "alloc")] -pub fn new_boxed(additional_bytes: &[u8]) -> Box { - let size = size_of::() + additional_bytes.iter().len(); +pub fn new_boxed(additional_bytes_slices: &[&[u8]]) -> Box { + let additional_size = additional_bytes_slices + .iter() + .map(|b| b.len()) + .sum::(); + + let size = size_of::() + additional_size; let alloc_size = increase_to_alignment(size); let layout = Layout::from_size_align(alloc_size, ALIGNMENT).unwrap(); let heap_ptr = unsafe { alloc::alloc::alloc(layout) }; @@ -55,9 +61,16 @@ pub fn new_boxed(additional_bytes: &[u8]) -> Box { heap_ptr.cast::().write(T::ID.val()); heap_ptr.cast::().add(1).write(size as u32); } - unsafe { - let ptr = heap_ptr.add(size_of::()); - ptr::copy_nonoverlapping(additional_bytes.as_ptr(), ptr, additional_bytes.len()); + + let mut write_offset = size_of::(); + for &bytes in additional_bytes_slices { + unsafe { + let len = bytes.len(); + let src = bytes.as_ptr(); + let dst = heap_ptr.add(write_offset); + ptr::copy_nonoverlapping(src, dst, len); + write_offset += len; + } } let header = unsafe { heap_ptr.cast::().as_ref() }.unwrap(); @@ -128,10 +141,16 @@ mod tests { #[test] fn test_new_boxed() { - let tag = new_boxed::(&[0, 1, 2, 3]); + let tag = new_boxed::(&[&[0, 1, 2, 3]]); assert_eq!(tag.header().typ, GenericTag::ID); - {} - let tag = new_boxed::("hello\0".as_bytes()); + assert_eq!(tag.payload(), &[0, 1, 2, 3]); + + // Test that bytes are added consecutively without gaps. + let tag = new_boxed::(&[&[0], &[1], &[2, 3]]); + assert_eq!(tag.header().typ, GenericTag::ID); + assert_eq!(tag.payload(), &[0, 1, 2, 3]); + + let tag = new_boxed::(&["hello\0".as_bytes()]); assert_eq!(tag.cmdline(), Ok("hello")); } } From 4ee788458316da367d9dd1b4c300258d56a0dc90 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 10:52:01 +0200 Subject: [PATCH 06/17] multiboot2: more unit tests --- multiboot2/src/boot_loader_name.rs | 6 ++++++ multiboot2/src/command_line.rs | 6 ++++++ multiboot2/src/module.rs | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/multiboot2/src/boot_loader_name.rs b/multiboot2/src/boot_loader_name.rs index 4603116a..3fc1810a 100644 --- a/multiboot2/src/boot_loader_name.rs +++ b/multiboot2/src/boot_loader_name.rs @@ -121,6 +121,12 @@ mod tests { assert_eq!(bytes, &get_bytes()[..tag.size()]); assert_eq!(tag.name(), Ok("hello")); + // With terminating null. + let tag = BootLoaderNameTag::new("hello\0"); + let bytes = tag.as_bytes(); + assert_eq!(bytes, &get_bytes()[..tag.size()]); + assert_eq!(tag.name(), Ok("hello")); + // test also some bigger message let tag = BootLoaderNameTag::new("AbCdEfGhUjK YEAH"); assert_eq!(tag.name(), Ok("AbCdEfGhUjK YEAH")); diff --git a/multiboot2/src/command_line.rs b/multiboot2/src/command_line.rs index 432b3023..7cbfd06f 100644 --- a/multiboot2/src/command_line.rs +++ b/multiboot2/src/command_line.rs @@ -115,6 +115,12 @@ mod tests { assert_eq!(bytes, &get_bytes()[..tag.size()]); assert_eq!(tag.cmdline(), Ok("hello")); + // With terminating null. + let tag = CommandLineTag::new("hello\0"); + let bytes = tag.as_bytes(); + assert_eq!(bytes, &get_bytes()[..tag.size()]); + assert_eq!(tag.cmdline(), Ok("hello")); + // test also some bigger message let tag = CommandLineTag::new("AbCdEfGhUjK YEAH"); assert_eq!(tag.cmdline(), Ok("AbCdEfGhUjK YEAH")); diff --git a/multiboot2/src/module.rs b/multiboot2/src/module.rs index 75910560..d36ee549 100644 --- a/multiboot2/src/module.rs +++ b/multiboot2/src/module.rs @@ -166,6 +166,12 @@ mod tests { assert_eq!(bytes, &get_bytes()[..tag.size()]); assert_eq!(tag.cmdline(), Ok("hello")); + // With terminating null. + let tag = ModuleTag::new(0xff00, 0xffff, "hello\0"); + let bytes = tag.as_bytes(); + assert_eq!(bytes, &get_bytes()[..tag.size()]); + assert_eq!(tag.cmdline(), Ok("hello")); + // test also some bigger message let tag = ModuleTag::new(0, 1, "AbCdEfGhUjK YEAH"); assert_eq!(tag.cmdline(), Ok("AbCdEfGhUjK YEAH")); From 78d42b86e947c3f62e10307e5ae4ee490496165a Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 10:52:01 +0200 Subject: [PATCH 07/17] multiboot2: fix Miri issues in elf_sections() test Miri still outputs a warning tho: "warning: integer-to-pointer cast" Nevertheless, no we are UB free! --- multiboot2/src/boot_information.rs | 2 +- multiboot2/src/elf_sections.rs | 41 +++++++++++------------------- multiboot2/src/lib.rs | 4 --- 3 files changed, 16 insertions(+), 31 deletions(-) diff --git a/multiboot2/src/boot_information.rs b/multiboot2/src/boot_information.rs index f014db3c..68863af8 100644 --- a/multiboot2/src/boot_information.rs +++ b/multiboot2/src/boot_information.rs @@ -440,7 +440,7 @@ impl fmt::Debug for BootInformation<'_> { if elf_sections_tag_entries_count > ELF_SECTIONS_LIMIT { debug.field("elf_sections (count)", &elf_sections_tag_entries_count); } else { - debug.field("elf_sections", &self.elf_sections().unwrap_or_default()); + debug.field("elf_sections", &self.elf_sections()); } } diff --git a/multiboot2/src/elf_sections.rs b/multiboot2/src/elf_sections.rs index 55c6e9e6..27c4c3c1 100644 --- a/multiboot2/src/elf_sections.rs +++ b/multiboot2/src/elf_sections.rs @@ -2,6 +2,7 @@ use crate::{TagHeader, TagTrait, TagType}; use core::fmt::{Debug, Formatter}; +use core::marker::{PhantomData, PhantomPinned}; use core::mem; use core::str::Utf8Error; #[cfg(feature = "builder")] @@ -34,21 +35,18 @@ impl ElfSectionsTag { } /// Get an iterator of loaded ELF sections. - pub(crate) const fn sections(&self) -> ElfSectionIter { + pub(crate) fn sections(&self) -> ElfSectionIter { let string_section_offset = (self.shndx * self.entry_size) as isize; let string_section_ptr = - unsafe { self.first_section().offset(string_section_offset) as *const _ }; + unsafe { self.sections.as_ptr().offset(string_section_offset) as *const _ }; ElfSectionIter { - current_section: self.first_section(), + current_section: self.sections.as_ptr(), remaining_sections: self.number_of_sections, entry_size: self.entry_size, string_section: string_section_ptr, + _phantom_data: PhantomData::default(), } } - - const fn first_section(&self) -> *const u8 { - &(self.sections[0]) as *const _ - } } impl TagTrait for ElfSectionsTag { @@ -75,23 +73,24 @@ impl Debug for ElfSectionsTag { /// An iterator over some ELF sections. #[derive(Clone)] -/// TODO make this memory safe with lifetime capture. -pub struct ElfSectionIter { +pub struct ElfSectionIter<'a> { current_section: *const u8, remaining_sections: u32, entry_size: u32, string_section: *const u8, + _phantom_data: PhantomData<&'a ()>, } -impl Iterator for ElfSectionIter { - type Item = ElfSection; +impl<'a> Iterator for ElfSectionIter<'a> { + type Item = ElfSection<'a>; - fn next(&mut self) -> Option { + fn next(&mut self) -> Option> { while self.remaining_sections != 0 { let section = ElfSection { inner: self.current_section, string_section: self.string_section, entry_size: self.entry_size, + _phantom: PhantomData::default(), }; self.current_section = unsafe { self.current_section.offset(self.entry_size as isize) }; @@ -105,7 +104,7 @@ impl Iterator for ElfSectionIter { } } -impl Debug for ElfSectionIter { +impl<'a> Debug for ElfSectionIter<'a> { fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { let mut debug = f.debug_list(); self.clone().for_each(|ref e| { @@ -115,23 +114,13 @@ impl Debug for ElfSectionIter { } } -impl Default for ElfSectionIter { - fn default() -> Self { - Self { - current_section: core::ptr::null(), - remaining_sections: 0, - entry_size: 0, - string_section: core::ptr::null(), - } - } -} - /// A single generic ELF Section. #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct ElfSection { +pub struct ElfSection<'a> { inner: *const u8, string_section: *const u8, entry_size: u32, + _phantom: PhantomData<&'a ()>, } #[derive(Clone, Copy, Debug)] @@ -164,7 +153,7 @@ struct ElfSectionInner64 { entry_size: u64, } -impl ElfSection { +impl<'a> ElfSection<'a> { /// Get the section type as a `ElfSectionType` enum variant. #[must_use] pub fn section_type(&self) -> ElfSectionType { diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index 555fd833..0b2007f2 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -537,8 +537,6 @@ mod tests { /// Tests to parse a MBI that was statically extracted from a test run with /// GRUB as bootloader. #[test] - // TODO fix Miri - #[cfg_attr(miri, ignore)] fn grub2() { let mut bytes = AlignedBytes([ 192, 3, 0, 0, // total_size @@ -944,8 +942,6 @@ mod tests { } #[test] - // TODO fix Miri - #[cfg_attr(miri, ignore)] fn elf_sections() { let mut bytes = AlignedBytes([ 168, 0, 0, 0, // total_size From fb6c12b0d36040175d38ccb3b399d37407074d96 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 10:52:01 +0200 Subject: [PATCH 08/17] multiboot2: fix all warnings of rustc and clippy --- multiboot2/src/builder/information.rs | 73 +++++++++++++-------------- multiboot2/src/command_line.rs | 2 +- multiboot2/src/elf_sections.rs | 8 +-- multiboot2/src/lib.rs | 6 ++- multiboot2/src/memory_map.rs | 4 +- multiboot2/src/module.rs | 2 +- multiboot2/src/tag.rs | 16 +++--- multiboot2/src/test_util.rs | 2 +- multiboot2/src/util.rs | 21 +++++--- 9 files changed, 69 insertions(+), 65 deletions(-) diff --git a/multiboot2/src/builder/information.rs b/multiboot2/src/builder/information.rs index 5b317bf4..6806de8f 100644 --- a/multiboot2/src/builder/information.rs +++ b/multiboot2/src/builder/information.rs @@ -7,7 +7,6 @@ use crate::{ EFISdt32Tag, EFISdt64Tag, ElfSectionsTag, EndTag, FramebufferTag, ImageLoadPhysAddrTag, MemoryMapTag, ModuleTag, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagTrait, TagType, ALIGNMENT, }; -use alloc::boxed::Box; use alloc::vec::Vec; use core::fmt::{Display, Formatter}; use core::mem::size_of; @@ -200,32 +199,32 @@ impl InformationBuilder { /// Adds a 'basic memory information' tag (represented by [`BasicMemoryInfoTag`]) to the builder. #[must_use] - pub fn basic_memory_info_tag(self, tag: BasicMemoryInfoTag) -> Self { - self.add_tag(&tag).unwrap() + pub fn basic_memory_info_tag(self, tag: &BasicMemoryInfoTag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'bootloader name' tag (represented by [`BootLoaderNameTag`]) to the builder. #[must_use] - pub fn bootloader_name_tag(self, tag: Box) -> Self { - self.add_tag(&*tag).unwrap() + pub fn bootloader_name_tag(self, tag: &BootLoaderNameTag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'command line' tag (represented by [`CommandLineTag`]) to the builder. #[must_use] - pub fn command_line_tag(self, tag: Box) -> Self { - self.add_tag(&*tag).unwrap() + pub fn command_line_tag(self, tag: &CommandLineTag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'EFI 32-bit system table pointer' tag (represented by [`EFISdt32Tag`]) to the builder. #[must_use] - pub fn efisdt32_tag(self, tag: EFISdt32Tag) -> Self { - self.add_tag(&tag).unwrap() + pub fn efisdt32_tag(self, tag: &EFISdt32Tag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'EFI 64-bit system table pointer' tag (represented by [`EFISdt64Tag`]) to the builder. #[must_use] - pub fn efisdt64_tag(self, tag: EFISdt64Tag) -> Self { - self.add_tag(&tag).unwrap() + pub fn efisdt64_tag(self, tag: &EFISdt64Tag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'EFI boot services not terminated' tag (represented by [`EFIBootServicesNotExitedTag`]) to the builder. @@ -236,69 +235,69 @@ impl InformationBuilder { /// Adds a 'EFI 32-bit image handle pointer' tag (represented by [`EFIImageHandle32Tag`]) to the builder. #[must_use] - pub fn efi_image_handle32(self, tag: EFIImageHandle32Tag) -> Self { - self.add_tag(&tag).unwrap() + pub fn efi_image_handle32(self, tag: &EFIImageHandle32Tag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'EFI 64-bit image handle pointer' tag (represented by [`EFIImageHandle64Tag`]) to the builder. #[must_use] - pub fn efi_image_handle64(self, tag: EFIImageHandle64Tag) -> Self { - self.add_tag(&tag).unwrap() + pub fn efi_image_handle64(self, tag: &EFIImageHandle64Tag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'EFI Memory map' tag (represented by [`EFIMemoryMapTag`]) to the builder. #[must_use] - pub fn efi_memory_map_tag(self, tag: Box) -> Self { - self.add_tag(&*tag).unwrap() + pub fn efi_memory_map_tag(self, tag: &EFIMemoryMapTag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'ELF-Symbols' tag (represented by [`ElfSectionsTag`]) to the builder. #[must_use] - pub fn elf_sections_tag(self, tag: Box) -> Self { - self.add_tag(&*tag).unwrap() + pub fn elf_sections_tag(self, tag: &ElfSectionsTag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'Framebuffer info' tag (represented by [`FramebufferTag`]) to the builder. #[must_use] - pub fn framebuffer_tag(self, tag: Box) -> Self { - self.add_tag(&*tag).unwrap() + pub fn framebuffer_tag(self, tag: &FramebufferTag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'Image load base physical address' tag (represented by [`ImageLoadPhysAddrTag`]) to the builder. #[must_use] - pub fn image_load_addr(self, tag: ImageLoadPhysAddrTag) -> Self { - self.add_tag(&tag).unwrap() + pub fn image_load_addr(self, tag: &ImageLoadPhysAddrTag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a (*none EFI*) 'memory map' tag (represented by [`MemoryMapTag`]) to the builder. #[must_use] - pub fn memory_map_tag(self, tag: Box) -> Self { - self.add_tag(&*tag).unwrap() + pub fn memory_map_tag(self, tag: &MemoryMapTag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'Modules' tag (represented by [`ModuleTag`]) to the builder. /// This tag can occur multiple times in boot information. #[must_use] - pub fn add_module_tag(self, tag: Box) -> Self { - self.add_tag(&*tag).unwrap() + pub fn add_module_tag(self, tag: &ModuleTag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'ACPI old RSDP' tag (represented by [`RsdpV1Tag`]) to the builder. #[must_use] - pub fn rsdp_v1_tag(self, tag: RsdpV1Tag) -> Self { - self.add_tag(&tag).unwrap() + pub fn rsdp_v1_tag(self, tag: &RsdpV1Tag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'ACPI new RSDP' tag (represented by [`RsdpV2Tag`]) to the builder. #[must_use] - pub fn rsdp_v2_tag(self, tag: RsdpV2Tag) -> Self { - self.add_tag(&tag).unwrap() + pub fn rsdp_v2_tag(self, tag: &RsdpV2Tag) -> Self { + self.add_tag(tag).unwrap() } /// Adds a 'SMBIOS tables' tag (represented by [`SmbiosTag`]) to the builder. #[must_use] - pub fn smbios_tag(self, tag: Box) -> Self { - self.add_tag(&*tag).unwrap() + pub fn smbios_tag(self, tag: &SmbiosTag) -> Self { + self.add_tag(tag).unwrap() } const fn tag_is_allowed_multiple_times(tag_type: TagType) -> bool { @@ -322,18 +321,18 @@ mod tests { assert_eq!(builder.expected_len(), expected_len); // the most simple tag - builder = builder.basic_memory_info_tag(BasicMemoryInfoTag::new(640, 7 * 1024)); + builder = builder.basic_memory_info_tag(&BasicMemoryInfoTag::new(640, 7 * 1024)); expected_len += 16; assert_eq!(builder.expected_len(), expected_len); // a tag that has a dynamic size - builder = builder.command_line_tag(CommandLineTag::new("test")); + builder = builder.command_line_tag(&CommandLineTag::new("test")); expected_len += 8 + 5 + 3; // padding assert_eq!(builder.expected_len(), expected_len); // many modules - builder = builder.add_module_tag(ModuleTag::new(0, 1234, "module1")); + builder = builder.add_module_tag(&ModuleTag::new(0, 1234, "module1")); expected_len += 16 + 8; assert_eq!(builder.expected_len(), expected_len); - builder = builder.add_module_tag(ModuleTag::new(5678, 6789, "module2")); + builder = builder.add_module_tag(&ModuleTag::new(5678, 6789, "module2")); expected_len += 16 + 8; assert_eq!(builder.expected_len(), expected_len); diff --git a/multiboot2/src/command_line.rs b/multiboot2/src/command_line.rs index 7cbfd06f..fedcc06f 100644 --- a/multiboot2/src/command_line.rs +++ b/multiboot2/src/command_line.rs @@ -6,7 +6,7 @@ use core::fmt::{Debug, Formatter}; use core::mem; use core::str; #[cfg(feature = "builder")] -use {crate::new_boxed, alloc::boxed::Box, alloc::vec::Vec}; +use {crate::new_boxed, alloc::boxed::Box}; const METADATA_SIZE: usize = mem::size_of::(); diff --git a/multiboot2/src/elf_sections.rs b/multiboot2/src/elf_sections.rs index 27c4c3c1..842da777 100644 --- a/multiboot2/src/elf_sections.rs +++ b/multiboot2/src/elf_sections.rs @@ -2,7 +2,7 @@ use crate::{TagHeader, TagTrait, TagType}; use core::fmt::{Debug, Formatter}; -use core::marker::{PhantomData, PhantomPinned}; +use core::marker::PhantomData; use core::mem; use core::str::Utf8Error; #[cfg(feature = "builder")] @@ -35,7 +35,7 @@ impl ElfSectionsTag { } /// Get an iterator of loaded ELF sections. - pub(crate) fn sections(&self) -> ElfSectionIter { + pub(crate) const fn sections(&self) -> ElfSectionIter { let string_section_offset = (self.shndx * self.entry_size) as isize; let string_section_ptr = unsafe { self.sections.as_ptr().offset(string_section_offset) as *const _ }; @@ -44,7 +44,7 @@ impl ElfSectionsTag { remaining_sections: self.number_of_sections, entry_size: self.entry_size, string_section: string_section_ptr, - _phantom_data: PhantomData::default(), + _phantom_data: PhantomData, } } } @@ -90,7 +90,7 @@ impl<'a> Iterator for ElfSectionIter<'a> { inner: self.current_section, string_section: self.string_section, entry_size: self.entry_size, - _phantom: PhantomData::default(), + _phantom: PhantomData, }; self.current_section = unsafe { self.current_section.offset(self.entry_size as isize) }; diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index 0b2007f2..96d4adf2 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -12,7 +12,7 @@ // now allow a few rules which are denied by the above statement // --> They are either ridiculous, not necessary, or we can't fix them. #![allow(clippy::multiple_crate_versions)] -// #![deny(missing_docs)] +#![deny(missing_docs)] #![deny(missing_debug_implementations)] #![deny(rustdoc::all)] // --- END STYLE CHECKS --- @@ -100,7 +100,9 @@ pub use smbios::SmbiosTag; pub use tag::TagHeader; pub use tag_trait::TagTrait; pub use tag_type::{TagType, TagTypeId}; -pub use util::{new_boxed, parse_slice_as_string, StringError}; +#[cfg(feature = "alloc")] +pub use util::new_boxed; +pub use util::{parse_slice_as_string, StringError}; pub use vbe_info::{ VBECapabilities, VBEControlInfo, VBEDirectColorAttributes, VBEField, VBEInfoTag, VBEMemoryModel, VBEModeAttributes, VBEModeInfo, VBEWindowAttributes, diff --git a/multiboot2/src/memory_map.rs b/multiboot2/src/memory_map.rs index 3f056c92..c7663d72 100644 --- a/multiboot2/src/memory_map.rs +++ b/multiboot2/src/memory_map.rs @@ -43,7 +43,7 @@ impl MemoryMapTag { let entry_version = 0_u32.to_ne_bytes(); let areas = { let ptr = areas.as_ptr().cast::(); - let len = areas.len() * size_of::(); + let len = mem::size_of_val(areas); unsafe { slice::from_raw_parts(ptr, len) } }; new_boxed(&[&entry_size, &entry_version, areas]) @@ -322,7 +322,7 @@ impl EFIMemoryMapTag { pub fn new_from_descs(descs: &[EFIMemoryDesc]) -> Box { let efi_mmap = { let ptr = descs.as_ptr().cast::(); - let len = descs.len() * size_of::(); + let len = mem::size_of_val(descs); unsafe { slice::from_raw_parts(ptr, len) } }; diff --git a/multiboot2/src/module.rs b/multiboot2/src/module.rs index d36ee549..c3c79aa5 100644 --- a/multiboot2/src/module.rs +++ b/multiboot2/src/module.rs @@ -5,7 +5,7 @@ use crate::{parse_slice_as_string, StringError, TagTrait, TagType}; use core::fmt::{Debug, Formatter}; use core::mem; #[cfg(feature = "builder")] -use {crate::new_boxed, alloc::boxed::Box, alloc::vec::Vec}; +use {crate::new_boxed, alloc::boxed::Box}; const METADATA_SIZE: usize = mem::size_of::() + 2 * mem::size_of::(); diff --git a/multiboot2/src/tag.rs b/multiboot2/src/tag.rs index 3336f145..24242c02 100644 --- a/multiboot2/src/tag.rs +++ b/multiboot2/src/tag.rs @@ -57,7 +57,7 @@ impl TagHeader { /// is [`ALIGNMENT`]-aligned #[derive(Clone, Debug, PartialEq, Eq)] #[repr(transparent)] -pub(crate) struct TagBytesRef<'a>(&'a [u8]); +pub struct TagBytesRef<'a>(&'a [u8]); impl<'a> TryFrom<&'a [u8]> for TagBytesRef<'a> { type Error = MemoryError; @@ -124,18 +124,16 @@ impl GenericTag { let dst_len = Self::dst_len(header); assert_eq!(header.size as usize, Self::BASE_SIZE + dst_len); - let generic_tag: *const GenericTag = - ptr_meta::from_raw_parts(bytes.as_ptr().cast(), dst_len); - let generic_tag = unsafe { &*generic_tag }; - - generic_tag + let generic_tag: *const Self = ptr_meta::from_raw_parts(bytes.as_ptr().cast(), dst_len); + unsafe { &*generic_tag } } - pub fn header(&self) -> &TagHeader { + pub const fn header(&self) -> &TagHeader { &self.header } - pub fn payload(&self) -> &[u8] { + #[cfg(all(test, feature = "builder"))] + pub const fn payload(&self) -> &[u8] { &self.payload } @@ -346,7 +344,7 @@ mod tests { // Guaranteed wrong alignment let unaligned_slice = &slice[3..]; assert_eq!( - TagBytesRef::try_from(&unaligned_slice[..]), + TagBytesRef::try_from(unaligned_slice), Err(MemoryError::WrongAlignment) ); diff --git a/multiboot2/src/test_util.rs b/multiboot2/src/test_util.rs index a56156d0..9bf466e8 100644 --- a/multiboot2/src/test_util.rs +++ b/multiboot2/src/test_util.rs @@ -9,7 +9,7 @@ use core::ops::Deref; /// information or just the bytes for simple tags, in a manual and raw approach. #[cfg(test)] #[repr(C, align(8))] -pub(crate) struct AlignedBytes(pub [u8; N]); +pub struct AlignedBytes(pub [u8; N]); impl AlignedBytes { pub const fn new(bytes: [u8; N]) -> Self { diff --git a/multiboot2/src/util.rs b/multiboot2/src/util.rs index 8cdaa671..c9ed3fa6 100644 --- a/multiboot2/src/util.rs +++ b/multiboot2/src/util.rs @@ -1,11 +1,14 @@ //! Various utilities. -use crate::tag::GenericTag; -use crate::{TagHeader, TagTrait, TagType, ALIGNMENT}; +use crate::ALIGNMENT; use core::fmt; use core::fmt::{Display, Formatter}; use core::str::Utf8Error; -use core::{ptr, slice}; +#[cfg(feature = "alloc")] +use { + crate::{TagHeader, TagTrait}, + core::{mem, ptr}, +}; #[cfg(feature = "builder")] use {alloc::alloc::Layout, alloc::boxed::Box}; @@ -45,13 +48,14 @@ impl core::error::Error for StringError { /// without additional padding in-between. You don't need to add the bytes /// for [`TagHeader`], but only additional ones. #[cfg(feature = "alloc")] +#[must_use] pub fn new_boxed(additional_bytes_slices: &[&[u8]]) -> Box { let additional_size = additional_bytes_slices .iter() .map(|b| b.len()) .sum::(); - let size = size_of::() + additional_size; + let size = mem::size_of::() + additional_size; let alloc_size = increase_to_alignment(size); let layout = Layout::from_size_align(alloc_size, ALIGNMENT).unwrap(); let heap_ptr = unsafe { alloc::alloc::alloc(layout) }; @@ -62,7 +66,7 @@ pub fn new_boxed(additional_bytes_slices: &[&[u8]]) -> Box heap_ptr.cast::().add(1).write(size as u32); } - let mut write_offset = size_of::(); + let mut write_offset = mem::size_of::(); for &bytes in additional_bytes_slices { unsafe { let len = bytes.len(); @@ -101,8 +105,8 @@ pub const fn increase_to_alignment(size: usize) -> usize { #[cfg(test)] mod tests { use super::*; - use crate::tag::GenericTag; - use crate::CommandLineTag; + #[cfg(feature = "alloc")] + use {crate::tag::GenericTag, crate::CommandLineTag}; #[test] fn test_parse_slice_as_string() { @@ -140,6 +144,7 @@ mod tests { } #[test] + #[cfg(feature = "alloc")] fn test_new_boxed() { let tag = new_boxed::(&[&[0, 1, 2, 3]]); assert_eq!(tag.header().typ, GenericTag::ID); @@ -150,7 +155,7 @@ mod tests { assert_eq!(tag.header().typ, GenericTag::ID); assert_eq!(tag.payload(), &[0, 1, 2, 3]); - let tag = new_boxed::(&["hello\0".as_bytes()]); + let tag = new_boxed::(&[b"hello\0"]); assert_eq!(tag.cmdline(), Ok("hello")); } } From 41cbe18ecf749d0ba9adbfa074e54d82a4d2cc06 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 11:00:16 +0200 Subject: [PATCH 09/17] multiboot2: tiny code improvement --- .../bins/multiboot2_chainloader/src/loader.rs | 8 ++++---- multiboot2/src/boot_loader_name.rs | 3 ++- multiboot2/src/command_line.rs | 3 ++- multiboot2/src/module.rs | 3 ++- multiboot2/src/smbios.rs | 3 ++- multiboot2/src/tag.rs | 13 +++++++------ multiboot2/src/test_util.rs | 2 +- 7 files changed, 20 insertions(+), 15 deletions(-) diff --git a/integration-test/bins/multiboot2_chainloader/src/loader.rs b/integration-test/bins/multiboot2_chainloader/src/loader.rs index 896cf2af..3668595e 100644 --- a/integration-test/bins/multiboot2_chainloader/src/loader.rs +++ b/integration-test/bins/multiboot2_chainloader/src/loader.rs @@ -44,15 +44,15 @@ pub fn load_module(mut modules: multiboot::information::ModuleIter) -> ! { // build MBI let mbi = multiboot2::builder::InformationBuilder::new() - .bootloader_name_tag(BootLoaderNameTag::new("mb2_integrationtest_chainloader")) - .command_line_tag(CommandLineTag::new("chainloaded YEAH")) + .bootloader_name_tag(&BootLoaderNameTag::new("mb2_integrationtest_chainloader")) + .command_line_tag(&CommandLineTag::new("chainloaded YEAH")) // random non-sense memory map - .memory_map_tag(MemoryMapTag::new(&[MemoryArea::new( + .memory_map_tag(&MemoryMapTag::new(&[MemoryArea::new( 0, 0xffffffff, MemoryAreaType::Reserved, )])) - .add_module_tag(ModuleTag::new( + .add_module_tag(&ModuleTag::new( elf_mod.start as u32, elf_mod.end as u32, elf_mod.string.unwrap(), diff --git a/multiboot2/src/boot_loader_name.rs b/multiboot2/src/boot_loader_name.rs index 3fc1810a..32db2cfc 100644 --- a/multiboot2/src/boot_loader_name.rs +++ b/multiboot2/src/boot_loader_name.rs @@ -89,6 +89,7 @@ mod tests { use super::*; use crate::tag::{GenericTag, TagBytesRef}; use crate::test_util::AlignedBytes; + use core::borrow::Borrow; #[rustfmt::skip] fn get_bytes() -> AlignedBytes<16> { @@ -105,7 +106,7 @@ mod tests { #[test] fn test_parse_str() { let bytes = get_bytes(); - let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let bytes = TagBytesRef::try_from(bytes.borrow()).unwrap(); let tag = GenericTag::ref_from(bytes); let tag = tag.cast::(); assert_eq!(tag.header.typ, TagType::BootLoaderName); diff --git a/multiboot2/src/command_line.rs b/multiboot2/src/command_line.rs index fedcc06f..cd387f25 100644 --- a/multiboot2/src/command_line.rs +++ b/multiboot2/src/command_line.rs @@ -83,6 +83,7 @@ mod tests { use super::*; use crate::tag::{GenericTag, TagBytesRef}; use crate::test_util::AlignedBytes; + use core::borrow::Borrow; #[rustfmt::skip] fn get_bytes() -> AlignedBytes<16> { @@ -99,7 +100,7 @@ mod tests { #[test] fn test_parse_str() { let bytes = get_bytes(); - let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let bytes = TagBytesRef::try_from(bytes.borrow()).unwrap(); let tag = GenericTag::ref_from(bytes); let tag = tag.cast::(); assert_eq!(tag.header.typ, TagType::Cmdline); diff --git a/multiboot2/src/module.rs b/multiboot2/src/module.rs index c3c79aa5..c77eb7e7 100644 --- a/multiboot2/src/module.rs +++ b/multiboot2/src/module.rs @@ -130,6 +130,7 @@ mod tests { use super::*; use crate::tag::{GenericTag, TagBytesRef}; use crate::test_util::AlignedBytes; + use core::borrow::Borrow; #[rustfmt::skip] fn get_bytes() -> AlignedBytes<24> { @@ -150,7 +151,7 @@ mod tests { #[test] fn test_parse_str() { let bytes = get_bytes(); - let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let bytes = TagBytesRef::try_from(bytes.borrow()).unwrap(); let tag = GenericTag::ref_from(bytes); let tag = tag.cast::(); assert_eq!(tag.header.typ, TagType::Module); diff --git a/multiboot2/src/smbios.rs b/multiboot2/src/smbios.rs index 208268ae..abd0e842 100644 --- a/multiboot2/src/smbios.rs +++ b/multiboot2/src/smbios.rs @@ -73,6 +73,7 @@ mod tests { use super::*; use crate::tag::{GenericTag, TagBytesRef}; use crate::test_util::AlignedBytes; + use core::borrow::Borrow; #[rustfmt::skip] fn get_bytes() -> AlignedBytes<32> { @@ -96,7 +97,7 @@ mod tests { #[test] fn test_parse() { let bytes = get_bytes(); - let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let bytes = TagBytesRef::try_from(bytes.borrow()).unwrap(); let tag = GenericTag::ref_from(bytes); let tag = tag.cast::(); assert_eq!(tag.header.typ, TagType::Smbios); diff --git a/multiboot2/src/tag.rs b/multiboot2/src/tag.rs index 24242c02..8463e1dd 100644 --- a/multiboot2/src/tag.rs +++ b/multiboot2/src/tag.rs @@ -236,6 +236,7 @@ impl<'a> Iterator for TagIter<'a> { mod tests { use super::*; use crate::test_util::AlignedBytes; + use core::borrow::Borrow; use core::mem; #[test] @@ -248,7 +249,7 @@ mod tests { 0x13, 0x37, 0x13, 0x37, ]); - let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let bytes = TagBytesRef::try_from(bytes.borrow()).unwrap(); let tag = GenericTag::ref_from(bytes); assert_eq!(tag.header.typ, 0xffff_ffff); assert_eq!(tag.header.size, 16); @@ -277,7 +278,7 @@ mod tests { 0xef, 0xbe, 0xad, 0xde, /* field b: 0x1337_1337 */ 0x37, 0x13, 0x37, 0x13, ]); - let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let bytes = TagBytesRef::try_from(bytes.borrow()).unwrap(); let tag = GenericTag::ref_from(bytes); let custom_tag = tag.cast::(); @@ -313,7 +314,7 @@ mod tests { 0x37, 0x13, 0x37, 0x13, ]); - let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let bytes = TagBytesRef::try_from(bytes.borrow()).unwrap(); let tag = GenericTag::ref_from(bytes); let custom_tag = tag.cast::(); @@ -369,7 +370,7 @@ mod tests { 0, 0, 0, 0, 0, 0 ], ); - let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let bytes = TagBytesRef::try_from(bytes.borrow()).unwrap(); let tag = GenericTag::ref_from(bytes); assert_eq!(tag.header.typ, TagType::Cmdline); assert_eq!(tag.header.size, 8 + 10); @@ -391,7 +392,7 @@ mod tests { 0, 0, 0, 0, 0, 0 ], ); - let bytes = TagBytesRef::try_from(&bytes[..]).unwrap(); + let bytes = TagBytesRef::try_from(bytes.borrow()).unwrap(); let tag = GenericTag::ref_from(bytes); // Main objective here is also that this test passes Miri. @@ -419,7 +420,7 @@ mod tests { 8, 0, 0, 0, ], ); - let mut iter = TagIter::new(&bytes[..]); + let mut iter = TagIter::new(bytes.borrow()); let first = iter.next().unwrap(); assert_eq!(first.header.typ, TagType::Custom(0xff)); assert_eq!(first.header.size, 8); diff --git a/multiboot2/src/test_util.rs b/multiboot2/src/test_util.rs index 9bf466e8..2a983dc9 100644 --- a/multiboot2/src/test_util.rs +++ b/multiboot2/src/test_util.rs @@ -82,6 +82,6 @@ mod tests { 0, 0, ] ); - let _a = TagBytesRef::try_from(&bytes[..]).unwrap(); + let _a = TagBytesRef::try_from(bytes.borrow()).unwrap(); } } From 8d39ec8ef1782b401c3a534713087f490bc24c39 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 11:02:20 +0200 Subject: [PATCH 10/17] multiboot2: doc fixes --- multiboot2/src/boot_information.rs | 4 +++- multiboot2/src/tag.rs | 3 ++- multiboot2/src/tag_type.rs | 4 ++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/multiboot2/src/boot_information.rs b/multiboot2/src/boot_information.rs index 68863af8..6ee085d6 100644 --- a/multiboot2/src/boot_information.rs +++ b/multiboot2/src/boot_information.rs @@ -8,7 +8,7 @@ use crate::{ module, BasicMemoryInfoTag, BootLoaderNameTag, CommandLineTag, EFIBootServicesNotExitedTag, EFIImageHandle32Tag, EFIImageHandle64Tag, EFIMemoryMapTag, EFISdt32Tag, EFISdt64Tag, ElfSectionIter, ElfSectionsTag, EndTag, FramebufferTag, ImageLoadPhysAddrTag, MemoryMapTag, - ModuleIter, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagTrait, VBEInfoTag, + ModuleIter, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagTrait, TagType, VBEInfoTag, }; use core::fmt; use core::mem; @@ -221,6 +221,8 @@ impl<'a> BootInformation<'a> { /// Otherwise, if the [`TagType::EfiBs`] tag is present, this returns `None` /// as it is strictly recommended to get the memory map from the `uefi` /// services. + /// + /// [`TagType::EfiBs`]: crate::TagType::EfiBs #[must_use] pub fn efi_memory_map_tag(&self) -> Option<&EFIMemoryMapTag> { // If the EFIBootServicesNotExited is present, then we should not use diff --git a/multiboot2/src/tag.rs b/multiboot2/src/tag.rs index 8463e1dd..3943456d 100644 --- a/multiboot2/src/tag.rs +++ b/multiboot2/src/tag.rs @@ -17,10 +17,11 @@ use core::ops::Deref; use core::ptr; /// The common header that all tags have in common. This type is ABI compatible. -/// It is the sized counterpart of [`GenericTag`]. /// /// Not to be confused with Multiboot header tags, which are something /// different. +/// +/// It is the sized counterpart of `GenericTag`, an internal type. #[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] #[repr(C, align(8))] // Alignment also propagates to all tag types using this. pub struct TagHeader { diff --git a/multiboot2/src/tag_type.rs b/multiboot2/src/tag_type.rs index 9c47c089..f7ae50df 100644 --- a/multiboot2/src/tag_type.rs +++ b/multiboot2/src/tag_type.rs @@ -7,10 +7,10 @@ use core::hash::Hash; /// Serialized form of [`TagType`] that matches the binary representation /// (`u32`). The abstraction corresponds to the `typ`/`type` field of a -/// Multiboot2 [`Tag`]. This type can easily be created from or converted to +/// Multiboot2 [`TagHeader`]. This type can easily be created from or converted to /// [`TagType`]. /// -/// [`Tag`]: crate::Tag +/// [`TagHeader`]: crate::TagHeader #[repr(transparent)] #[derive(Copy, Clone, PartialOrd, PartialEq, Eq, Ord, Hash)] pub struct TagTypeId(u32); From 4edcf47fde55aa030b70cd4bd7481b3f91f8c8f9 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 11:38:13 +0200 Subject: [PATCH 11/17] multiboot2: streamline tags - all public fields -> methods - Added missing InformationBuilder support for VBEInfoTag --- multiboot2/src/boot_information.rs | 8 ++- multiboot2/src/boot_loader_name.rs | 6 +- multiboot2/src/builder/information.rs | 10 +++- multiboot2/src/elf_sections.rs | 27 +++++++-- multiboot2/src/lib.rs | 74 ++++++++++++------------- multiboot2/src/vbe_info.rs | 80 +++++++++++++++++++++------ 6 files changed, 139 insertions(+), 66 deletions(-) diff --git a/multiboot2/src/boot_information.rs b/multiboot2/src/boot_information.rs index 6ee085d6..9cd2485e 100644 --- a/multiboot2/src/boot_information.rs +++ b/multiboot2/src/boot_information.rs @@ -8,7 +8,7 @@ use crate::{ module, BasicMemoryInfoTag, BootLoaderNameTag, CommandLineTag, EFIBootServicesNotExitedTag, EFIImageHandle32Tag, EFIImageHandle64Tag, EFIMemoryMapTag, EFISdt32Tag, EFISdt64Tag, ElfSectionIter, ElfSectionsTag, EndTag, FramebufferTag, ImageLoadPhysAddrTag, MemoryMapTag, - ModuleIter, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagTrait, TagType, VBEInfoTag, + ModuleIter, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagTrait, VBEInfoTag, }; use core::fmt; use core::mem; @@ -279,8 +279,8 @@ impl<'a> BootInformation<'a> { pub fn elf_sections(&self) -> Option { let tag = self.get_tag::(); tag.map(|t| { - assert!((t.entry_size * t.shndx) <= t.size() as u32); - t.sections() + assert!((t.entry_size() * t.shndx()) <= t.size() as u32); + t.sections_iter() }) } @@ -397,6 +397,8 @@ impl<'a> BootInformation<'a> { /// .unwrap(); /// assert_eq!(tag.name(), Ok("name")); /// ``` + /// + /// [`TagType`]: crate::TagType #[must_use] pub fn get_tag(&'a self) -> Option<&'a TagT> { self.tags() diff --git a/multiboot2/src/boot_loader_name.rs b/multiboot2/src/boot_loader_name.rs index 32db2cfc..60a2cc74 100644 --- a/multiboot2/src/boot_loader_name.rs +++ b/multiboot2/src/boot_loader_name.rs @@ -1,11 +1,11 @@ //! Module for [`BootLoaderNameTag`]. use crate::tag::TagHeader; -use crate::{new_boxed, parse_slice_as_string, StringError, TagTrait, TagType}; -#[cfg(feature = "builder")] -use alloc::boxed::Box; +use crate::{parse_slice_as_string, StringError, TagTrait, TagType}; use core::fmt::{Debug, Formatter}; use core::mem; +#[cfg(feature = "builder")] +use {crate::new_boxed, alloc::boxed::Box}; const METADATA_SIZE: usize = mem::size_of::(); diff --git a/multiboot2/src/builder/information.rs b/multiboot2/src/builder/information.rs index 6806de8f..a52a48da 100644 --- a/multiboot2/src/builder/information.rs +++ b/multiboot2/src/builder/information.rs @@ -5,7 +5,8 @@ use crate::{ BasicMemoryInfoTag, BootInformationHeader, BootLoaderNameTag, CommandLineTag, EFIBootServicesNotExitedTag, EFIImageHandle32Tag, EFIImageHandle64Tag, EFIMemoryMapTag, EFISdt32Tag, EFISdt64Tag, ElfSectionsTag, EndTag, FramebufferTag, ImageLoadPhysAddrTag, - MemoryMapTag, ModuleTag, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagTrait, TagType, ALIGNMENT, + MemoryMapTag, ModuleTag, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagTrait, TagType, VBEInfoTag, + ALIGNMENT, }; use alloc::vec::Vec; use core::fmt::{Display, Formatter}; @@ -300,6 +301,13 @@ impl InformationBuilder { self.add_tag(tag).unwrap() } + /// Adds a 'VBE Info' tag (represented by [`VBEInfoTag`]) to the builder. + #[must_use] + pub fn vbe_info_tag(self, tag: &VBEInfoTag) -> Self { + self.add_tag(tag).unwrap() + } + + #[must_use] const fn tag_is_allowed_multiple_times(tag_type: TagType) -> bool { matches!( tag_type, diff --git a/multiboot2/src/elf_sections.rs b/multiboot2/src/elf_sections.rs index 842da777..decbe734 100644 --- a/multiboot2/src/elf_sections.rs +++ b/multiboot2/src/elf_sections.rs @@ -18,8 +18,8 @@ const METADATA_SIZE: usize = mem::size_of::() + 3 * mem::size_of:: ElfSectionIter { + #[must_use] + pub(crate) const fn sections_iter(&self) -> ElfSectionIter { let string_section_offset = (self.shndx * self.entry_size) as isize; let string_section_ptr = unsafe { self.sections.as_ptr().offset(string_section_offset) as *const _ }; @@ -47,6 +48,24 @@ impl ElfSectionsTag { _phantom_data: PhantomData, } } + + /// Returns the amount of sections. + #[must_use] + pub const fn number_of_sections(&self) -> u32 { + self.number_of_sections + } + + /// Returns the size of each entry. + #[must_use] + pub const fn entry_size(&self) -> u32 { + self.entry_size + } + + /// Returns the index of the section header string table. + #[must_use] + pub const fn shndx(&self) -> u32 { + self.shndx + } } impl TagTrait for ElfSectionsTag { @@ -66,7 +85,7 @@ impl Debug for ElfSectionsTag { .field("number_of_sections", &self.number_of_sections) .field("entry_size", &self.entry_size) .field("shndx", &self.shndx) - .field("sections", &self.sections()) + .field("sections", &self.sections_iter()) .finish() } } diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index 96d4adf2..90f6b6c6 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -448,83 +448,83 @@ mod tests { let vbe = bi.vbe_info_tag().unwrap(); use vbe_info::*; - assert_eq!({ vbe.mode }, 16762); - assert_eq!({ vbe.interface_segment }, 65535); - assert_eq!({ vbe.interface_offset }, 24576); - assert_eq!({ vbe.interface_length }, 79); - assert_eq!({ vbe.control_info.signature }, [86, 69, 83, 65]); - assert_eq!({ vbe.control_info.version }, 768); - assert_eq!({ vbe.control_info.oem_string_ptr }, 3221247964); + assert_eq!({ vbe.mode() }, 16762); + assert_eq!({ vbe.interface_segment() }, 65535); + assert_eq!({ vbe.interface_offset() }, 24576); + assert_eq!({ vbe.interface_length() }, 79); + assert_eq!({ vbe.control_info().signature }, [86, 69, 83, 65]); + assert_eq!({ vbe.control_info().version }, 768); + assert_eq!({ vbe.control_info().oem_string_ptr }, 3221247964); assert_eq!( - { vbe.control_info.capabilities }, + { vbe.control_info().capabilities }, VBECapabilities::SWITCHABLE_DAC ); - assert_eq!({ vbe.control_info.mode_list_ptr }, 1610645538); - assert_eq!({ vbe.control_info.total_memory }, 256); - assert_eq!({ vbe.control_info.oem_software_revision }, 0); - assert_eq!({ vbe.control_info.oem_vendor_name_ptr }, 3221247984); - assert_eq!({ vbe.control_info.oem_product_name_ptr }, 3221248003); - assert_eq!({ vbe.control_info.oem_product_revision_ptr }, 3221248023); - assert!({ vbe.mode_info.mode_attributes }.contains( + assert_eq!({ vbe.control_info().mode_list_ptr }, 1610645538); + assert_eq!({ vbe.control_info().total_memory }, 256); + assert_eq!({ vbe.control_info().oem_software_revision }, 0); + assert_eq!({ vbe.control_info().oem_vendor_name_ptr }, 3221247984); + assert_eq!({ vbe.control_info().oem_product_name_ptr }, 3221248003); + assert_eq!({ vbe.control_info().oem_product_revision_ptr }, 3221248023); + assert!({ vbe.mode_info().mode_attributes }.contains( VBEModeAttributes::SUPPORTED | VBEModeAttributes::COLOR | VBEModeAttributes::GRAPHICS | VBEModeAttributes::NOT_VGA_COMPATIBLE | VBEModeAttributes::LINEAR_FRAMEBUFFER )); - assert!(vbe.mode_info.window_a_attributes.contains( + assert!(vbe.mode_info().window_a_attributes.contains( VBEWindowAttributes::RELOCATABLE | VBEWindowAttributes::READABLE | VBEWindowAttributes::WRITEABLE )); - assert_eq!({ vbe.mode_info.window_granularity }, 64); - assert_eq!({ vbe.mode_info.window_size }, 64); - assert_eq!({ vbe.mode_info.window_a_segment }, 40960); - assert_eq!({ vbe.mode_info.window_function_ptr }, 3221247162); - assert_eq!({ vbe.mode_info.pitch }, 5120); - assert_eq!({ vbe.mode_info.resolution }, (1280, 800)); - assert_eq!(vbe.mode_info.character_size, (8, 16)); - assert_eq!(vbe.mode_info.number_of_planes, 1); - assert_eq!(vbe.mode_info.bpp, 32); - assert_eq!(vbe.mode_info.number_of_banks, 1); - assert_eq!(vbe.mode_info.memory_model, VBEMemoryModel::DirectColor); - assert_eq!(vbe.mode_info.bank_size, 0); - assert_eq!(vbe.mode_info.number_of_image_pages, 3); + assert_eq!({ vbe.mode_info().window_granularity }, 64); + assert_eq!({ vbe.mode_info().window_size }, 64); + assert_eq!({ vbe.mode_info().window_a_segment }, 40960); + assert_eq!({ vbe.mode_info().window_function_ptr }, 3221247162); + assert_eq!({ vbe.mode_info().pitch }, 5120); + assert_eq!({ vbe.mode_info().resolution }, (1280, 800)); + assert_eq!(vbe.mode_info().character_size, (8, 16)); + assert_eq!(vbe.mode_info().number_of_planes, 1); + assert_eq!(vbe.mode_info().bpp, 32); + assert_eq!(vbe.mode_info().number_of_banks, 1); + assert_eq!(vbe.mode_info().memory_model, VBEMemoryModel::DirectColor); + assert_eq!(vbe.mode_info().bank_size, 0); + assert_eq!(vbe.mode_info().number_of_image_pages, 3); assert_eq!( - vbe.mode_info.red_field, + vbe.mode_info().red_field, VBEField { position: 16, size: 8, } ); assert_eq!( - vbe.mode_info.green_field, + vbe.mode_info().green_field, VBEField { position: 8, size: 8, } ); assert_eq!( - vbe.mode_info.blue_field, + vbe.mode_info().blue_field, VBEField { position: 0, size: 8, } ); assert_eq!( - vbe.mode_info.reserved_field, + vbe.mode_info().reserved_field, VBEField { position: 24, size: 8, } ); assert_eq!( - vbe.mode_info.direct_color_attributes, + vbe.mode_info().direct_color_attributes, VBEDirectColorAttributes::RESERVED_USABLE ); - assert_eq!({ vbe.mode_info.framebuffer_base_ptr }, 4244635648); - assert_eq!({ vbe.mode_info.offscreen_memory_offset }, 0); - assert_eq!({ vbe.mode_info.offscreen_memory_size }, 0); + assert_eq!({ vbe.mode_info().framebuffer_base_ptr }, 4244635648); + assert_eq!({ vbe.mode_info().offscreen_memory_offset }, 0); + assert_eq!({ vbe.mode_info().offscreen_memory_size }, 0); } #[test] diff --git a/multiboot2/src/vbe_info.rs b/multiboot2/src/vbe_info.rs index 961cc883..8670c0ae 100644 --- a/multiboot2/src/vbe_info.rs +++ b/multiboot2/src/vbe_info.rs @@ -1,42 +1,86 @@ //! Module for [`VBEInfoTag`]. -use crate::{TagHeader, TagTrait, TagType, TagTypeId}; +use crate::{TagHeader, TagTrait, TagType}; use core::fmt; +use core::mem; /// This tag contains VBE metadata, VBE controller information returned by the /// VBE Function 00h and VBE mode information returned by the VBE Function 01h. #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] #[repr(C, align(8))] pub struct VBEInfoTag { - typ: TagTypeId, - length: u32, + header: TagHeader, + mode: u16, + interface_segment: u16, + interface_offset: u16, + interface_length: u16, + control_info: VBEControlInfo, + mode_info: VBEModeInfo, +} + +impl VBEInfoTag { + /// Constructs a new tag. + #[cfg(feature = "builder")] + #[must_use] + pub fn new( + mode: u16, + interface_segment: u16, + interface_offset: u16, + interface_length: u16, + control_info: VBEControlInfo, + mode_info: VBEModeInfo, + ) -> Self { + Self { + header: TagHeader::new(Self::ID, mem::size_of::().try_into().unwrap()), + mode, + interface_segment, + interface_offset, + interface_length, + control_info, + mode_info, + } + } /// Indicates current video mode in the format specified in VBE 3.0. - pub mode: u16, + #[must_use] + pub const fn mode(&self) -> u16 { + self.mode + } - /// Contain the segment of the table of a protected mode interface defined in VBE 2.0+. + /// Returns the segment of the table of a protected mode interface defined in VBE 2.0+. /// /// If the information for a protected mode interface is not available /// this field is set to zero. - pub interface_segment: u16, - - /// Contain the segment offset of the table of a protected mode interface defined in VBE 2.0+. + #[must_use] + pub const fn interface_segment(&self) -> u16 { + self.interface_segment + } + /// Returns the segment offset of the table of a protected mode interface defined in VBE 2.0+. /// /// If the information for a protected mode interface is not available /// this field is set to zero. - pub interface_offset: u16, - - /// Contain the segment length of the table of a protected mode interface defined in VBE 2.0+. + #[must_use] + pub const fn interface_offset(&self) -> u16 { + self.interface_offset + } + /// Returns the segment length of the table of a protected mode interface defined in VBE 2.0+. /// /// If the information for a protected mode interface is not available /// this field is set to zero. - pub interface_length: u16, - - /// Contains VBE controller information returned by the VBE Function `00h`. - pub control_info: VBEControlInfo, - - /// Contains VBE mode information returned by the VBE Function `01h`. - pub mode_info: VBEModeInfo, + #[must_use] + pub const fn interface_length(&self) -> u16 { + self.interface_length + } + /// Returns VBE controller information returned by the VBE Function `00h`. + #[must_use] + pub const fn control_info(&self) -> VBEControlInfo { + self.control_info + } + /// Returns VBE mode information returned by the VBE Function `01h`. + #[must_use] + pub const fn mode_info(&self) -> VBEModeInfo { + self.mode_info + } } impl TagTrait for VBEInfoTag { From 4f93c1c54a5a052777e74fac112c5fbc280b4338 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 11:48:24 +0200 Subject: [PATCH 12/17] doc: update changelog --- multiboot2/Changelog.md | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/multiboot2/Changelog.md b/multiboot2/Changelog.md index 3951b01d..bc377e33 100644 --- a/multiboot2/Changelog.md +++ b/multiboot2/Changelog.md @@ -2,16 +2,34 @@ ## Unreleased -- **Breaking** All functions that returns something useful are now `#[must_use]` -- **Breaking** More public fields in tags were replaced by public getters, such +## 0.21.0 (2024-08-17) + +This release contains a massive refactoring of various internals. Now, **all +unit tests pass Miri**, thus we removed lots of undefined behaviour and +increased the memory safety! 🎉 Only a small part of these internal refactorings +leak to the public interface. If you don't provide external custom tags, you +should be fine. + +Please note that **all previous releases** must be considered unsafe, as they +contain UB. However, it is never clear how UB results in immediate incorrect +behaviour and it _might_ work. **Nevertheless, please migrate to the latest +release and you'll be fine!** + +- **Breaking:** All functions that returns something useful are + now `#[must_use]` +- **Breaking:** More public fields in tags were replaced by public getters, such as `SmbiosTag::major()` -- **BREAKING:** `multiboot2::{StringError};` -> \ - `multiboot2::util::{StringError};` -- updated dependencies -- MSRV is 1.75 +- **Breaking:** Methods of `InformationBuilder` to add tags now consume + references instead of owned values +- **Breaking:** The `BoxedDst` has been removed in favor of a normal Rust `Box`. + This only affects you if you use the `builder` feature. +- **Breaking:** MSRV is 1.75 +- **Breaking:** Introduced new `TagHeader` type as replacement for the `Tag` + type that will be changed in the next step. `Tag` has been renamed to an + internal-only `GenericTag` type. +- Added missing `InformationBuilder::vbe_info_tag` - documentation enhancements -- Introduced new `TagHeader` type as replacement for the `Tag` type that will - be changed in the next step. +- updated dependencies ## 0.20.2 (2024-05-26) From 6c1aa358db1a0d5e10a79c879788d162a8e3aa39 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 11:49:29 +0200 Subject: [PATCH 13/17] Revert "msrv: update from 1.70 to 1.75" This reverts commit 484160e8 --- .github/workflows/rust.yml | 6 +++--- multiboot2-header/Cargo.toml | 2 +- multiboot2-header/Changelog.md | 2 -- multiboot2-header/README.md | 2 +- multiboot2-header/src/lib.rs | 2 +- multiboot2/Cargo.toml | 2 +- multiboot2/README.md | 2 +- multiboot2/src/lib.rs | 2 +- 8 files changed, 9 insertions(+), 11 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 3c914f55..70715b72 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -24,7 +24,7 @@ jobs: name: build (msrv) uses: ./.github/workflows/_build-rust.yml with: - rust-version: 1.75.0 # MSRV + rust-version: 1.70.0 # MSRV do-style-check: false features: builder @@ -50,7 +50,7 @@ jobs: needs: build_msrv uses: ./.github/workflows/_build-rust.yml with: - rust-version: 1.75.0 # MSRV + rust-version: 1.70.0 # MSRV do-style-check: false rust-target: thumbv7em-none-eabihf features: builder @@ -107,7 +107,7 @@ jobs: needs: build_msrv uses: ./.github/workflows/_build-rust.yml with: - rust-version: 1.75.0 # MSRV + rust-version: 1.70.0 # MSRV do-style-check: true do-test: false features: builder diff --git a/multiboot2-header/Cargo.toml b/multiboot2-header/Cargo.toml index e94b20e7..0fd73855 100644 --- a/multiboot2-header/Cargo.toml +++ b/multiboot2-header/Cargo.toml @@ -26,7 +26,7 @@ readme = "README.md" homepage = "https://github.com/rust-osdev/multiboot2-header" repository = "https://github.com/rust-osdev/multiboot2" documentation = "https://docs.rs/multiboot2-header" -rust-version = "1.75" +rust-version = "1.70" [[example]] name = "minimal" diff --git a/multiboot2-header/Changelog.md b/multiboot2-header/Changelog.md index df9f05e8..e37f1b34 100644 --- a/multiboot2-header/Changelog.md +++ b/multiboot2-header/Changelog.md @@ -3,8 +3,6 @@ ## Unreleased - **Breaking** All functions that returns something useful are now `#[must_use]` -- updated dependencies -- documentation enhancements ## 0.4.0 (2024-05-01) diff --git a/multiboot2-header/README.md b/multiboot2-header/README.md index 577a75a8..eebac8d0 100644 --- a/multiboot2-header/README.md +++ b/multiboot2-header/README.md @@ -77,7 +77,7 @@ bytes of the ELF. See Multiboot2 specification. ## MSRV -The MSRV is 1.75.0 stable. +The MSRV is 1.70.0 stable. ## License & Contribution diff --git a/multiboot2-header/src/lib.rs b/multiboot2-header/src/lib.rs index 578511e8..06b6b726 100644 --- a/multiboot2-header/src/lib.rs +++ b/multiboot2-header/src/lib.rs @@ -34,7 +34,7 @@ //! //! ## MSRV //! -//! The MSRV is 1.75.0 stable. +//! The MSRV is 1.70.0 stable. #![no_std] #![cfg_attr(feature = "unstable", feature(error_in_core))] diff --git a/multiboot2/Cargo.toml b/multiboot2/Cargo.toml index c01e8b1b..1fd7de2b 100644 --- a/multiboot2/Cargo.toml +++ b/multiboot2/Cargo.toml @@ -31,7 +31,7 @@ readme = "README.md" homepage = "https://github.com/rust-osdev/multiboot2" repository = "https://github.com/rust-osdev/multiboot2" documentation = "https://docs.rs/multiboot2" -rust-version = "1.75" +rust-version = "1.70" [features] default = ["builder"] diff --git a/multiboot2/README.md b/multiboot2/README.md index 52a207e5..4bdd1893 100644 --- a/multiboot2/README.md +++ b/multiboot2/README.md @@ -45,7 +45,7 @@ tag_, which is a tag of type `0` and size `8`. ## MSRV -The MSRV is 1.75.0 stable. +The MSRV is 1.70.0 stable. ## License & Contribution diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index 90f6b6c6..6357e71f 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -41,7 +41,7 @@ //! ``` //! //! ## MSRV -//! The MSRV is 1.75.0 stable. +//! The MSRV is 1.70.0 stable. #[cfg(feature = "builder")] extern crate alloc; From 9df999fb53b8cc45af7d083d36639767775c35c9 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 11:59:21 +0200 Subject: [PATCH 14/17] workspace: downgrade derive_more to 0.99 This way, I don't have to increase the MSRV for the next release. I want as many users as possible to adopt the latest version, so I want to make it less breaking. --- Cargo.lock | 20 ++------------------ Cargo.toml | 2 +- integration-test/bins/Cargo.lock | 20 ++------------------ multiboot2/Changelog.md | 4 ++++ multiboot2/src/boot_information.rs | 6 +++--- multiboot2/src/framebuffer.rs | 2 +- 6 files changed, 13 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6e378ffe..a17cb6d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10,23 +10,13 @@ checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" [[package]] name = "derive_more" -version = "1.0.0" +version = "0.99.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4a9b99b9cbbe49445b21764dc0625032a89b145a2642e67603e1c936f5458d05" -dependencies = [ - "derive_more-impl", -] - -[[package]] -name = "derive_more-impl" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cb7330aeadfbe296029522e6c40f315320aba36fc43a5b3632f3795348f3bd22" +checksum = "5f33878137e4dafd7fa914ad4e259e18a4e8e532b9617a2d0150262bf53abfce" dependencies = [ "proc-macro2", "quote", "syn 2.0.74", - "unicode-xid", ] [[package]] @@ -136,9 +126,3 @@ name = "unicode-ident" version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" - -[[package]] -name = "unicode-xid" -version = "0.2.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f962df74c8c05a667b5ee8bcf162993134c104e96440b663c8daa176dc772d8c" diff --git a/Cargo.toml b/Cargo.toml index e5eacb6c..59e8e1e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ exclude = [ [workspace.dependencies] bitflags = "2.6.0" -derive_more = { version = "1.0.0", default-features = false, features = ["display"] } +derive_more = { version = "~0.99.18", default-features = false, features = ["display"] } log = { version = "~0.4", default-features = false } # This way, the "multiboot2" dependency in the multiboot2-header crate can be diff --git a/integration-test/bins/Cargo.lock b/integration-test/bins/Cargo.lock index d87a36f5..ec2c77f7 100644 --- a/integration-test/bins/Cargo.lock +++ b/integration-test/bins/Cargo.lock @@ -34,23 +34,13 @@ checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" [[package]] name = "derive_more" -version = "1.0.0" +version = "0.99.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4a9b99b9cbbe49445b21764dc0625032a89b145a2642e67603e1c936f5458d05" -dependencies = [ - "derive_more-impl", -] - -[[package]] -name = "derive_more-impl" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cb7330aeadfbe296029522e6c40f315320aba36fc43a5b3632f3795348f3bd22" +checksum = "5f33878137e4dafd7fa914ad4e259e18a4e8e532b9617a2d0150262bf53abfce" dependencies = [ "proc-macro2", "quote", "syn 2.0.74", - "unicode-xid", ] [[package]] @@ -277,12 +267,6 @@ version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" -[[package]] -name = "unicode-xid" -version = "0.2.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f962df74c8c05a667b5ee8bcf162993134c104e96440b663c8daa176dc772d8c" - [[package]] name = "util" version = "0.1.0" diff --git a/multiboot2/Changelog.md b/multiboot2/Changelog.md index bc377e33..2e3968c3 100644 --- a/multiboot2/Changelog.md +++ b/multiboot2/Changelog.md @@ -2,6 +2,8 @@ ## Unreleased +- + ## 0.21.0 (2024-08-17) This release contains a massive refactoring of various internals. Now, **all @@ -15,6 +17,8 @@ contain UB. However, it is never clear how UB results in immediate incorrect behaviour and it _might_ work. **Nevertheless, please migrate to the latest release and you'll be fine!** +All previous releases on crates.io have been yanked. + - **Breaking:** All functions that returns something useful are now `#[must_use]` - **Breaking:** More public fields in tags were replaced by public getters, such diff --git a/multiboot2/src/boot_information.rs b/multiboot2/src/boot_information.rs index 9cd2485e..8ed11b6b 100644 --- a/multiboot2/src/boot_information.rs +++ b/multiboot2/src/boot_information.rs @@ -21,15 +21,15 @@ use derive_more::Display; pub enum MbiLoadError { /// The address is invalid. Make sure that the address is 8-byte aligned, /// according to the spec. - #[display("The address is invalid")] + #[display(fmt = "The address is invalid")] IllegalAddress, /// The total size of the multiboot2 information structure must be not zero /// and a multiple of 8. - #[display("The size of the MBI is unexpected")] + #[display(fmt = "The size of the MBI is unexpected")] IllegalTotalSize(u32), /// Missing end tag. Each multiboot2 boot information requires to have an /// end tag. - #[display("There is no end tag")] + #[display(fmt = "There is no end tag")] NoEndTag, } diff --git a/multiboot2/src/framebuffer.rs b/multiboot2/src/framebuffer.rs index 0083905a..d1370ee8 100644 --- a/multiboot2/src/framebuffer.rs +++ b/multiboot2/src/framebuffer.rs @@ -331,7 +331,7 @@ impl AsBytes for FramebufferColor {} /// Error when an unknown [`FramebufferTypeId`] is found. #[derive(Debug, Copy, Clone, Display, PartialEq, Eq)] -#[display("Unknown framebuffer type {}", _0)] +#[display(fmt = "Unknown framebuffer type {}", _0)] pub struct UnknownFramebufferType(u8); #[cfg(feature = "unstable")] From 9f23b9a2b7b4f8ed7b1694c02166995dd6342727 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 10:52:02 +0200 Subject: [PATCH 15/17] ci: run miri also with tree-borrow model --- .github/workflows/_build-rust.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/_build-rust.yml b/.github/workflows/_build-rust.yml index 27c01a2f..80971d17 100644 --- a/.github/workflows/_build-rust.yml +++ b/.github/workflows/_build-rust.yml @@ -98,8 +98,9 @@ jobs: run: cargo test --verbose - name: Unit Test with Miri if: inputs.do-miri - # "--tests" so that the doctests are skipped. Currently, the doctest - # in miri fails. run: | rustup component add miri - cargo miri test --tests + # Run with stack-borrow model + cargo miri test + # Run with tree-borrow model + MIRIFLAGS=-Zmiri-tree-borrows cargo +nightly miri test From 3cb74b12fa027c6513a2a52d8328edef5f471b90 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 12:15:26 +0200 Subject: [PATCH 16/17] ci: temporarily, run miri only for multiboot2 The same changes made in this crate, also needs to be done in multiboot2-header. --- .github/workflows/_build-rust.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/_build-rust.yml b/.github/workflows/_build-rust.yml index 80971d17..d855a822 100644 --- a/.github/workflows/_build-rust.yml +++ b/.github/workflows/_build-rust.yml @@ -101,6 +101,8 @@ jobs: run: | rustup component add miri # Run with stack-borrow model - cargo miri test + # XXX Temporarily, just for multiboot2 crate. + cargo miri test -p multiboot2 # Run with tree-borrow model - MIRIFLAGS=-Zmiri-tree-borrows cargo +nightly miri test + # XXX Temporarily, just for multiboot2 crate. + MIRIFLAGS=-Zmiri-tree-borrows cargo +nightly miri test -p multiboot2 From 3484e63022d65135802ee54ecf3b4a3d85ef2292 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 17 Aug 2024 12:30:30 +0200 Subject: [PATCH 17/17] multiboot2: clippy fixes only needed for Rust 1.70 --- multiboot2-header/src/builder/header.rs | 1 + multiboot2/src/lib.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/multiboot2-header/src/builder/header.rs b/multiboot2-header/src/builder/header.rs index ff6ba7bb..2bf3ea93 100644 --- a/multiboot2-header/src/builder/header.rs +++ b/multiboot2-header/src/builder/header.rs @@ -255,6 +255,7 @@ impl HeaderBuilder { /// Adds information requests from the /// [`InformationRequestHeaderTagBuilder`] to the builder. #[must_use] + #[allow(clippy::missing_const_for_fn)] // only in Rust 1.70 necessary pub fn information_request_tag( mut self, information_request_tag: InformationRequestHeaderTagBuilder, diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index 6357e71f..f43f9cfe 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -125,6 +125,7 @@ mod tests { /// This test is relevant to give library users flexebility in passing the /// struct around. #[test] + #[allow(clippy::missing_const_for_fn)] // only in Rust 1.70 necessary fn boot_information_is_send_and_sync() { fn accept(_: T) {} let bytes = AlignedBytes([