refactor: multiboot tags reading and fix soundness issues

kernel/src/file/fs/ext2/inode.rs

@@ -420,7 +420,7 @@ impl Ext2INode {
 		let mut buf = vec![0u8; blk_size as _]?;
 		for off in &offsets[1..depth] {
 			read_block(blk.get() as _, blk_size, io, &mut buf)?;
-			let ents = bytes::slice_from_bytes(&buf);
+			let ents = bytes::slice_from_bytes(&buf).unwrap();
 			let Some(b) = check_blk_off(ents[*off], superblock)? else {
 				return Ok(None);
 			};
@@ -456,7 +456,7 @@ impl Ext2INode {
 		let mut buf = vec![0u8; blk_size as _]?;
 		for off in &offsets[1..depth] {
 			read_block(blk.get() as _, blk_size, io, &mut buf)?;
-			let ents = bytes::slice_from_bytes_mut(&mut buf);
+			let ents = bytes::slice_from_bytes_mut(&mut buf).unwrap();
 			let b = ensure_allocated(&mut ents[*off], superblock, io)?;
 			// TODO avoided if unnecessary
 			write_block(blk.get() as _, blk_size, io, &buf)?;
@@ -477,7 +477,7 @@ impl Ext2INode {
 		let blk_size = superblock.get_block_size();
 		let mut buf = vec![0u8; blk_size as _]?;
 		read_block(blk as _, blk_size, io, &mut buf)?;
-		let ents = bytes::slice_from_bytes_mut(&mut buf);
+		let ents = bytes::slice_from_bytes_mut(&mut buf).unwrap();
 		let b = &mut ents[*off];
 		// Handle child block and determine whether the entry in the current block should be freed
 		let free = Self::free_content_blk_impl(*b, &offsets[1..], superblock, io)?;
@@ -671,7 +671,8 @@ impl Ext2INode {
 		let blk_size = superblock.get_block_size();
 		let mut buf = vec![0; blk_size as _]?;
 		read_block(blk as _, blk_size, io, &mut buf)?;
-		for blk in bytes::slice_from_bytes(&buf) {
+		let ents = bytes::slice_from_bytes(&buf).unwrap();
+		for blk in ents {
 			let Some(blk) = check_blk_off(*blk, superblock)? else {
 				continue;
 			};

kernel/src/file/fs/initramfs.rs

@@ -22,7 +22,6 @@
 use crate::{
 	device, file,
 	file::{perm::AccessProfile, vfs, vfs::ResolutionSettings, FileType, Stat},
-	println,
 };
 use utils::{collections::path::Path, cpio::CPIOParser, errno, errno::EResult, ptr::arc::Arc};
 
@@ -37,7 +36,6 @@ fn update_parent<'p>(
 	parent: &mut (&'p Path, Arc<vfs::Entry>),
 	retry: bool,
 ) -> EResult<()> {
-	println!("chemaing {new}");
 	// Get the parent
 	let result = match new.strip_prefix(parent.0) {
 		Some(suffix) => {

kernel/src/kernel.rs

@@ -176,12 +176,10 @@ fn kernel_main_inner(magic: u32, multiboot_ptr: *const c_void) {
 	if magic != multiboot::BOOTLOADER_MAGIC || !multiboot_ptr.is_aligned_to(8) {
 		panic!("Bootloader non compliant with Multiboot2!");
 	}
-	unsafe {
-		multiboot::read_tags(multiboot_ptr);
-	}
+	let boot_info = unsafe { multiboot::read(multiboot_ptr) };
 
 	// Initialize memory management
-	memory::memmap::init(multiboot_ptr);
+	memory::memmap::init(boot_info);
 	#[cfg(debug_assertions)]
 	memory::memmap::print_entries();
 	memory::alloc::init();
@@ -199,8 +197,6 @@ fn kernel_main_inner(magic: u32, multiboot_ptr: *const c_void) {
 	#[cfg(test)]
 	kernel_selftest();
 
-	let boot_info = multiboot::get_boot_info();
-
 	// Parse bootloader command line arguments
 	let cmdline = boot_info.cmdline.unwrap_or_default();
 	let args_parser = match cmdline::ArgsParser::parse(cmdline) {

kernel/src/memory/memmap.rs

@@ -21,7 +21,7 @@
 //! information. These data are meant to be used by the memory allocators.
 
 use super::{kern_to_phys, stats};
-use crate::{elf::kernel::sections, multiboot};
+use crate::{elf::kernel::sections, multiboot, multiboot::BootInfo};
 use core::{cmp::*, ffi::c_void, iter, ptr::null};
 use utils::{limits::PAGE_SIZE, lock::once::OnceInit};
 
@@ -86,8 +86,7 @@ pub(crate) fn print_entries() {
 }
 
 /// Computes and returns the physical address to the end of the kernel's ELF sections' content.
-fn sections_end() -> *const c_void {
-	let boot_info = multiboot::get_boot_info();
+fn sections_end(boot_info: &BootInfo) -> *const c_void {
 	// The end of ELF sections list
 	let sections_list_end = (boot_info.elf_sections as usize
 		+ boot_info.elf_num as usize * boot_info.elf_entsize as usize)
@@ -105,14 +104,12 @@ fn sections_end() -> *const c_void {
 
 /// Returns the pointer to the beginning of the main physical allocatable memory
 /// and its size in number of pages.
-fn get_phys_main(multiboot_ptr: *const c_void) -> (*const c_void, usize) {
-	let boot_info = multiboot::get_boot_info();
-	// Get end of multiboot tags
-	let multiboot_tags_size = unsafe { multiboot::get_tags_size(multiboot_ptr) };
-	let multiboot_tags_end = ((multiboot_ptr as usize) + multiboot_tags_size) as *const _;
-	// Get end of the ELF sections
-	let sections_end = sections_end();
-	// Get end of the loaded initramfs
+fn get_phys_main(boot_info: &BootInfo) -> (*const c_void, usize) {
+	// The end address of multiboot tags
+	let multiboot_tags_end = (boot_info.tags_ptr as usize + boot_info.tags_size) as *const _;
+	// The end address of the ELF sections
+	let sections_end = sections_end(boot_info);
+	// The end address of the loaded initramfs
 	let initramfs_end = boot_info
 		.initramfs
 		.map(|initramfs| {
@@ -133,17 +130,16 @@ fn get_phys_main(multiboot_ptr: *const c_void) -> (*const c_void, usize) {
 }
 
 /// Fills the memory mapping structure according to Multiboot's information.
-pub(crate) fn init(multiboot_ptr: *const c_void) {
-	let boot_info = multiboot::get_boot_info();
+pub(crate) fn init(boot_info: &BootInfo) {
 	// Set memory information
-	let (main_begin, main_pages) = get_phys_main(multiboot_ptr);
+	let (phys_main_begin, phys_main_pages) = get_phys_main(boot_info);
 	let phys_map = PhysMapInfo {
 		memory_maps_size: boot_info.memory_maps_size,
 		memory_maps_entry_size: boot_info.memory_maps_entry_size,
 		memory_maps: boot_info.memory_maps,
 
-		phys_main_begin: main_begin,
-		phys_main_pages: main_pages,
+		phys_main_begin,
+		phys_main_pages,
 	};
 	unsafe {
 		MAP.init(phys_map);
@@ -151,5 +147,5 @@ pub(crate) fn init(multiboot_ptr: *const c_void) {
 	// Update memory stats
 	let mut stats = stats::MEM_INFO.lock();
 	stats.mem_total = min(boot_info.mem_upper, 4194304) as _; // TODO Handle 64-bits systems
-	stats.mem_free = main_pages * 4;
+	stats.mem_free = phys_main_pages * 4;
 }

kernel/src/multiboot.rs

@@ -133,12 +133,19 @@ impl MmapEntry {
 impl Tag {
 	/// Returns the pointer to the next Multiboot tag after the current tag.
 	pub fn next(&self) -> *const Self {
-		((self as *const _ as usize) + (((self.size + 7) & !7) as usize)) as *const _
+		unsafe {
+			(self as *const _ as *const c_void).add(((self.size + 7) & !7) as usize) as *const _
+		}
 	}
 }
 
 /// Kernel boot information provided by Multiboot, structured and filtered.
 pub struct BootInfo {
+	/// The pointer to the beginning of the tags.
+	pub tags_ptr: *const c_void,
+	/// The size of the tags in bytes.
+	pub tags_size: usize,
+
 	/// The command line used to boot the kernel.
 	pub cmdline: Option<&'static [u8]>,
 	/// The bootloader's name.
@@ -173,6 +180,8 @@ pub struct BootInfo {
 impl Default for BootInfo {
 	fn default() -> Self {
 		Self {
+			tags_ptr: null(),
+			tags_size: 0,
 			cmdline: None,
 			loader_name: None,
 			mem_lower: 0,
@@ -246,32 +255,23 @@ fn handle_tag(boot_info: &mut BootInfo, tag: &Tag) {
 	}
 }
 
-/// Returns the size in bytes of Multiboot tags pointed to by `ptr`.
+/// Reads the multiboot tags from the given `ptr` and returns relevant information.
 ///
 /// # Safety
 ///
 /// The caller must ensure the given pointer is valid and points to Multiboot tags.
-pub(crate) unsafe fn get_tags_size(ptr: *const c_void) -> usize {
-	let mut tag = ptr.offset(8) as *const Tag;
-	while (*tag).type_ != TAG_TYPE_END {
-		tag = (*tag).next();
-	}
-	tag = (*tag).next();
-	tag as usize - ptr as usize
-}
-
-/// Reads the multiboot tags from the given `ptr` and fills the boot
-/// information structure.
-///
-/// # Safety
-///
-/// The caller must ensure the given pointer is valid and points to Multiboot tags.
-pub(crate) unsafe fn read_tags(ptr: *const c_void) {
+pub(crate) unsafe fn read(ptr: *const c_void) -> &'static BootInfo {
 	let mut boot_info = BootInfo::default();
 	let mut tag = ptr.offset(8) as *const Tag;
 	while (*tag).type_ != TAG_TYPE_END {
 		handle_tag(&mut boot_info, &*tag);
 		tag = (*tag).next();
 	}
+	// Pass end tag
+	tag = (*tag).next();
+	boot_info.tags_ptr = ptr;
+	boot_info.tags_size = tag as usize - ptr as usize;
+	// Write to static variable and return
 	BOOT_INFO.init(boot_info);
+	BOOT_INFO.get()
 }

utils/src/bytes.rs

@@ -19,7 +19,7 @@
 //! Utility functions for byte representations of types.
 
 use core::{
-	mem::{size_of, size_of_val},
+	mem::{align_of, size_of, size_of_val},
 	slice,
 };
 
@@ -56,7 +56,9 @@ pub fn as_bytes_mut<T: ?Sized>(val: &mut T) -> &mut [u8] {
 ///
 /// If the size or alignment of the structure is invalid, the function returns `None`.
 pub fn from_bytes<T: AnyRepr>(slice: &[u8]) -> Option<&T> {
-	if size_of::<T>() <= slice.len() && slice.as_ptr().is_aligned() {
+	let size = size_of::<T>();
+	let align = align_of::<T>();
+	if size <= slice.len() && slice.as_ptr().is_aligned_to(align) {
 		// Safe because the slice is large enough
 		let val = unsafe { &*(slice.as_ptr() as *const T) };
 		Some(val)
@@ -69,13 +71,27 @@ pub fn from_bytes<T: AnyRepr>(slice: &[u8]) -> Option<&T> {
 ///
 /// If the length of `slice` is not a multiple of the size of `T`, the function truncates the
 /// output slice.
-pub fn slice_from_bytes<T: AnyRepr>(slice: &[u8]) -> &[T] {
+///
+/// If the alignment is invalid, the function returns `None`.
+pub fn slice_from_bytes<T: AnyRepr>(slice: &[u8]) -> Option<&[T]> {
 	let len = slice.len() / size_of::<T>();
-	unsafe { slice::from_raw_parts(slice.as_ptr() as _, len) }
+	let align = align_of::<T>();
+	if slice.as_ptr().is_aligned_to(align) {
+		let val = unsafe { slice::from_raw_parts(slice.as_ptr() as _, len) };
+		Some(val)
+	} else {
+		None
+	}
 }
 
 /// Same as [`slice_from_bytes`], but mutable.
-pub fn slice_from_bytes_mut<T: AnyRepr>(slice: &mut [u8]) -> &mut [T] {
+pub fn slice_from_bytes_mut<T: AnyRepr>(slice: &mut [u8]) -> Option<&mut [T]> {
 	let len = slice.len() / size_of::<T>();
-	unsafe { slice::from_raw_parts_mut(slice.as_mut_ptr() as _, len) }
+	let align = align_of::<T>();
+	if slice.as_ptr().is_aligned_to(align) {
+		let val = unsafe { slice::from_raw_parts_mut(slice.as_mut_ptr() as _, len) };
+		Some(val)
+	} else {
+		None
+	}
 }

utils/src/collections/path.rs

@@ -194,12 +194,12 @@ impl Path {
 	}
 
 	/// Returns the length of the path in bytes.
-	pub fn len(&self) -> usize {
+	pub const fn len(&self) -> usize {
 		self.0.len()
 	}
 
 	/// Tells whether the path is empty.
-	pub fn is_empty(&self) -> bool {
+	pub const fn is_empty(&self) -> bool {
 		self.0.is_empty()
 	}
 
@@ -209,7 +209,7 @@ impl Path {
 	}
 
 	/// Returns slice of the bytes representation of the path.
-	pub fn as_bytes(&self) -> &[u8] {
+	pub const fn as_bytes(&self) -> &[u8] {
 		&self.0
 	}
 

utils/src/lib.rs

@@ -37,6 +37,7 @@
 #![feature(dispatch_from_dyn)]
 #![feature(fmt_internals)]
 #![feature(is_sorted)]
+#![feature(pointer_is_aligned_to)]
 #![feature(portable_simd)]
 #![feature(set_ptr_value)]
 #![feature(trusted_len)]