Browse Source

multiboot2: fix possible panics for elf sections

Philipp Schuster 2 years ago
parent
commit
ae6d501f58
4 changed files with 36 additions and 25 deletions
  1. 1 0
      multiboot2/Cargo.toml
  2. 5 2
      multiboot2/Changelog.md
  3. 12 5
      multiboot2/src/elf_sections.rs
  4. 18 18
      multiboot2/src/lib.rs

+ 1 - 0
multiboot2/Cargo.toml

@@ -39,3 +39,4 @@ unstable = []
 [dependencies]
 [dependencies]
 bitflags = "1"
 bitflags = "1"
 derive_more = { version = "0.99.17", default-features = false, features = ["display"] }
 derive_more = { version = "0.99.17", default-features = false, features = ["display"] }
+log = { version = "0.4.17", default-features = false }

+ 5 - 2
multiboot2/Changelog.md

@@ -1,15 +1,18 @@
 # CHANGELOG for crate `multiboot2`
 # CHANGELOG for crate `multiboot2`
 
 
 ## 0.15.1 (2023-03-18)
 ## 0.15.1 (2023-03-18)
-- **BREAKING** `MemoryMapTag::all_memory_areas` was renamed to `memory_areas`
+- **BREAKING** `MemoryMapTag::all_memory_areas()` was renamed to `memory_areas`
   and now returns `MemoryAreaIter` instead of
   and now returns `MemoryAreaIter` instead of
   `impl Iterator<Item = &MemoryArea>`. Experience showed that its better to
   `impl Iterator<Item = &MemoryArea>`. Experience showed that its better to
   return the specific iterator whenever possible.
   return the specific iterator whenever possible.
-- **BREAKING** `MemoryMapTag::memory_areas` was renamed to
+- **BREAKING** `MemoryMapTag::memory_areas()` was renamed to
   `available_memory_areas`
   `available_memory_areas`
   (_Sorry for the breaking changes in a minor release, but I just stumbled upon
   (_Sorry for the breaking changes in a minor release, but I just stumbled upon
   this und since the last breaking release was just yesterday, users have to
   this und since the last breaking release was just yesterday, users have to
   deal with changes anyway._)
   deal with changes anyway._)
+- **BREAKING** `ElfSection::name()` now returns a Result instead of just the
+  value. This prevents possible panics.
+- fix: prevent a possible panic in `ElfSection::section_type()`
 
 
 ## 0.15.0 (2023-03-17)
 ## 0.15.0 (2023-03-17)
 - **BREAKING** MSRV is 1.56.1
 - **BREAKING** MSRV is 1.56.1

+ 12 - 5
multiboot2/src/elf_sections.rs

@@ -1,6 +1,7 @@
 use crate::tag_type::Tag;
 use crate::tag_type::Tag;
 use crate::TagType;
 use crate::TagType;
 use core::fmt::{Debug, Formatter};
 use core::fmt::{Debug, Formatter};
+use core::str::Utf8Error;
 
 
 /// This tag contains section header table from an ELF kernel.
 /// This tag contains section header table from an ELF kernel.
 ///
 ///
@@ -178,7 +179,13 @@ impl ElfSection {
             11 => ElfSectionType::DynamicLoaderSymbolTable,
             11 => ElfSectionType::DynamicLoaderSymbolTable,
             0x6000_0000..=0x6FFF_FFFF => ElfSectionType::EnvironmentSpecific,
             0x6000_0000..=0x6FFF_FFFF => ElfSectionType::EnvironmentSpecific,
             0x7000_0000..=0x7FFF_FFFF => ElfSectionType::ProcessorSpecific,
             0x7000_0000..=0x7FFF_FFFF => ElfSectionType::ProcessorSpecific,
-            _ => panic!(),
+            e => {
+                log::warn!(
+                    "Unknown section type {:x}. Treating as ElfSectionType::Unused",
+                    e
+                );
+                ElfSectionType::Unused
+            }
         }
         }
     }
     }
 
 
@@ -188,7 +195,7 @@ impl ElfSection {
     }
     }
 
 
     /// Read the name of the section.
     /// Read the name of the section.
-    pub fn name(&self) -> &str {
+    pub fn name(&self) -> Result<&str, Utf8Error> {
         use core::{slice, str};
         use core::{slice, str};
 
 
         let name_ptr = unsafe { self.string_table().offset(self.get().name_index() as isize) };
         let name_ptr = unsafe { self.string_table().offset(self.get().name_index() as isize) };
@@ -202,7 +209,7 @@ impl ElfSection {
             len as usize
             len as usize
         };
         };
 
 
-        str::from_utf8(unsafe { slice::from_raw_parts(name_ptr, strlen) }).unwrap()
+        str::from_utf8(unsafe { slice::from_raw_parts(name_ptr, strlen) })
     }
     }
 
 
     /// Get the physical start address of the section.
     /// Get the physical start address of the section.
