multiboot2: more safety for tags that hold an str

As discussed in #22
we do not want to rely on bootloaders following
the specification in every case. Thus, this PR
adds more safety for when parsing strings from tags.

Author: phip1611

multiboot2/src/boot_loader_name.rs

@@ -1,4 +1,5 @@
 use crate::TagType;
+use core::str::Utf8Error;
 
 /// This tag contains the name of the bootloader that is booting the kernel.
 ///
@@ -9,11 +10,14 @@ use crate::TagType;
 pub struct BootLoaderNameTag {
     typ: TagType,
     size: u32,
+    /// Null-terminated UTF-8 string
     string: u8,
 }
 
 impl BootLoaderNameTag {
     /// Read the name of the bootloader that is booting the kernel.
+    /// This is an null-terminated UTF-8 string. If this returns `Err` then perhaps the memory
+    /// is invalid or the bootloader doesn't follow the spec.
     ///
     /// # Examples
     ///
@@ -23,11 +27,10 @@ impl BootLoaderNameTag {
     ///     assert_eq!("GRUB 2.02~beta3-5", name);
     /// }
     /// ```
-    pub fn name(&self) -> &str {
+    pub fn name(&self) -> Result<&str, Utf8Error> {
         use core::{mem, slice, str};
-        unsafe {
-            let strlen = self.size as usize - mem::size_of::<BootLoaderNameTag>();
-            str::from_utf8_unchecked(slice::from_raw_parts((&self.string) as *const u8, strlen))
-        }
+        let strlen = self.size as usize - mem::size_of::<BootLoaderNameTag>();
+        let bytes = unsafe { slice::from_raw_parts((&self.string) as *const u8, strlen) };
+        str::from_utf8(bytes)
     }
 }

multiboot2/src/command_line.rs

@@ -1,4 +1,9 @@
+//! Module for [CommandLineTag].
+
 use crate::TagType;
+use core::mem;
+use core::slice;
+use core::str::Utf8Error;
 
 /// This tag contains the command line string.
 ///
@@ -9,11 +14,14 @@ use crate::TagType;
 pub struct CommandLineTag {
     typ: TagType,
     size: u32,
+    /// Null-terminated UTF-8 string
     string: u8,
 }
 
 impl CommandLineTag {
     /// Read the command line string that is being passed to the booting kernel.
+    /// This is an null-terminated UTF-8 string. If this returns `Err` then perhaps the memory
+    /// is invalid or the bootloader doesn't follow the spec.
     ///
     /// # Examples
     ///
@@ -23,11 +31,9 @@ impl CommandLineTag {
     ///     assert_eq!("/bootarg", command_line);
     /// }
     /// ```
-    pub fn command_line(&self) -> &str {
-        use core::{mem, slice, str};
-        unsafe {
-            let strlen = self.size as usize - mem::size_of::<CommandLineTag>();
-            str::from_utf8_unchecked(slice::from_raw_parts((&self.string) as *const u8, strlen))
-        }
+    pub fn command_line(&self) -> Result<&str, Utf8Error> {
+        let strlen = self.size as usize - mem::size_of::<CommandLineTag>();
+        let bytes = unsafe { slice::from_raw_parts((&self.string) as *const u8, strlen) };
+        core::str::from_utf8(bytes)
     }
 }

multiboot2/src/lib.rs

@@ -352,14 +352,14 @@ impl fmt::Debug for BootInformation {
                 "boot_loader_name_tag",
                 &self
                     .boot_loader_name_tag()
-                    .map(|x| x.name())
+                    .and_then(|x| x.name().ok())
                     .unwrap_or("<unknown>"),
             )
             .field(
                 "command_line",
                 &self
                     .command_line_tag()
-                    .map(|x| x.command_line())
+                    .and_then(|x| x.command_line().ok())
                     .unwrap_or(""),
             )
             .field("memory_areas", &self.memory_map_tag())
@@ -532,7 +532,13 @@ mod tests {
         assert!(bi.elf_sections_tag().is_none());
         assert!(bi.memory_map_tag().is_none());
         assert!(bi.module_tags().next().is_none());
-        assert_eq!("name", bi.boot_loader_name_tag().unwrap().name());
+        assert_eq!(
+            "name",
+            bi.boot_loader_name_tag()
+                .expect("tag must be present")
+                .name()
+                .expect("must be valid utf8")
+        );
         assert!(bi.command_line_tag().is_none());
     }
 
@@ -1231,9 +1237,18 @@ mod tests {
         assert!(bi.module_tags().next().is_none());
         assert_eq!(
             "GRUB 2.02~beta3-5",
-            bi.boot_loader_name_tag().unwrap().name()
+            bi.boot_loader_name_tag()
+                .expect("tag must be present")
+                .name()
+                .expect("must be valid utf-8")
+        );
+        assert_eq!(
+            "",
+            bi.command_line_tag()
+                .expect("tag must present")
+                .command_line()
+                .expect("must be valid utf-8")
         );
-        assert_eq!("", bi.command_line_tag().unwrap().command_line());
 
         // Test the Framebuffer tag
         let fbi = bi.framebuffer_tag().unwrap();

multiboot2/src/module.rs

@@ -1,5 +1,6 @@
 use crate::tag_type::{Tag, TagIter, TagType};
 use core::fmt::{Debug, Formatter};
+use core::str::Utf8Error;
 
 /// This tag indicates to the kernel what boot module was loaded along with
 /// the kernel image, and where it can be found.
@@ -10,25 +11,23 @@ pub struct ModuleTag {
     size: u32,
     mod_start: u32,
     mod_end: u32,
-    /// Begin of the command line string.
+    /// Null-terminated UTF-8 string
     cmdline_str: u8,
 }
 
 impl ModuleTag {
-    // The multiboot specification defines the module str as valid utf-8 (zero terminated string),
-    // therefore this function produces defined behavior
-    /// Get the cmdline of the module. If the GRUB configuration contains
-    /// `module2 /foobar/some_boot_module --test cmdline-option`, then this method
+    /// Returns the cmdline of the module.
+    /// This is an null-terminated UTF-8 string. If this returns `Err` then perhaps the memory
+    /// is invalid or the bootloader doesn't follow the spec.
+    ///
+    /// For example: If the GRUB configuration contains
+    /// `module2 /foobar/some_boot_module --test cmdline-option` then this method
     /// will return `--test cmdline-option`.
-    pub fn cmdline(&self) -> &str {
+    pub fn cmdline(&self) -> Result<&str, Utf8Error> {
         use core::{mem, slice, str};
         let strlen = self.size as usize - mem::size_of::<ModuleTag>();
-        unsafe {
-            str::from_utf8_unchecked(slice::from_raw_parts(
-                &self.cmdline_str as *const u8,
-                strlen,
-            ))
-        }
+        let bytes = unsafe { slice::from_raw_parts((&self.cmdline_str) as *const u8, strlen) };
+        str::from_utf8(bytes)
     }
 
     /// Start address of the module.