diff --git a/multiboot2-header/src/builder/header.rs b/multiboot2-header/src/builder/header.rs index d15c542e..d406d13e 100644 --- a/multiboot2-header/src/builder/header.rs +++ b/multiboot2-header/src/builder/header.rs @@ -8,7 +8,6 @@ use crate::{ EntryAddressHeaderTag, EntryEfi32HeaderTag, EntryEfi64HeaderTag, FramebufferHeaderTag, ModuleAlignHeaderTag, Multiboot2BasicHeader, RelocatableHeaderTag, }; -use alloc::boxed::Box; use alloc::vec::Vec; use core::mem::size_of; use core::ops::Deref; @@ -22,7 +21,7 @@ pub struct HeaderBytes { // guarantee alignment at the moment. offset: usize, structure_len: usize, - bytes: Box<[u8]>, + bytes: Vec, } impl HeaderBytes { @@ -163,7 +162,7 @@ impl HeaderBuilder { pub fn build(mut self) -> HeaderBytes { const ALIGN: usize = 8; - // PHASE 1/3: Prepare Vector + // PHASE 1/2: Prepare Vector // We allocate more than necessary so that we can ensure an correct // alignment within this data. @@ -184,17 +183,9 @@ impl HeaderBuilder { bytes.extend([0].repeat(offset)); // ----------------------------------------------- - // PHASE 2/3: Add Tags + // PHASE 2/2: Add Tags self.build_add_tags(&mut bytes); - // ----------------------------------------------- - // PHASE 3/3: Finalize Vector - - // Ensure that the vector has the same length as it's capacity. This is - // important so that miri doesn't complain that the boxed memory is - // smaller than the original allocation. - bytes.extend([0].repeat(bytes.capacity() - bytes.len())); - assert_eq!( alloc_ptr, bytes.as_ptr(), @@ -206,11 +197,6 @@ impl HeaderBuilder { "The offset to alignment area should be zero." ); - // Construct a box from a vec without `into_boxed_slice`. The latter - // calls `shrink` on the allocator, which might reallocate this memory. - // We don't want that! - let bytes = unsafe { Box::from_raw(bytes.leak()) }; - HeaderBytes { offset, bytes, @@ -316,6 +302,42 @@ mod tests { RelocatableHeaderTagPreference, }; + fn create_builder() -> HeaderBuilder { + let builder = HeaderBuilder::new(HeaderTagISA::I386); + // Multiboot2 basic header + end tag + let mut expected_len = 16 + 8; + assert_eq!(builder.expected_len(), expected_len); + + // add information request tag + let ifr_builder = + InformationRequestHeaderTagBuilder::new(HeaderTagFlag::Required).add_irs(&[ + MbiTagType::EfiMmap, + MbiTagType::Cmdline, + MbiTagType::ElfSections, + ]); + let ifr_tag_size_with_padding = ifr_builder.expected_len() + 4; + assert_eq!( + ifr_tag_size_with_padding % 8, + 0, + "the length of the IFR tag with padding must be a multiple of 8" + ); + expected_len += ifr_tag_size_with_padding; + let builder = builder.information_request_tag(ifr_builder); + assert_eq!(builder.expected_len(), expected_len); + + let builder = builder.relocatable_tag(RelocatableHeaderTag::new( + HeaderTagFlag::Required, + 0x1337, + 0xdeadbeef, + 4096, + RelocatableHeaderTagPreference::None, + )); + expected_len += 0x18; + assert_eq!(builder.expected_len(), expected_len); + + builder + } + #[test] fn test_size_or_up_aligned() { assert_eq!(0, HeaderBuilder::size_or_up_aligned(0)); @@ -324,73 +346,44 @@ mod tests { assert_eq!(16, HeaderBuilder::size_or_up_aligned(9)); } + /// Test of the `build` method in isolation specifically for miri to check + /// for memory issues. + #[test] + fn test_builder_miri() { + let builder = create_builder(); + let expected_len = builder.expected_len(); + assert_eq!(builder.build().as_bytes().len(), expected_len); + } + #[test] fn test_builder() { // Step 1/2: Build Header - let (mb2_hdr_data, expected_len) = { - let builder = HeaderBuilder::new(HeaderTagISA::I386); - // Multiboot2 basic header + end tag - let mut expected_len = 16 + 8; - assert_eq!(builder.expected_len(), expected_len); - - // add information request tag - let ifr_builder = InformationRequestHeaderTagBuilder::new(HeaderTagFlag::Required) - .add_irs(&[ - MbiTagType::EfiMmap, - MbiTagType::Cmdline, - MbiTagType::ElfSections, - ]); - let ifr_tag_size_with_padding = ifr_builder.expected_len() + 4; - assert_eq!( - ifr_tag_size_with_padding % 8, - 0, - "the length of the IFR tag with padding must be a multiple of 8" - ); - expected_len += ifr_tag_size_with_padding; - let builder = builder.information_request_tag(ifr_builder); - assert_eq!(builder.expected_len(), expected_len); - - let builder = builder.relocatable_tag(RelocatableHeaderTag::new( - HeaderTagFlag::Required, - 0x1337, - 0xdeadbeef, - 4096, - RelocatableHeaderTagPreference::None, - )); - expected_len += 0x18; - assert_eq!(builder.expected_len(), expected_len); - - println!("builder: {:#?}", builder); - println!("expected_len: {} bytes", builder.expected_len()); - - (builder.build(), expected_len) - }; - - assert_eq!(mb2_hdr_data.as_bytes().len(), expected_len); + let mb2_hdr_data = create_builder().build(); // Step 2/2: Test the built Header - { - let mb2_hdr = mb2_hdr_data.as_ptr().cast(); - let mb2_hdr = unsafe { Multiboot2Header::load(mb2_hdr) } - .expect("the generated header to be loadable"); - println!("{:#?}", mb2_hdr); - assert_eq!( - mb2_hdr.relocatable_tag().unwrap().flags(), - HeaderTagFlag::Required - ); - assert_eq!(mb2_hdr.relocatable_tag().unwrap().min_addr(), 0x1337); - assert_eq!(mb2_hdr.relocatable_tag().unwrap().max_addr(), 0xdeadbeef); - assert_eq!(mb2_hdr.relocatable_tag().unwrap().align(), 4096); - assert_eq!( - mb2_hdr.relocatable_tag().unwrap().preference(), - RelocatableHeaderTagPreference::None - ); - - /* you can write the binary to a file and a tool such as crate "bootinfo" - will be able to fully parse the MB2 header - let mut file = std::file::File::create("mb2_hdr.bin").unwrap(); - use std::io::Write; - file.write_all(mb2_hdr_data.as_slice()).unwrap();*/ - } + let mb2_hdr = mb2_hdr_data.as_ptr().cast(); + let mb2_hdr = unsafe { Multiboot2Header::load(mb2_hdr) } + .expect("the generated header to be loadable"); + println!("{:#?}", mb2_hdr); + assert_eq!( + mb2_hdr.relocatable_tag().unwrap().flags(), + HeaderTagFlag::Required + ); + assert_eq!(mb2_hdr.relocatable_tag().unwrap().min_addr(), 0x1337); + assert_eq!(mb2_hdr.relocatable_tag().unwrap().max_addr(), 0xdeadbeef); + assert_eq!(mb2_hdr.relocatable_tag().unwrap().align(), 4096); + assert_eq!( + mb2_hdr.relocatable_tag().unwrap().preference(), + RelocatableHeaderTagPreference::None + ); + + // Printing the header transitively ensures that a lot of stuff works. + println!("{:#?}", mb2_hdr); + + /* you can write the binary to a file and a tool such as crate "bootinfo" + will be able to fully parse the MB2 header + let mut file = std::file::File::create("mb2_hdr.bin").unwrap(); + use std::io::Write; + file.write_all(mb2_hdr_data.as_slice()).unwrap();*/ } } diff --git a/multiboot2/src/boot_loader_name.rs b/multiboot2/src/boot_loader_name.rs index 20088e71..5e9863fc 100644 --- a/multiboot2/src/boot_loader_name.rs +++ b/multiboot2/src/boot_loader_name.rs @@ -5,8 +5,8 @@ use core::str::Utf8Error; #[cfg(feature = "builder")] use { - crate::builder::boxed_dst_tag, crate::builder::traits::StructAsBytes, crate::TagType, - alloc::boxed::Box, alloc::vec::Vec, + crate::builder::traits::StructAsBytes, crate::builder::BoxedDst, crate::TagType, + alloc::vec::Vec, }; const METADATA_SIZE: usize = size_of::() + size_of::(); @@ -23,13 +23,13 @@ pub struct BootLoaderNameTag { impl BootLoaderNameTag { #[cfg(feature = "builder")] - pub fn new(name: &str) -> Box { + pub fn new(name: &str) -> BoxedDst { let mut bytes: Vec<_> = name.bytes().collect(); if !bytes.ends_with(&[0]) { // terminating null-byte bytes.push(0); } - boxed_dst_tag(TagType::BootLoaderName, &bytes) + BoxedDst::new(TagType::BootLoaderName, &bytes) } /// Reads the name of the bootloader that is booting the kernel as Rust @@ -119,5 +119,11 @@ mod tests { let bytes = tag.struct_as_bytes(); assert_eq!(bytes, get_bytes()); assert_eq!(tag.name(), Ok(MSG)); + + // test also some bigger message + let tag = BootLoaderNameTag::new("AbCdEfGhUjK YEAH"); + assert_eq!(tag.name(), Ok("AbCdEfGhUjK YEAH")); + let tag = BootLoaderNameTag::new("AbCdEfGhUjK YEAH".repeat(42).as_str()); + assert_eq!(tag.name(), Ok("AbCdEfGhUjK YEAH".repeat(42).as_str())); } } diff --git a/multiboot2/src/builder/information.rs b/multiboot2/src/builder/information.rs index b3efa781..adc23415 100644 --- a/multiboot2/src/builder/information.rs +++ b/multiboot2/src/builder/information.rs @@ -7,7 +7,7 @@ use crate::{ MemoryMapTag, ModuleTag, RsdpV1Tag, RsdpV2Tag, SmbiosTag, }; -use alloc::boxed::Box; +use crate::builder::BoxedDst; use alloc::vec::Vec; use core::mem::size_of; use core::ops::Deref; @@ -21,7 +21,7 @@ pub struct BootInformationBytes { // guarantee alignment at the moment. offset: usize, structure_len: usize, - bytes: Box<[u8]>, + bytes: Vec, } impl BootInformationBytes { @@ -49,22 +49,22 @@ impl Deref for BootInformationBytes { #[derive(Debug, PartialEq, Eq)] pub struct InformationBuilder { basic_memory_info_tag: Option, - boot_loader_name_tag: Option>, - command_line_tag: Option>, + boot_loader_name_tag: Option>, + command_line_tag: Option>, efi_boot_services_not_exited_tag: Option, efi_image_handle32: Option, efi_image_handle64: Option, - efi_memory_map_tag: Option>, - elf_sections_tag: Option>, - framebuffer_tag: Option>, + efi_memory_map_tag: Option>, + elf_sections_tag: Option>, + framebuffer_tag: Option>, image_load_addr: Option, - memory_map_tag: Option>, - module_tags: Vec>, + memory_map_tag: Option>, + module_tags: Vec>, efisdt32_tag: Option, efisdt64_tag: Option, rsdp_v1_tag: Option, rsdp_v2_tag: Option, - smbios_tags: Vec>, + smbios_tags: Vec>, } impl InformationBuilder { @@ -183,7 +183,7 @@ impl InformationBuilder { pub fn build(self) -> BootInformationBytes { const ALIGN: usize = 8; - // PHASE 1/3: Prepare Vector + // PHASE 1/2: Prepare Vector // We allocate more than necessary so that we can ensure an correct // alignment within this data. @@ -204,17 +204,9 @@ impl InformationBuilder { bytes.extend([0].repeat(offset)); // ----------------------------------------------- - // PHASE 2/3: Add Tags + // PHASE 2/2: Add Tags self.build_add_tags(&mut bytes); - // ----------------------------------------------- - // PHASE 3/3: Finalize Vector - - // Ensure that the vector has the same length as it's capacity. This is - // important so that miri doesn't complain that the boxed memory is - // smaller than the original allocation. - bytes.extend([0].repeat(bytes.capacity() - bytes.len())); - assert_eq!( alloc_ptr, bytes.as_ptr(), @@ -226,13 +218,6 @@ impl InformationBuilder { "The offset to alignment area should be zero." ); - // Construct a box from a vec without `into_boxed_slice`. The latter - // calls `shrink` on the allocator, which might reallocate this memory. - // We don't want that! - let bytes = unsafe { Box::from_raw(bytes.leak()) }; - - assert_eq!(bytes.len(), alloc_len); - BootInformationBytes { offset, bytes, @@ -306,11 +291,11 @@ impl InformationBuilder { self.basic_memory_info_tag = Some(basic_memory_info_tag) } - pub fn bootloader_name_tag(&mut self, boot_loader_name_tag: Box) { + pub fn bootloader_name_tag(&mut self, boot_loader_name_tag: BoxedDst) { self.boot_loader_name_tag = Some(boot_loader_name_tag); } - pub fn command_line_tag(&mut self, command_line_tag: Box) { + pub fn command_line_tag(&mut self, command_line_tag: BoxedDst) { self.command_line_tag = Some(command_line_tag); } @@ -334,15 +319,15 @@ impl InformationBuilder { self.efi_image_handle64 = Some(efi_image_handle64); } - pub fn efi_memory_map_tag(&mut self, efi_memory_map_tag: Box) { + pub fn efi_memory_map_tag(&mut self, efi_memory_map_tag: BoxedDst) { self.efi_memory_map_tag = Some(efi_memory_map_tag); } - pub fn elf_sections_tag(&mut self, elf_sections_tag: Box) { + pub fn elf_sections_tag(&mut self, elf_sections_tag: BoxedDst) { self.elf_sections_tag = Some(elf_sections_tag); } - pub fn framebuffer_tag(&mut self, framebuffer_tag: Box) { + pub fn framebuffer_tag(&mut self, framebuffer_tag: BoxedDst) { self.framebuffer_tag = Some(framebuffer_tag); } @@ -350,11 +335,11 @@ impl InformationBuilder { self.image_load_addr = Some(image_load_addr); } - pub fn memory_map_tag(&mut self, memory_map_tag: Box) { + pub fn memory_map_tag(&mut self, memory_map_tag: BoxedDst) { self.memory_map_tag = Some(memory_map_tag); } - pub fn add_module_tag(&mut self, module_tag: Box) { + pub fn add_module_tag(&mut self, module_tag: BoxedDst) { self.module_tags.push(module_tag); } @@ -366,7 +351,7 @@ impl InformationBuilder { self.rsdp_v2_tag = Some(rsdp_v2_tag); } - pub fn add_smbios_tag(&mut self, smbios_tag: Box) { + pub fn add_smbios_tag(&mut self, smbios_tag: BoxedDst) { self.smbios_tags.push(smbios_tag); } } @@ -376,6 +361,35 @@ mod tests { use crate::builder::information::InformationBuilder; use crate::{BasicMemoryInfoTag, BootInformation, CommandLineTag, ModuleTag}; + fn create_builder() -> InformationBuilder { + let mut builder = InformationBuilder::new(); + + // Multiboot2 basic information + end tag + let mut expected_len = 8 + 8; + assert_eq!(builder.expected_len(), expected_len); + + // the most simple tag + 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.command_line_tag(CommandLineTag::new("test")); + expected_len += 8 + 5 + 3; // padding + assert_eq!(builder.expected_len(), expected_len); + // many modules + builder.add_module_tag(ModuleTag::new(0, 1234, "module1")); + expected_len += 16 + 8; + assert_eq!(builder.expected_len(), expected_len); + builder.add_module_tag(ModuleTag::new(5678, 6789, "module2")); + expected_len += 16 + 8; + assert_eq!(builder.expected_len(), expected_len); + + println!("builder: {:#?}", builder); + println!("expected_len: {} bytes", builder.expected_len()); + + builder + } + #[test] fn test_size_or_up_aligned() { assert_eq!(0, InformationBuilder::size_or_up_aligned(0)); @@ -384,65 +398,42 @@ mod tests { 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] + fn test_builder_miri() { + let builder = create_builder(); + let expected_len = builder.expected_len(); + assert_eq!(builder.build().as_bytes().len(), expected_len); + } + #[test] fn test_builder() { // Step 1/2: Build MBI - let (mb2i_data, expected_len) = { - let mut builder = InformationBuilder::new(); - - // Multiboot2 basic information + end tag - let mut expected_len = 8 + 8; - assert_eq!(builder.expected_len(), expected_len); - - // the most simple tag - 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.command_line_tag(CommandLineTag::new("test")); - expected_len += 8 + 5 + 3; // padding - assert_eq!(builder.expected_len(), expected_len); - // many modules - builder.add_module_tag(ModuleTag::new(0, 1234, "module1")); - expected_len += 16 + 8; - assert_eq!(builder.expected_len(), expected_len); - builder.add_module_tag(ModuleTag::new(5678, 6789, "module2")); - expected_len += 16 + 8; - assert_eq!(builder.expected_len(), expected_len); - - println!("builder: {:#?}", builder); - println!("expected_len: {} bytes", builder.expected_len()); - assert_eq!(builder.expected_len(), expected_len); - - (builder.build(), expected_len) - }; - - assert_eq!(mb2i_data.as_bytes().len(), expected_len); + let mb2i_data = create_builder().build(); // Step 2/2: Test the built MBI - { - let mb2i = unsafe { BootInformation::load(mb2i_data.as_ptr().cast()) } - .expect("generated information should be readable"); - - assert_eq!(mb2i.basic_memory_info_tag().unwrap().memory_lower(), 640); - assert_eq!( - mb2i.basic_memory_info_tag().unwrap().memory_upper(), - 7 * 1024 - ); - assert_eq!(mb2i.command_line_tag().unwrap().cmdline().unwrap(), "test"); - let mut modules = mb2i.module_tags(); - let module_1 = modules.next().unwrap(); - assert_eq!(module_1.start_address(), 0); - assert_eq!(module_1.end_address(), 1234); - assert_eq!(module_1.cmdline().unwrap(), "module1"); - let module_2 = modules.next().unwrap(); - assert_eq!(module_2.start_address(), 5678); - assert_eq!(module_2.end_address(), 6789); - assert_eq!(module_2.cmdline().unwrap(), "module2"); - assert!(modules.next().is_none()); - - // Printing the MBI transitively ensures that a lot of stuff works. - println!("{:#?}", mb2i); - } + let mb2i = unsafe { BootInformation::load(mb2i_data.as_ptr().cast()) } + .expect("generated information should be readable"); + + assert_eq!(mb2i.basic_memory_info_tag().unwrap().memory_lower(), 640); + assert_eq!( + mb2i.basic_memory_info_tag().unwrap().memory_upper(), + 7 * 1024 + ); + assert_eq!(mb2i.command_line_tag().unwrap().cmdline().unwrap(), "test"); + let mut modules = mb2i.module_tags(); + let module_1 = modules.next().unwrap(); + assert_eq!(module_1.start_address(), 0); + assert_eq!(module_1.end_address(), 1234); + assert_eq!(module_1.cmdline().unwrap(), "module1"); + let module_2 = modules.next().unwrap(); + assert_eq!(module_2.start_address(), 5678); + assert_eq!(module_2.end_address(), 6789); + assert_eq!(module_2.cmdline().unwrap(), "module2"); + assert!(modules.next().is_none()); + + // Printing the MBI transitively ensures that a lot of stuff works. + println!("{:#?}", mb2i); } } diff --git a/multiboot2/src/builder/mod.rs b/multiboot2/src/builder/mod.rs index bd5d4698..d9b1b0d0 100644 --- a/multiboot2/src/builder/mod.rs +++ b/multiboot2/src/builder/mod.rs @@ -6,59 +6,104 @@ pub(crate) mod traits; pub use information::InformationBuilder; use alloc::alloc::alloc; -use alloc::boxed::Box; use core::alloc::Layout; -use core::mem::{align_of_val, size_of}; +use core::marker::PhantomData; +use core::mem::size_of; use core::ops::Deref; +use core::ptr::NonNull; use crate::{Tag, TagTrait, TagTypeId}; -/// Create a boxed tag with the given content. +/// 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 +/// found any way to figure out the right layout for a DST. Miri always reported +/// issues that the deallocation used a wrong layout. /// -/// # Parameters -/// - `typ` - The given [`TagTypeId`] -/// - `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(super) fn boxed_dst_tag + ?Sized>( - typ: impl Into, - content: &[u8], -) -> Box { - // Currently, I do not find a nice way of making this dynamic so that also - // miri is happy. But it seems that 4 is fine. - const ALIGN: usize = 4; - - let tag_size = size_of::() + size_of::() + content.len(); - let layout = Layout::from_size_align(tag_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(typ.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; +/// 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 + /// - `typ` - The given [`TagTypeId`] + /// - `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(typ: impl Into, 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(); + + // 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 + 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(typ.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_size(base_tag)); + + Self { + ptr: NonNull::new(raw).unwrap(), + layout, + _marker: PhantomData, } } +} - let base_tag = unsafe { &*ptr.cast::() }; - let raw: *mut T = ptr_meta::from_raw_parts_mut(ptr.cast(), T::dst_size(base_tag)); +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() } + } +} - unsafe { - let boxed = Box::from_raw(raw); - let reference: &T = boxed.deref(); - // If this panics, please create an issue on GitHub. - assert_eq!(align_of_val(reference), ALIGN); - boxed +impl PartialEq for BoxedDst { + fn eq(&self, other: &Self) -> bool { + self.deref().eq(other.deref()) } } @@ -94,7 +139,7 @@ mod tests { let tag_type_id = 1337_u32; let content = "hallo"; - let tag = boxed_dst_tag::(tag_type_id, content.as_bytes()); + let tag = unsafe { BoxedDst::::new(tag_type_id, content.as_bytes()) }; assert_eq!(tag.typ, tag_type_id); assert_eq!(tag.size as usize, METADATA_SIZE + content.len()); assert_eq!(tag.string(), Ok(content)); diff --git a/multiboot2/src/command_line.rs b/multiboot2/src/command_line.rs index 51a4d8b6..fc15cbb3 100644 --- a/multiboot2/src/command_line.rs +++ b/multiboot2/src/command_line.rs @@ -8,8 +8,8 @@ use core::str; #[cfg(feature = "builder")] use { - crate::builder::boxed_dst_tag, crate::builder::traits::StructAsBytes, crate::TagType, - alloc::boxed::Box, alloc::vec::Vec, core::convert::TryInto, + crate::builder::traits::StructAsBytes, crate::builder::BoxedDst, crate::TagType, + alloc::vec::Vec, core::convert::TryInto, }; pub(crate) const METADATA_SIZE: usize = mem::size_of::() + mem::size_of::(); @@ -30,13 +30,13 @@ pub struct CommandLineTag { impl CommandLineTag { /// Create a new command line tag from the given string. #[cfg(feature = "builder")] - pub fn new(command_line: &str) -> Box { + pub fn new(command_line: &str) -> BoxedDst { let mut bytes: Vec<_> = command_line.bytes().collect(); if !bytes.ends_with(&[0]) { // terminating null-byte bytes.push(0); } - boxed_dst_tag(TagType::Cmdline, &bytes) + BoxedDst::new(TagType::Cmdline, &bytes) } /// Reads the command line of the kernel as Rust string slice without @@ -128,5 +128,11 @@ mod tests { let bytes = tag.struct_as_bytes(); assert_eq!(bytes, get_bytes()); assert_eq!(tag.cmdline(), Ok(MSG)); + + // test also some bigger message + let tag = CommandLineTag::new("AbCdEfGhUjK YEAH"); + assert_eq!(tag.cmdline(), Ok("AbCdEfGhUjK YEAH")); + let tag = CommandLineTag::new("AbCdEfGhUjK YEAH".repeat(42).as_str()); + assert_eq!(tag.cmdline(), Ok("AbCdEfGhUjK YEAH".repeat(42).as_str())); } } diff --git a/multiboot2/src/elf_sections.rs b/multiboot2/src/elf_sections.rs index 9135f9c9..a17412f0 100644 --- a/multiboot2/src/elf_sections.rs +++ b/multiboot2/src/elf_sections.rs @@ -5,10 +5,7 @@ use core::mem::size_of; use core::str::Utf8Error; #[cfg(feature = "builder")] -use { - crate::builder::boxed_dst_tag, crate::builder::traits::StructAsBytes, crate::TagType, - alloc::boxed::Box, -}; +use {crate::builder::traits::StructAsBytes, crate::builder::BoxedDst, crate::TagType}; const METADATA_SIZE: usize = size_of::() + 4 * size_of::(); @@ -29,7 +26,12 @@ pub struct ElfSectionsTag { impl ElfSectionsTag { /// Create a new ElfSectionsTag with the given data. #[cfg(feature = "builder")] - pub fn new(number_of_sections: u32, entry_size: u32, shndx: u32, sections: &[u8]) -> Box { + pub fn new( + number_of_sections: u32, + entry_size: u32, + shndx: u32, + sections: &[u8], + ) -> BoxedDst { let mut bytes = [ number_of_sections.to_le_bytes(), entry_size.to_le_bytes(), @@ -37,7 +39,7 @@ impl ElfSectionsTag { ] .concat(); bytes.extend_from_slice(sections); - boxed_dst_tag(TagType::ElfSections, &bytes) + BoxedDst::new(TagType::ElfSections, &bytes) } /// Get an iterator of loaded ELF sections. diff --git a/multiboot2/src/framebuffer.rs b/multiboot2/src/framebuffer.rs index 3e5e3f34..43def241 100644 --- a/multiboot2/src/framebuffer.rs +++ b/multiboot2/src/framebuffer.rs @@ -7,8 +7,8 @@ use derive_more::Display; #[cfg(feature = "builder")] use { - crate::builder::boxed_dst_tag, crate::builder::traits::StructAsBytes, crate::TagType, - alloc::boxed::Box, alloc::vec::Vec, + crate::builder::traits::StructAsBytes, crate::builder::BoxedDst, crate::TagType, + alloc::vec::Vec, }; /// Helper struct to read bytes from a raw pointer and increase the pointer @@ -95,14 +95,14 @@ impl FramebufferTag { height: u32, bpp: u8, buffer_type: FramebufferType, - ) -> Box { + ) -> BoxedDst { 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()); - boxed_dst_tag(TagType::Framebuffer, &bytes) + BoxedDst::new(TagType::Framebuffer, &bytes) } /// Contains framebuffer physical address. diff --git a/multiboot2/src/memory_map.rs b/multiboot2/src/memory_map.rs index 8bcbdc33..888e42fb 100644 --- a/multiboot2/src/memory_map.rs +++ b/multiboot2/src/memory_map.rs @@ -8,7 +8,7 @@ pub use uefi_raw::table::boot::MemoryDescriptor as EFIMemoryDesc; pub use uefi_raw::table::boot::MemoryType as EFIMemoryAreaType; #[cfg(feature = "builder")] -use {crate::builder::boxed_dst_tag, crate::builder::traits::StructAsBytes, alloc::boxed::Box}; +use {crate::builder::traits::StructAsBytes, crate::builder::BoxedDst}; const METADATA_SIZE: usize = mem::size_of::() + 3 * mem::size_of::(); @@ -34,14 +34,14 @@ pub struct MemoryMapTag { impl MemoryMapTag { #[cfg(feature = "builder")] - pub fn new(areas: &[MemoryArea]) -> Box { + pub fn new(areas: &[MemoryArea]) -> BoxedDst { 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.struct_as_bytes()); } - boxed_dst_tag(TagType::Mmap, bytes.as_slice()) + BoxedDst::new(TagType::Mmap, bytes.as_slice()) } /// Returns the entry size. @@ -218,7 +218,7 @@ 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. - pub fn new(descs: &[EFIMemoryDesc]) -> Box { + pub fn new(descs: &[EFIMemoryDesc]) -> BoxedDst { // update this when updating EFIMemoryDesc const MEMORY_DESCRIPTOR_VERSION: u32 = 1; let mut bytes = [ @@ -229,7 +229,7 @@ impl EFIMemoryMapTag { for desc in descs { bytes.extend(desc.struct_as_bytes()); } - boxed_dst_tag(TagType::EfiMmap, bytes.as_slice()) + BoxedDst::new(TagType::EfiMmap, bytes.as_slice()) } /// Return an iterator over ALL marked memory areas. diff --git a/multiboot2/src/module.rs b/multiboot2/src/module.rs index 14d1e10f..9661d889 100644 --- a/multiboot2/src/module.rs +++ b/multiboot2/src/module.rs @@ -5,10 +5,7 @@ use core::mem::size_of; use core::str::Utf8Error; #[cfg(feature = "builder")] -use { - crate::builder::boxed_dst_tag, crate::builder::traits::StructAsBytes, alloc::boxed::Box, - alloc::vec::Vec, -}; +use {crate::builder::traits::StructAsBytes, crate::builder::BoxedDst, alloc::vec::Vec}; const METADATA_SIZE: usize = size_of::() + 3 * size_of::(); @@ -28,7 +25,7 @@ pub struct ModuleTag { impl ModuleTag { #[cfg(feature = "builder")] - pub fn new(start: u32, end: u32, cmdline: &str) -> Box { + pub fn new(start: u32, end: u32, cmdline: &str) -> BoxedDst { let mut cmdline_bytes: Vec<_> = cmdline.bytes().collect(); if !cmdline_bytes.ends_with(&[0]) { // terminating null-byte @@ -38,7 +35,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); - boxed_dst_tag(TagType::Module, &content_bytes) + BoxedDst::new(TagType::Module, &content_bytes) } /// Reads the command line of the boot module as Rust string slice without @@ -174,5 +171,11 @@ mod tests { let bytes = tag.struct_as_bytes(); assert_eq!(bytes, get_bytes()); assert_eq!(tag.cmdline(), Ok(MSG)); + + // test also some bigger message + let tag = ModuleTag::new(0, 0, "AbCdEfGhUjK YEAH"); + assert_eq!(tag.cmdline(), Ok("AbCdEfGhUjK YEAH")); + let tag = ModuleTag::new(0, 0, "AbCdEfGhUjK YEAH".repeat(42).as_str()); + assert_eq!(tag.cmdline(), Ok("AbCdEfGhUjK YEAH".repeat(42).as_str())); } } diff --git a/multiboot2/src/smbios.rs b/multiboot2/src/smbios.rs index 6b9ed22c..bd7eff27 100644 --- a/multiboot2/src/smbios.rs +++ b/multiboot2/src/smbios.rs @@ -2,8 +2,8 @@ use crate::{Tag, TagTrait, TagTypeId}; use core::fmt::Debug; #[cfg(feature = "builder")] use { - crate::builder::boxed_dst_tag, crate::builder::traits::StructAsBytes, crate::TagType, - alloc::boxed::Box, core::convert::TryInto, + crate::builder::traits::StructAsBytes, crate::builder::BoxedDst, crate::TagType, + core::convert::TryInto, }; const METADATA_SIZE: usize = core::mem::size_of::() @@ -24,10 +24,10 @@ pub struct SmbiosTag { impl SmbiosTag { #[cfg(feature = "builder")] - pub fn new(major: u8, minor: u8, tables: &[u8]) -> Box { + pub fn new(major: u8, minor: u8, tables: &[u8]) -> BoxedDst { let mut bytes = [major, minor, 0, 0, 0, 0, 0, 0].to_vec(); bytes.extend(tables); - boxed_dst_tag(TagType::Smbios, &bytes) + BoxedDst::new(TagType::Smbios, &bytes) } }