@@ -246,7 +253,7 @@ impl ElfSection {
         match self.entry_size {
         match self.entry_size {
             40 => unsafe { &*(self.inner as *const ElfSectionInner32) },
             40 => unsafe { &*(self.inner as *const ElfSectionInner32) },
             64 => unsafe { &*(self.inner as *const ElfSectionInner64) },
             64 => unsafe { &*(self.inner as *const ElfSectionInner64) },
-            _ => panic!(),
+            s => panic!("Unexpected entry size: {}", s),
         }
         }
     }
     }
 
 
@@ -254,7 +261,7 @@ impl ElfSection {
         let addr = match self.entry_size {
         let addr = match self.entry_size {
             40 => (*(self.string_section as *const ElfSectionInner32)).addr as usize,
             40 => (*(self.string_section as *const ElfSectionInner32)).addr as usize,
             64 => (*(self.string_section as *const ElfSectionInner64)).addr as usize,
             64 => (*(self.string_section as *const ElfSectionInner64)).addr as usize,
-            _ => panic!(),
+            s => panic!("Unexpected entry size: {}", s),
         };
         };
         (addr + self.offset) as *const _
         (addr + self.offset) as *const _
     }
     }

+ 18 - 18
multiboot2/src/lib.rs

@@ -1188,15 +1188,15 @@ mod tests {
         assert_eq!(bytes.len(), bi.total_size());
         assert_eq!(bytes.len(), bi.total_size());
         let es = bi.elf_sections_tag().unwrap();
         let es = bi.elf_sections_tag().unwrap();
         let mut s = es.sections();
         let mut s = es.sections();
-        let s1 = s.next().unwrap();
-        assert_eq!(".rodata", s1.name());
+        let s1 = s.next().expect("Should have one more section");
+        assert_eq!(".rodata", s1.name().expect("Should be valid utf-8"));
         assert_eq!(0xFFFF_8000_0010_0000, s1.start_address());
         assert_eq!(0xFFFF_8000_0010_0000, s1.start_address());
         assert_eq!(0xFFFF_8000_0010_3000, s1.end_address());
         assert_eq!(0xFFFF_8000_0010_3000, s1.end_address());
         assert_eq!(0x0000_0000_0000_3000, s1.size());
         assert_eq!(0x0000_0000_0000_3000, s1.size());
         assert_eq!(ElfSectionFlags::ALLOCATED, s1.flags());
         assert_eq!(ElfSectionFlags::ALLOCATED, s1.flags());
         assert_eq!(ElfSectionType::ProgramSection, s1.section_type());
         assert_eq!(ElfSectionType::ProgramSection, s1.section_type());
-        let s2 = s.next().unwrap();
-        assert_eq!(".text", s2.name());
+        let s2 = s.next().expect("Should have one more section");
+        assert_eq!(".text", s2.name().expect("Should be valid utf-8"));
         assert_eq!(0xFFFF_8000_0010_3000, s2.start_address());
         assert_eq!(0xFFFF_8000_0010_3000, s2.start_address());
         assert_eq!(0xFFFF_8000_0010_C000, s2.end_address());
         assert_eq!(0xFFFF_8000_0010_C000, s2.end_address());
         assert_eq!(0x0000_0000_0000_9000, s2.size());
         assert_eq!(0x0000_0000_0000_9000, s2.size());
@@ -1205,8 +1205,8 @@ mod tests {
             s2.flags()
             s2.flags()
         );
         );
         assert_eq!(ElfSectionType::ProgramSection, s2.section_type());
         assert_eq!(ElfSectionType::ProgramSection, s2.section_type());
-        let s3 = s.next().unwrap();
-        assert_eq!(".data", s3.name());
+        let s3 = s.next().expect("Should have one more section");
+        assert_eq!(".data", s3.name().expect("Should be valid utf-8"));
         assert_eq!(0xFFFF_8000_0010_C000, s3.start_address());
         assert_eq!(0xFFFF_8000_0010_C000, s3.start_address());
         assert_eq!(0xFFFF_8000_0010_E000, s3.end_address());
         assert_eq!(0xFFFF_8000_0010_E000, s3.end_address());
         assert_eq!(0x0000_0000_0000_2000, s3.size());
         assert_eq!(0x0000_0000_0000_2000, s3.size());
@@ -1215,8 +1215,8 @@ mod tests {
             s3.flags()
             s3.flags()
         );
         );
         assert_eq!(ElfSectionType::ProgramSection, s3.section_type());
         assert_eq!(ElfSectionType::ProgramSection, s3.section_type());
-        let s4 = s.next().unwrap();
-        assert_eq!(".bss", s4.name());
+        let s4 = s.next().expect("Should have one more section");
+        assert_eq!(".bss", s4.name().expect("Should be valid utf-8"));
         assert_eq!(0xFFFF_8000_0010_E000, s4.start_address());
         assert_eq!(0xFFFF_8000_0010_E000, s4.start_address());
         assert_eq!(0xFFFF_8000_0011_3000, s4.end_address());
         assert_eq!(0xFFFF_8000_0011_3000, s4.end_address());
         assert_eq!(0x0000_0000_0000_5000, s4.size());
         assert_eq!(0x0000_0000_0000_5000, s4.size());
