Skip to content

multiboot2: fix memory issue in boxed_dst_tag #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 21, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 84 additions & 21 deletions multiboot2/src/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,37 +8,100 @@ pub use information::InformationBuilder;
use alloc::alloc::alloc;
use alloc::boxed::Box;
use core::alloc::Layout;
use core::mem::size_of;
use core::mem::{align_of_val, size_of, size_of_val};
use core::ops::Deref;

use crate::{TagTrait, TagTypeId};
use crate::{Tag, TagTrait, TagTypeId};

/// Create a boxed tag with the given content.
///
/// # Parameters
/// - `typ` - The given [`TagTypeId`]
/// - `content` - All payload bytes of the DST tag without the tag type or the
/// size. The memory is only read and can be discarded afterwards.
pub(super) fn boxed_dst_tag<T: TagTrait<Metadata = usize> + ?Sized>(
typ: impl Into<TagTypeId>,
content: &[u8],
) -> Box<T> {
// based on https://stackoverflow.com/a/64121094/2192464
let (layout, size_offset) = Layout::new::<TagTypeId>()
.extend(Layout::new::<u32>())
.unwrap();
let (layout, inner_offset) = layout
.extend(Layout::array::<usize>(content.len()).unwrap())
.unwrap();
// Currently, I do not find a nice way of making this dynamic so that also
// miri is happy. But it seems that 4 is fine.
const ALIGN: usize = 4;

let tag_size = size_of::<TagTypeId>() + size_of::<u32>() + content.len();
// round up to the next multiple of 8
// Rust uses this convention for all types. I found out so by checking
// miris output of the corresponding unit test.
let alloc_size = (tag_size + 7) & !0b111;
let layout = Layout::from_size_align(alloc_size, ALIGN).unwrap();
let ptr = unsafe { alloc(layout) };
assert!(!ptr.is_null());

// write tag content to memory
unsafe {
// initialize the content as good as we can
ptr.cast::<TagTypeId>().write(typ.into());
ptr.add(size_offset).cast::<u32>().write(
(content.len() + size_of::<TagTypeId>() + size_of::<u32>())
.try_into()
.unwrap(),
);
// initialize body
let content_ptr = ptr.add(inner_offset);
for (idx, val) in content.iter().enumerate() {
content_ptr.add(idx).write(*val);
// write tag type
let ptrx = ptr.cast::<TagTypeId>();
ptrx.write(typ.into());

// write tag size
let ptrx = ptrx.add(1).cast::<u32>();
ptrx.write(tag_size as u32);

// write rest of content
let ptrx = ptrx.add(1).cast::<u8>();
let tag_content_slice = core::slice::from_raw_parts_mut(ptrx, content.len());
for (i, &byte) in content.iter().enumerate() {
tag_content_slice[i] = byte;
}
}

let base_tag = unsafe { &*ptr.cast::<Tag>() };
let raw: *mut T = ptr_meta::from_raw_parts_mut(ptr.cast(), T::dst_size(base_tag));

unsafe {
let boxed = Box::from_raw(raw);
let reference: &T = boxed.deref();
// If this panics, please create an issue on GitHub.
assert_eq!(size_of_val(reference), alloc_size);
assert_eq!(align_of_val(reference), ALIGN);
boxed
}
}

#[cfg(test)]
mod tests {
use super::*;

const METADATA_SIZE: usize = 8;

#[derive(ptr_meta::Pointee)]
#[repr(C)]
struct CustomTag {
typ: TagTypeId,
size: u32,
string: [u8],
}

impl CustomTag {
fn string(&self) -> Result<&str, core::str::Utf8Error> {
Tag::get_dst_str_slice(&self.string)
}
Box::from_raw(ptr_meta::from_raw_parts_mut(ptr as *mut (), content.len()))
}

impl TagTrait for CustomTag {
fn dst_size(base_tag: &Tag) -> usize {
assert!(base_tag.size as usize >= METADATA_SIZE);
base_tag.size as usize - METADATA_SIZE
}
}

#[test]
fn test_boxed_dst_tag() {
let tag_type_id = 1337_u32;
let content = "hallo";

let tag = boxed_dst_tag::<CustomTag>(tag_type_id, content.as_bytes());
assert_eq!(tag.typ, tag_type_id);
assert_eq!(tag.size as usize, METADATA_SIZE + content.len());
assert_eq!(tag.string(), Ok(content));
}
}