Skip to content
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

uefi-raw: Add explicit padding field for MemoryDescriptor #1334

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions uefi-raw/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# uefi-raw - [Unreleased]

## Changed
- `MemoryDescriptor` now has an explicit padding field, allowing trivial
memory-safe serialization and deserialization

# uefi-raw - 0.8.0 (2024-09-09)

Expand Down
5 changes: 5 additions & 0 deletions uefi-raw/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
unused
)]

// Useful for convenience in tests, such as `dbg!`
#[cfg_attr(test, macro_use)]
#[cfg(test)]
extern crate std;

#[macro_use]
mod enums;

Expand Down
38 changes: 36 additions & 2 deletions uefi-raw/src/table/boot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,8 @@ bitflags! {
pub struct MemoryDescriptor {
/// Type of memory occupying this range.
pub ty: MemoryType,
// Implicit 32-bit padding.
/// Padding field. Ignore.
pub padding0: u32,
/// Starting physical address.
pub phys_start: PhysicalAddress,
/// Starting virtual address.
Expand All @@ -370,6 +371,7 @@ impl Default for MemoryDescriptor {
fn default() -> Self {
Self {
ty: MemoryType::RESERVED,
padding0: 0,
phys_start: 0,
virt_start: 0,
page_count: 0,
Expand All @@ -379,7 +381,7 @@ impl Default for MemoryDescriptor {
}

newtype_enum! {
/// The type of a memory range.
/// The type of memory range.
///
/// UEFI allows firmwares and operating systems to introduce new memory types
/// in the `0x7000_0000..=0xFFFF_FFFF` range. Therefore, we don't know the full set
Expand Down Expand Up @@ -484,3 +486,35 @@ pub enum Tpl: usize => {
/// Note that this is not necessarily the processor's page size. The UEFI page
/// size is always 4 KiB.
pub const PAGE_SIZE: usize = 4096;

#[cfg(test)]
mod tests {
use core::{mem, slice};
use super::*;

// This tests succeeds if Miri doesn't complain about uninitialized memory.
#[test]
fn memory_descriptor_trivial_serialization() {
let descs = [
MemoryDescriptor {
ty: MemoryType::CONVENTIONAL,
padding0: 0,
phys_start: 0x1000,
virt_start: 0x1000,
page_count: 1,
att: Default::default(),
},
];


let raw_bytes: &[u8] = {
let ptr = descs.as_ptr().cast::<u8>();
let len = mem::size_of_val(&descs);
unsafe { slice::from_raw_parts(ptr.cast(), len) }
};

// Test succeeds if Miri doesn't complain about initialized memory
// (implicit padding fields).
dbg!(&raw_bytes);
}
}
7 changes: 6 additions & 1 deletion uefi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
//!
//! Status::SUCCESS
//! }
//! # extern crate std;
//! # extern crate std; // For panic handler
//! ```
//!
//! Please find more info in our [Rust UEFI Book].
Expand Down Expand Up @@ -235,6 +235,11 @@ extern crate self as uefi;
#[macro_use]
extern crate uefi_raw;

// Useful for convenience in tests, such as `dbg!`
#[cfg_attr(test, macro_use)]
#[cfg(test)]
extern crate std;

#[macro_use]
pub mod data_types;
pub mod allocator;
Expand Down
3 changes: 3 additions & 0 deletions uefi/src/mem/memory_map/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,20 +432,23 @@ mod tests {
const BASE_MMAP_UNSORTED: [MemoryDescriptor; 3] = [
MemoryDescriptor {
ty: MemoryType::CONVENTIONAL,
padding0: 0,
phys_start: 0x3000,
virt_start: 0x3000,
page_count: 1,
att: MemoryAttribute::WRITE_BACK,
},
MemoryDescriptor {
ty: MemoryType::CONVENTIONAL,
padding0: 0,
phys_start: 0x2000,
virt_start: 0x2000,
page_count: 1,
att: MemoryAttribute::WRITE_BACK,
},
MemoryDescriptor {
ty: MemoryType::CONVENTIONAL,
padding0: 0,
phys_start: 0x1000,
virt_start: 0x1000,
page_count: 1,
Expand Down
12 changes: 12 additions & 0 deletions uefi/src/mem/memory_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ mod tests_mmap_artificial {

const BASE: MemoryDescriptor = MemoryDescriptor {
ty: TY,
padding0: 0,
phys_start: 0,
virt_start: 0,
page_count: 0,
Expand Down Expand Up @@ -169,6 +170,7 @@ mod tests_mmap_artificial {

const BASE: MemoryDescriptor = MemoryDescriptor {
ty: TY,
padding0: 0,
phys_start: 0,
virt_start: 0,
page_count: 0,
Expand Down Expand Up @@ -278,6 +280,7 @@ mod tests_mmap_real {
| MemoryAttribute::WRITE_COMBINE
| MemoryAttribute::WRITE_THROUGH
| MemoryAttribute::WRITE_BACK,
..Default::default()
},
MemoryDescriptor {
ty: MemoryType::CONVENTIONAL,
Expand All @@ -288,6 +291,7 @@ mod tests_mmap_real {
| MemoryAttribute::WRITE_COMBINE
| MemoryAttribute::WRITE_THROUGH
| MemoryAttribute::WRITE_BACK,
..Default::default()
},
MemoryDescriptor {
ty: MemoryType::BOOT_SERVICES_DATA,
Expand All @@ -298,6 +302,7 @@ mod tests_mmap_real {
| MemoryAttribute::WRITE_COMBINE
| MemoryAttribute::WRITE_THROUGH
| MemoryAttribute::WRITE_BACK,
..Default::default()
},
MemoryDescriptor {
ty: MemoryType::CONVENTIONAL,
Expand All @@ -308,6 +313,7 @@ mod tests_mmap_real {
| MemoryAttribute::WRITE_COMBINE
| MemoryAttribute::WRITE_THROUGH
| MemoryAttribute::WRITE_BACK,
..Default::default()
},
MemoryDescriptor {
ty: MemoryType::CONVENTIONAL,
Expand All @@ -318,6 +324,7 @@ mod tests_mmap_real {
| MemoryAttribute::WRITE_COMBINE
| MemoryAttribute::WRITE_THROUGH
| MemoryAttribute::WRITE_BACK,
..Default::default()
},
MemoryDescriptor {
ty: MemoryType::ACPI_NON_VOLATILE,
Expand All @@ -328,6 +335,7 @@ mod tests_mmap_real {
| MemoryAttribute::WRITE_COMBINE
| MemoryAttribute::WRITE_THROUGH
| MemoryAttribute::WRITE_BACK,
..Default::default()
},
MemoryDescriptor {
ty: MemoryType::CONVENTIONAL,
Expand All @@ -338,6 +346,7 @@ mod tests_mmap_real {
| MemoryAttribute::WRITE_COMBINE
| MemoryAttribute::WRITE_THROUGH
| MemoryAttribute::WRITE_BACK,
..Default::default()
},
MemoryDescriptor {
ty: MemoryType::ACPI_NON_VOLATILE,
Expand All @@ -348,6 +357,7 @@ mod tests_mmap_real {
| MemoryAttribute::WRITE_COMBINE
| MemoryAttribute::WRITE_THROUGH
| MemoryAttribute::WRITE_BACK,
..Default::default()
},
MemoryDescriptor {
ty: MemoryType::CONVENTIONAL,
Expand All @@ -358,6 +368,7 @@ mod tests_mmap_real {
| MemoryAttribute::WRITE_COMBINE
| MemoryAttribute::WRITE_THROUGH
| MemoryAttribute::WRITE_BACK,
..Default::default()
},
MemoryDescriptor {
ty: MemoryType::ACPI_NON_VOLATILE,
Expand All @@ -368,6 +379,7 @@ mod tests_mmap_real {
| MemoryAttribute::WRITE_COMBINE
| MemoryAttribute::WRITE_THROUGH
| MemoryAttribute::WRITE_BACK,
..Default::default()
},
];
assert_eq!(entries.as_slice(), &expected);
Expand Down
3 changes: 3 additions & 0 deletions uefi/tests/memory_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,23 @@ fn parse_boot_information_efi_mmap() {
virt_start: 0x3000,
page_count: 1,
att: MemoryAttribute::WRITE_BACK,
..Default::default()
},
MemoryDescriptor {
ty: MemoryType::CONVENTIONAL,
phys_start: 0x2000,
virt_start: 0x2000,
page_count: 1,
att: MemoryAttribute::WRITE_BACK,
..Default::default()
},
MemoryDescriptor {
ty: MemoryType::CONVENTIONAL,
phys_start: 0x1000,
virt_start: 0x1000,
page_count: 1,
att: MemoryAttribute::WRITE_BACK,
..Default::default()
},
];
let map_size = mmap_source.len() * desc_size;
Expand Down
8 changes: 7 additions & 1 deletion xtask/src/check_raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,13 @@ fn check_item(item: &Item, src: &Path) -> Result<(), Error> {
// Allow.
}
item => {
return Err(Error::new(ErrorKind::ForbiddenItemKind, src, item));
let allowed_exception = match item {
Item::ExternCrate(x) => x.ident == "std",
_ => false,
};
if !allowed_exception {
return Err(Error::new(ErrorKind::ForbiddenItemKind, src, item));
}
}
}

Expand Down