@@ -1225,8 +1225,8 @@ mod tests {
             s4.flags()
             s4.flags()
         );
         );
         assert_eq!(ElfSectionType::Uninitialized, s4.section_type());
         assert_eq!(ElfSectionType::Uninitialized, s4.section_type());
-        let s5 = s.next().unwrap();
-        assert_eq!(".data.rel.ro", s5.name());
+        let s5 = s.next().expect("Should have one more section");
+        assert_eq!(".data.rel.ro", s5.name().expect("Should be valid utf-8"));
         assert_eq!(0xFFFF_8000_0011_3000, s5.start_address());
         assert_eq!(0xFFFF_8000_0011_3000, s5.start_address());
         assert_eq!(0xFFFF_8000_0011_3000, s5.end_address());
         assert_eq!(0xFFFF_8000_0011_3000, s5.end_address());
         assert_eq!(0x0000_0000_0000_0000, s5.size());
         assert_eq!(0x0000_0000_0000_0000, s5.size());
@@ -1235,22 +1235,22 @@ mod tests {
             s5.flags()
             s5.flags()
         );
         );
         assert_eq!(ElfSectionType::ProgramSection, s5.section_type());
         assert_eq!(ElfSectionType::ProgramSection, s5.section_type());
-        let s6 = s.next().unwrap();
-        assert_eq!(".symtab", s6.name());
+        let s6 = s.next().expect("Should have one more section");
+        assert_eq!(".symtab", s6.name().expect("Should be valid utf-8"));
         assert_eq!(0x0000_0000_0011_3000, s6.start_address());
         assert_eq!(0x0000_0000_0011_3000, s6.start_address());
         assert_eq!(0x0000_0000_0011_5BE0, s6.end_address());
         assert_eq!(0x0000_0000_0011_5BE0, s6.end_address());
         assert_eq!(0x0000_0000_0000_2BE0, s6.size());
         assert_eq!(0x0000_0000_0000_2BE0, s6.size());
         assert_eq!(ElfSectionFlags::empty(), s6.flags());
         assert_eq!(ElfSectionFlags::empty(), s6.flags());
         assert_eq!(ElfSectionType::LinkerSymbolTable, s6.section_type());
         assert_eq!(ElfSectionType::LinkerSymbolTable, s6.section_type());
-        let s7 = s.next().unwrap();
-        assert_eq!(".strtab", s7.name());
+        let s7 = s.next().expect("Should have one more section");
+        assert_eq!(".strtab", s7.name().expect("Should be valid utf-8"));
         assert_eq!(0x0000_0000_0011_5BE0, s7.start_address());
         assert_eq!(0x0000_0000_0011_5BE0, s7.start_address());
         assert_eq!(0x0000_0000_0011_9371, s7.end_address());
         assert_eq!(0x0000_0000_0011_9371, s7.end_address());
         assert_eq!(0x0000_0000_0000_3791, s7.size());
         assert_eq!(0x0000_0000_0000_3791, s7.size());
         assert_eq!(ElfSectionFlags::empty(), s7.flags());
         assert_eq!(ElfSectionFlags::empty(), s7.flags());
         assert_eq!(ElfSectionType::StringTable, s7.section_type());
         assert_eq!(ElfSectionType::StringTable, s7.section_type());
-        let s8 = s.next().unwrap();
-        assert_eq!(".shstrtab", s8.name());
+        let s8 = s.next().expect("Should have one more section");
+        assert_eq!(".shstrtab", s8.name().expect("Should be valid utf-8"));
         assert_eq!(string_addr, s8.start_address());
         assert_eq!(string_addr, s8.start_address());
         assert_eq!(string_addr + string_bytes.len() as u64, s8.end_address());
         assert_eq!(string_addr + string_bytes.len() as u64, s8.end_address());
         assert_eq!(string_bytes.len() as u64, s8.size());
         assert_eq!(string_bytes.len() as u64, s8.size());
@@ -1373,8 +1373,8 @@ mod tests {
         assert_eq!(bytes.0.len(), bi.total_size());
         assert_eq!(bytes.0.len(), bi.total_size());
         let es = bi.elf_sections_tag().unwrap();
         let es = bi.elf_sections_tag().unwrap();
         let mut s = es.sections();
         let mut s = es.sections();
-        let s1 = s.next().unwrap();
-        assert_eq!(".shstrtab", s1.name());
+        let s1 = s.next().expect("Should have one more section");
+        assert_eq!(".shstrtab", s1.name().expect("Should be valid utf-8"));
         assert_eq!(string_addr, s1.start_address());
         assert_eq!(string_addr, s1.start_address());
         assert_eq!(string_addr + string_bytes.0.len() as u64, s1.end_address());
         assert_eq!(string_addr + string_bytes.0.len() as u64, s1.end_address());
         assert_eq!(string_bytes.0.len() as u64, s1.size());
         assert_eq!(string_bytes.0.len() as u64, s1.size());