From 2cdbe5f6ce350281cf619dcab4fa6eb6f863fe68 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Tue, 20 Aug 2024 17:01:31 +0200 Subject: [PATCH] workspace: streamline error types for load() functions --- multiboot2-common/src/bytes_ref.rs | 6 ++--- multiboot2-common/src/lib.rs | 6 ++--- multiboot2-header/Changelog.md | 2 ++ multiboot2-header/src/header.rs | 36 ++++++++++++++++++------------ multiboot2/Changelog.md | 2 ++ multiboot2/src/boot_information.rs | 26 ++++++++++++++------- multiboot2/src/lib.rs | 4 ++-- 7 files changed, 52 insertions(+), 30 deletions(-) diff --git a/multiboot2-common/src/bytes_ref.rs b/multiboot2-common/src/bytes_ref.rs index 30fa7096..45907418 100644 --- a/multiboot2-common/src/bytes_ref.rs +++ b/multiboot2-common/src/bytes_ref.rs @@ -22,7 +22,7 @@ impl<'a, H: Header> TryFrom<&'a [u8]> for BytesRef<'a, H> { fn try_from(bytes: &'a [u8]) -> Result { if bytes.len() < mem::size_of::() { - return Err(MemoryError::MinLengthNotSatisfied); + return Err(MemoryError::ShorterThanHeader); } // Doesn't work as expected: if align_of_val(&value[0]) < ALIGNMENT { if bytes.as_ptr().align_offset(ALIGNMENT) != 0 { @@ -57,13 +57,13 @@ mod tests { let empty: &[u8] = &[]; assert_eq!( BytesRef::<'_, DummyTestHeader>::try_from(empty), - Err(MemoryError::MinLengthNotSatisfied) + Err(MemoryError::ShorterThanHeader) ); let slice = &[0_u8, 1, 2, 3, 4, 5, 6]; assert_eq!( BytesRef::<'_, DummyTestHeader>::try_from(&slice[..]), - Err(MemoryError::MinLengthNotSatisfied) + Err(MemoryError::ShorterThanHeader) ); let slice = AlignedBytes([0_u8, 1, 2, 3, 4, 5, 6, 7, 0, 0, 0]); diff --git a/multiboot2-common/src/lib.rs b/multiboot2-common/src/lib.rs index 42d41b16..6f01e285 100644 --- a/multiboot2-common/src/lib.rs +++ b/multiboot2-common/src/lib.rs @@ -225,7 +225,7 @@ impl DynSizedStructure { let hdr = unsafe { &*ptr }; if hdr.payload_len() > bytes.len() { - return Err(MemoryError::InvalidHeaderSize); + return Err(MemoryError::InvalidReportedTotalSize); } // At this point we know that the memory slice fulfills the base @@ -308,14 +308,14 @@ pub enum MemoryError { WrongAlignment, /// The memory must cover at least the length of the sized structure header /// type. - MinLengthNotSatisfied, + ShorterThanHeader, /// The buffer misses the terminating padding to the next alignment /// boundary. The padding is relevant to satisfy Rustc/Miri, but also the /// spec mandates that the padding is added. MissingPadding, /// The size-property has an illegal value that can't be fulfilled with the /// given bytes. - InvalidHeaderSize, + InvalidReportedTotalSize, } #[cfg(feature = "unstable")] diff --git a/multiboot2-header/Changelog.md b/multiboot2-header/Changelog.md index def42138..57e517ea 100644 --- a/multiboot2-header/Changelog.md +++ b/multiboot2-header/Changelog.md @@ -13,6 +13,8 @@ place, please head to the documentation of `multiboot2-common`. - **Breaking** All functions that returns something useful are now `#[must_use]` - **Breaking** The builder type is now just called `Builder`. This needs the `builder` feature. +- **Breaking:** The error type returned by `Multiboot2Header::load` has been + changed. - Updated to latest `multiboot2` dependency ## 0.4.0 (2024-05-01) diff --git a/multiboot2-header/src/header.rs b/multiboot2-header/src/header.rs index 6408aa55..a55b14e4 100644 --- a/multiboot2-header/src/header.rs +++ b/multiboot2-header/src/header.rs @@ -4,10 +4,12 @@ use crate::{ HeaderTagType, InformationRequestHeaderTag, ModuleAlignHeaderTag, RelocatableHeaderTag, TagIter, }; +#[cfg(feature = "unstable")] +use core::error::Error; use core::fmt::{Debug, Formatter}; use core::mem::size_of; use core::ptr::NonNull; -use multiboot2_common::{DynSizedStructure, Header, Tag, ALIGNMENT}; +use multiboot2_common::{DynSizedStructure, Header, MemoryError, Tag, ALIGNMENT}; /// Magic value for a [`Multiboot2Header`], as defined by the spec. pub const MAGIC: u32 = 0xe85250d6; @@ -35,8 +37,8 @@ impl<'a> Multiboot2Header<'a> { /// This function may produce undefined behaviour, if the provided `addr` is not a valid /// Multiboot2 header pointer. pub unsafe fn load(ptr: *const Multiboot2BasicHeader) -> Result { - let ptr = NonNull::new(ptr.cast_mut()).ok_or(LoadError::InvalidAddress)?; - let inner = DynSizedStructure::ref_from_ptr(ptr).map_err(|_e| LoadError::TooSmall)?; + let ptr = NonNull::new(ptr.cast_mut()).ok_or(LoadError::Memory(MemoryError::Null))?; + let inner = DynSizedStructure::ref_from_ptr(ptr).map_err(LoadError::Memory)?; let this = Self(inner); let header = this.0.header(); @@ -58,7 +60,7 @@ impl<'a> Multiboot2Header<'a> { /// If there is no header, it returns `None`. pub fn find_header(buffer: &[u8]) -> Result, LoadError> { if buffer.as_ptr().align_offset(ALIGNMENT) != 0 { - return Err(LoadError::InvalidAddress); + return Err(LoadError::Memory(MemoryError::WrongAlignment)); } let mut windows = buffer[0..8192].windows(4); @@ -70,7 +72,7 @@ impl<'a> Multiboot2Header<'a> { if idx % 8 == 0 { idx } else { - return Err(LoadError::InvalidAddress); + return Err(LoadError::Memory(MemoryError::WrongAlignment)); } } None => return Ok(None), @@ -87,7 +89,7 @@ impl<'a> Multiboot2Header<'a> { let header_length: usize = u32::from_le_bytes( windows .next() - .ok_or(LoadError::TooSmall)? + .ok_or(LoadError::Memory(MemoryError::MissingPadding))? .try_into() .unwrap(), // 4 bytes are a u32 ) @@ -221,22 +223,28 @@ impl Debug for Multiboot2Header<'_> { } } -/// Errors that can occur when parsing a header from a slice. -/// See [`Multiboot2Header::find_header`]. +/// Errors that occur when a chunk of memory can't be parsed as +/// [`Multiboot2Header`]. #[derive(Copy, Clone, Debug, derive_more::Display, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum LoadError { - /// The checksum does not match the data. + /// The provided checksum does not match the expected value. ChecksumMismatch, - /// The header is not properly 64-bit aligned (or a null pointer). - InvalidAddress, /// The header does not contain the correct magic number. MagicNotFound, - /// The header is truncated. - TooSmall, + /// The provided memory can't be parsed as [`Multiboot2Header`]. + /// See [`MemoryError`]. + Memory(MemoryError), } #[cfg(feature = "unstable")] -impl core::error::Error for LoadError {} +impl Error for LoadError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::Memory(inner) => Some(inner), + _ => None, + } + } +} /// The "basic" Multiboot2 header. This means only the properties, that are known during /// compile time. All other information are derived during runtime from the size property. diff --git a/multiboot2/Changelog.md b/multiboot2/Changelog.md index bdb2e493..fce99e9e 100644 --- a/multiboot2/Changelog.md +++ b/multiboot2/Changelog.md @@ -18,6 +18,8 @@ place, please head to the documentation of `multiboot2-common`. - **Breaking:** The trait `TagTrait` was removed and was replaced by a new `Tag` trait coming from `multiboot2-common`. This only affects you if you provide custom tag types for the library. +- **Breaking:** The error type returned by `BootInformation::load` has been + changed. **General Note on Safety and UB (TL;DR: Crate is Safe)** diff --git a/multiboot2/src/boot_information.rs b/multiboot2/src/boot_information.rs index b4d5c028..6eb6628d 100644 --- a/multiboot2/src/boot_information.rs +++ b/multiboot2/src/boot_information.rs @@ -8,16 +8,19 @@ use crate::{ ElfSectionIter, ElfSectionsTag, EndTag, FramebufferTag, ImageLoadPhysAddrTag, MemoryMapTag, ModuleIter, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagIter, TagType, VBEInfoTag, }; +#[cfg(feature = "unstable")] +use core::error::Error; use core::fmt; use core::mem; use core::ptr::NonNull; use derive_more::Display; use multiboot2_common::{DynSizedStructure, Header, MaybeDynSized, MemoryError, Tag}; -/// Error type that describes errors while loading/parsing a multiboot2 information structure -/// from a given address. +/// Errors that occur when a chunk of memory can't be parsed as +/// [`BootInformation`]. #[derive(Display, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub enum MbiLoadError { +pub enum LoadError { + /// The provided memory can't be parsed as [`BootInformation`]. /// See [`MemoryError`]. Memory(MemoryError), /// Missing mandatory end tag. @@ -25,7 +28,14 @@ pub enum MbiLoadError { } #[cfg(feature = "unstable")] -impl core::error::Error for MbiLoadError {} +impl Error for LoadError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::Memory(inner) => Some(inner), + Self::NoEndTag => None, + } + } +} /// The basic header of a [`BootInformation`] as sized Rust type. #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -91,13 +101,13 @@ impl<'a> BootInformation<'a> { /// 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 BootInformationHeader) -> Result { - let ptr = NonNull::new(ptr.cast_mut()).ok_or(MbiLoadError::Memory(MemoryError::Null))?; - let inner = DynSizedStructure::ref_from_ptr(ptr).map_err(MbiLoadError::Memory)?; + pub unsafe fn load(ptr: *const BootInformationHeader) -> Result { + let ptr = NonNull::new(ptr.cast_mut()).ok_or(LoadError::Memory(MemoryError::Null))?; + let inner = DynSizedStructure::ref_from_ptr(ptr).map_err(LoadError::Memory)?; let this = Self(inner); if !this.has_valid_end_tag() { - return Err(MbiLoadError::NoEndTag); + return Err(LoadError::NoEndTag); } Ok(this) } diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index 77c0d28b..cc8f7f40 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -84,7 +84,7 @@ mod vbe_info; pub use multiboot2_common::{DynSizedStructure, MaybeDynSized, Tag}; -pub use boot_information::{BootInformation, BootInformationHeader, MbiLoadError}; +pub use boot_information::{BootInformation, BootInformationHeader, LoadError}; pub use boot_loader_name::BootLoaderNameTag; #[cfg(feature = "builder")] pub use builder::Builder; @@ -1098,7 +1098,7 @@ mod tests { /// This test succeeds if it compiles. fn mbi_load_error_implements_error() { fn consumer(_e: E) {} - consumer(MbiLoadError::IllegalAddress) + consumer(LoadError::NoEndTag) } /// Example for a custom tag.