multiboot2: model MBI as DST

This is an improvement that enables better memory checks by miri. Furthermore,
it is technically the right to model this in Rust.

Author: phip1611

multiboot2/src/builder/information.rs

@@ -1,7 +1,7 @@
 //! Exports item [`InformationBuilder`].
 use crate::builder::traits::StructAsBytes;
 use crate::{
-    BasicMemoryInfoTag, BootInformationInner, BootLoaderNameTag, CommandLineTag,
+    BasicMemoryInfoTag, BootInformationHeader, BootLoaderNameTag, CommandLineTag,
     EFIBootServicesNotExitedTag, EFIImageHandle32Tag, EFIImageHandle64Tag, EFIMemoryMapTag,
     EFISdt32Tag, EFISdt64Tag, ElfSectionsTag, EndTag, FramebufferTag, ImageLoadPhysAddrTag,
     MemoryMapTag, ModuleTag, RsdpV1Tag, RsdpV2Tag, SmbiosTag,
@@ -74,7 +74,7 @@ impl InformationBuilder {
     /// Returns the expected length of the Multiboot2 header,
     /// when the `build()`-method gets called.
     pub fn expected_len(&self) -> usize {
-        let base_len = size_of::<BootInformationInner>();
+        let base_len = size_of::<BootInformationHeader>();
         // size_or_up_aligned not required, because length is 16 and the
         // begin is 8 byte aligned => first tag automatically 8 byte aligned
         let mut len = Self::size_or_up_aligned(base_len);
@@ -156,7 +156,7 @@ impl InformationBuilder {
         Self::build_add_bytes(
             &mut data,
             // important that we write the correct expected length into the header!
-            &BootInformationInner::new(self.expected_len() as u32).struct_as_bytes(),
+            &BootInformationHeader::new(self.expected_len() as u32).struct_as_bytes(),
             false,
         );
 
@@ -327,7 +327,7 @@ mod tests {
         assert_eq!(builder.expected_len(), expected_len);
 
         let mb2i_data = builder.build();
-        let mb2i = unsafe { BootInformation::load(mb2i_data.as_ptr()) }
+        let mb2i = unsafe { BootInformation::load(mb2i_data.as_ptr().cast()) }
             .expect("the generated information to be readable");
         println!("{:#?}", mb2i);
         assert_eq!(mb2i.basic_memory_info_tag().unwrap().memory_lower(), 640);

multiboot2/src/lib.rs

@@ -22,10 +22,13 @@
 //! ## Example
 //!
 //! ```rust
-//! use multiboot2::BootInformation;
-//! fn kmain(multiboot_info_ptr: u32) {
-//!     let boot_info = unsafe { BootInformation::load(multiboot_info_ptr as *const u8).unwrap() };
-//!     println!("{:?}", boot_info);
+//! use multiboot2::{BootInformation, BootInformationHeader};
+//!
+//! fn kernel_entry(mb_magic: u32, mbi_ptr: u32) {
+//!     if mb_magic == multiboot2::MAGIC {
+//!         let boot_info = unsafe { BootInformation::load(mbi_ptr as *const BootInformationHeader).unwrap() };
+//!         let _cmd = boot_info.command_line_tag();
+//!     } else { /* Panic or use multiboot1 flow. */ }
 //! }
 //! ```
 //!
@@ -41,6 +44,7 @@ extern crate alloc;
 extern crate std;
 
 use core::fmt;
+use core::mem::size_of;
 use derive_more::Display;
 // Must be public so that custom tags can be DSTs.
 pub use ptr_meta::Pointee;
@@ -102,7 +106,7 @@ pub const MAGIC: u32 = 0x36d76289;
 /// Deprecated. Please use BootInformation::load() instead.
 #[deprecated = "Please use BootInformation::load() instead."]
 pub unsafe fn load<'a>(address: usize) -> Result<BootInformation<'a>, MbiLoadError> {
-    let ptr = address as *const u8;
+    let ptr = address as *const BootInformationHeader;
     BootInformation::load(ptr)
 }
 
@@ -139,39 +143,58 @@ pub enum MbiLoadError {
 #[cfg(feature = "unstable")]
 impl core::error::Error for MbiLoadError {}
 
+/// The basic header of a boot information.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[repr(C, align(8))]
-struct BootInformationInner {
+pub struct BootInformationHeader {
+    // size is multiple of 8
     total_size: u32,
     _reserved: u32,
-    // followed by various, dynamically sized multiboot2 tags
-    tags: [Tag; 0],
+    // Followed by the boot information tags.
 }
 
-impl BootInformationInner {
+impl BootInformationHeader {
     #[cfg(feature = "builder")]
     fn new(total_size: u32) -> Self {
         Self {
             total_size,
             _reserved: 0,
-            tags: [],
         }
     }
+}
+
+#[cfg(feature = "builder")]
+impl StructAsBytes for BootInformationHeader {
+    fn byte_size(&self) -> usize {
+        core::mem::size_of::<Self>()
+    }
+}
 
+/// 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)]
+struct BootInformationInner {
+    header: BootInformationHeader,
+    tags: [u8],
+}
+
+impl BootInformationInner {
+    /// Checks if the MBI has a valid end tag by checking the end of the mbi's
+    /// bytes.
     fn has_valid_end_tag(&self) -> bool {
         let end_tag_prototype = EndTag::default();
 
-        let self_ptr = self as *const _;
-        let end_tag_addr = self_ptr as usize + (self.total_size - end_tag_prototype.size) as usize;
-        let end_tag = unsafe { &*(end_tag_addr as *const Tag) };
+        let self_ptr = unsafe { self.tags.as_ptr().sub(size_of::<BootInformationHeader>()) };
 
-        end_tag.typ == end_tag_prototype.typ && end_tag.size == end_tag_prototype.size
-    }
-}
+        let end_tag_ptr = unsafe {
+            self_ptr
+                .add(self.header.total_size as usize)
+                .sub(size_of::<EndTag>())
+        };
+        let end_tag = unsafe { &*(end_tag_ptr as *const EndTag) };
 
-#[cfg(feature = "builder")]
-impl StructAsBytes for BootInformationInner {
-    fn byte_size(&self) -> usize {
-        core::mem::size_of::<Self>()
+        end_tag.typ == end_tag_prototype.typ && end_tag.size == end_tag_prototype.size
     }
 }
 
@@ -186,11 +209,11 @@ impl BootInformation<'_> {
     /// ## Example
     ///
     /// ```rust
-    /// use multiboot2::BootInformation;
+    /// use multiboot2::{BootInformation, BootInformationHeader};
     ///
     /// fn kernel_entry(mb_magic: u32, mbi_ptr: u32) {
     ///     if mb_magic == multiboot2::MAGIC {
-    ///         let boot_info = unsafe { BootInformation::load(mbi_ptr as *const u8).unwrap() };
+    ///         let boot_info = unsafe { BootInformation::load(mbi_ptr as *const BootInformationHeader).unwrap() };
     ///         let _cmd = boot_info.command_line_tag();
     ///     } else { /* Panic or use multiboot1 flow. */ }
     /// }
@@ -203,20 +226,26 @@ impl BootInformation<'_> {
     ///   boot environments, such as UEFI.
     /// * The memory at `ptr` must not be modified after calling `load` or the
     ///   program may observe unsynchronized mutation.
-    pub unsafe fn load(ptr: *const u8) -> Result<Self, MbiLoadError> {
+    pub unsafe fn load(ptr: *const BootInformationHeader) -> Result<Self, MbiLoadError> {
         // null or not aligned
         if ptr.is_null() || ptr.align_offset(8) != 0 {
             return Err(MbiLoadError::IllegalAddress);
         }
 
-        let mbi = &*ptr.cast::<BootInformationInner>();
+        // mbi: reference to basic header
+        let mbi = &*ptr;
 
         // Check if total size is a multiple of 8.
         // See MbiLoadError::IllegalTotalSize for comments
         if mbi.total_size & 0b111 != 0 {
             return Err(MbiLoadError::IllegalTotalSize(mbi.total_size));
         }
 
+        let slice_size = mbi.total_size as usize - size_of::<BootInformationHeader>();
+        // mbi: reference to full mbi
+        let mbi = ptr_meta::from_raw_parts::<BootInformationInner>(ptr.cast(), slice_size);
+        let mbi = &*mbi;
+
         if !mbi.has_valid_end_tag() {
             return Err(MbiLoadError::NoEndTag);
         }
@@ -226,7 +255,7 @@ impl BootInformation<'_> {
 
     /// Get the start address of the boot info.
     pub fn start_address(&self) -> usize {
-        core::ptr::addr_of!(*self.0) as usize
+        self.as_ptr() as usize
     }
 
     /// Get the start address of the boot info as pointer.
@@ -248,7 +277,7 @@ impl BootInformation<'_> {
 
     /// Get the total size of the boot info struct.
     pub fn total_size(&self) -> usize {
-        self.0.total_size as usize
+        self.0.header.total_size as usize
     }
 
     /// Search for the basic memory info tag.
@@ -1520,7 +1549,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()) };
+        let bi = unsafe { BootInformation::load(bytes2.0.as_ptr().cast()) };
         let bi = bi.unwrap();
         let efi_mmap = bi.efi_memory_map_tag();
         assert!(efi_mmap.is_none());