From df8f64e6f950947f6dc1f3ae478242932e4602cb Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Thu, 13 Jul 2023 15:08:10 +0200 Subject: [PATCH] multiboot2: cleanup builder module --- multiboot2/src/builder/boxed_dst.rs | 142 ++++++++++++++++++++++++ multiboot2/src/builder/information.rs | 13 +-- multiboot2/src/builder/mod.rs | 150 ++------------------------ 3 files changed, 153 insertions(+), 152 deletions(-) create mode 100644 multiboot2/src/builder/boxed_dst.rs diff --git a/multiboot2/src/builder/boxed_dst.rs b/multiboot2/src/builder/boxed_dst.rs new file mode 100644 index 00000000..f75da1fb --- /dev/null +++ b/multiboot2/src/builder/boxed_dst.rs @@ -0,0 +1,142 @@ +//! Module for [`BoxedDst`]. + +use crate::{Tag, 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 +/// found 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(); + + // 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(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_size(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)] +mod tests { + use super::*; + use crate::TagType; + + 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, core::str::Utf8Error> { + Tag::get_dst_str_slice(&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 + } + } + + #[test] + fn test_boxed_dst_tag() { + let content = "hallo"; + + let tag = BoxedDst::::new(content.as_bytes()); + assert_eq!(tag.typ, CustomTag::ID); + assert_eq!(tag.size as usize, METADATA_SIZE + content.len()); + assert_eq!(tag.string(), Ok(content)); + } +} diff --git a/multiboot2/src/builder/information.rs b/multiboot2/src/builder/information.rs index 09d8984f..221cd91f 100644 --- a/multiboot2/src/builder/information.rs +++ b/multiboot2/src/builder/information.rs @@ -1,26 +1,15 @@ //! Exports item [`InformationBuilder`]. +use crate::builder::{AsBytes, BoxedDst}; use crate::{ BasicMemoryInfoTag, BootInformationHeader, BootLoaderNameTag, CommandLineTag, EFIBootServicesNotExitedTag, EFIImageHandle32Tag, EFIImageHandle64Tag, EFIMemoryMapTag, EFISdt32Tag, EFISdt64Tag, ElfSectionsTag, EndTag, FramebufferTag, ImageLoadPhysAddrTag, MemoryMapTag, ModuleTag, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagTrait, TagType, }; - -use crate::builder::BoxedDst; use alloc::vec::Vec; use core::mem::size_of; use core::ops::Deref; -/// Helper trait for all structs that need to be serialized that do not -/// implement `TagTrait`. -pub trait AsBytes: Sized { - fn as_bytes(&self) -> &[u8] { - let ptr = core::ptr::addr_of!(*self); - let size = core::mem::size_of::(); - unsafe { core::slice::from_raw_parts(ptr.cast(), size) } - } -} - /// Holds the raw bytes of a boot information built with [`InformationBuilder`] /// on the heap. The bytes returned by [`BootInformationBytes::as_bytes`] are /// guaranteed to be properly aligned. diff --git a/multiboot2/src/builder/mod.rs b/multiboot2/src/builder/mod.rs index a03f016b..1a1d7425 100644 --- a/multiboot2/src/builder/mod.rs +++ b/multiboot2/src/builder/mod.rs @@ -1,148 +1,18 @@ //! Module for the builder-feature. +mod boxed_dst; mod information; -pub(crate) use information::AsBytes; +// This must by public to support external people to create boxed DSTs. +pub use boxed_dst::BoxedDst; pub use information::InformationBuilder; -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; - -use crate::{Tag, TagTrait, TagTypeId}; - -/// 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. -/// -/// 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(); - - // 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(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_size(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)] -mod tests { - use super::*; - use crate::TagType; - - 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, core::str::Utf8Error> { - Tag::get_dst_str_slice(&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 - } - } - - #[test] - fn test_boxed_dst_tag() { - let content = "hallo"; - - let tag = BoxedDst::::new(content.as_bytes()); - assert_eq!(tag.typ, CustomTag::ID); - assert_eq!(tag.size as usize, METADATA_SIZE + content.len()); - assert_eq!(tag.string(), Ok(content)); +/// Helper trait for all structs that need to be serialized that do not +/// implement `TagTrait`. +pub trait AsBytes: Sized { + fn as_bytes(&self) -> &[u8] { + let ptr = core::ptr::addr_of!(*self); + let size = core::mem::size_of::(); + unsafe { core::slice::from_raw_parts(ptr.cast(), size) } } }