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.

Sign up for GitHub

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

get rid of unnecessary uses of unsafe #460

Merged
merged 5 commits into from
Oct 25, 2024
Merged

get rid of unnecessary uses of unsafe #460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0cd83e8
acpi: remove unsafe
Freax13 Sep 19, 2024
7d7f82b
fw_meta: remove unsafe
Freax13 Sep 19, 2024
054f2af
string: simplify From<[u8; N]> impl
Freax13 Sep 19, 2024
48e2e3e
pld_repord: remove unsafe
Freax13 Sep 19, 2024
9ec1021
pagebox: simplify some impls
Freax13 Sep 19, 2024
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
85 changes: 34 additions & 51 deletions kernel/src/acpi/tables.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ use crate::fw_cfg::FwCfg;
use crate::string::FixedString;
use alloc::vec::Vec;
use core::mem;
use zerocopy::{FromBytes, FromZeros, Immutable, IntoBytes, KnownLayout};

/// ACPI Root System Description Pointer (RSDP)
/// used by ACPI programming interface
#[derive(Debug, Default)]
#[derive(Debug, Default, FromBytes, IntoBytes)]
#[repr(C, packed)]
struct RSDPDesc {
/// Signature must contain "RSD PTR"
Expand All @@ -40,7 +41,6 @@ impl RSDPDesc {
///
/// A [`Result`] containing the [`RSDPDesc`] if successful, or an [`SvsmError`] on failure.
fn from_fwcfg(fw_cfg: &FwCfg<'_>) -> Result<Self, SvsmError> {
let mut buf = mem::MaybeUninit::<Self>::uninit();
let path = option_env!("ACPI_RSDP_PATH").unwrap_or("etc/acpi/rsdp");
let file = fw_cfg.file_selector(path)?;

Expand All @@ -49,17 +49,13 @@ impl RSDPDesc {
}

fw_cfg.select(file.selector());
let ptr = buf.as_mut_ptr().cast::<u8>();
for i in 0..mem::size_of::<Self>() {
let byte: u8 = fw_cfg.read_le();
unsafe { ptr.add(i).write(byte) };
}

unsafe { Ok(buf.assume_init()) }
let mut this = Self::new_zeroed();
fw_cfg.read_bytes(this.as_mut_bytes());
Ok(this)
}
}

#[derive(Copy, Clone, Debug, Default)]
#[derive(Copy, Clone, Debug, Default, FromBytes, KnownLayout, Immutable)]
#[repr(C, packed)]
/// Raw header of an ACPI table. It corresponds to the beginning
/// portion of ACPI tables, before any specific table data
Expand Down Expand Up @@ -167,20 +163,17 @@ impl ACPITable {
///
/// A new [`ACPITable`] instance on success, or an [`SvsmError`] if parsing fails.
fn new(ptr: &[u8]) -> Result<Self, SvsmError> {
let raw_header = ptr
.get(..mem::size_of::<RawACPITableHeader>())
.ok_or(SvsmError::Acpi)?
.as_ptr()
.cast::<RawACPITableHeader>();
let size = unsafe { (*raw_header).len as usize };
let (raw_header, _) =
RawACPITableHeader::read_from_prefix(ptr).map_err(|_| SvsmError::Acpi)?;
let size = raw_header.len as usize;
let content = ptr.get(..size).ok_or(SvsmError::Acpi)?;

let mut buf = Vec::<u8>::new();
// Allow for a failable allocation before copying
buf.try_reserve(size).map_err(|_| SvsmError::Mem)?;
buf.extend_from_slice(content);

let header = unsafe { ACPITableHeader::new(*raw_header) };
let header = ACPITableHeader::new(raw_header);

Ok(Self { header, buf })
}
Expand Down Expand Up @@ -218,11 +211,14 @@ impl ACPITable {
///
/// # Returns
///
/// A pointer to the content of the ACPI table at specified offset as type `T`,
/// A reference to the content of the ACPI table at specified offset as type `T`,
/// or [`None`] if the offset is out of bounds.
fn content_ptr<T>(&self, offset: usize) -> Option<*const T> {
let end = offset.checked_add(mem::size_of::<T>())?;
Some(self.content()?.get(offset..end)?.as_ptr().cast::<T>())
fn content_ptr<T>(&self, offset: usize) -> Option<&T>
where
T: FromBytes + KnownLayout + Immutable,
{
let bytes = self.content()?.get(offset..)?;
T::ref_from_prefix(bytes).ok().map(|(value, _rest)| value)
}
}

Expand Down Expand Up @@ -289,14 +285,9 @@ impl ACPITableBuffer {
return Err(SvsmError::Mem);
}
buf.try_reserve(size).map_err(|_| SvsmError::Mem)?;
let ptr = buf.as_mut_ptr();

buf.resize(size, 0);
fw_cfg.select(file.selector());
for i in 0..size {
let byte: u8 = fw_cfg.read_le();
unsafe { ptr.add(i).write(byte) };
}
unsafe { buf.set_len(size) }
fw_cfg.read_bytes(&mut buf);

let mut acpibuf = Self {
buf,
Expand Down Expand Up @@ -328,15 +319,10 @@ impl ACPITableBuffer {
.map(|c| u32::from_le_bytes(c.try_into().unwrap()) as usize);

for offset in offsets {
let raw_header = offset
.checked_add(mem::size_of::<RawACPITableHeader>())
.and_then(|end| self.buf.get(offset..end))
.ok_or(SvsmError::Acpi)?
.as_ptr()
.cast::<RawACPITableHeader>();

let meta = unsafe { ACPITableMeta::new(&*raw_header, offset) };

let raw_header = self.buf.get(offset..).ok_or(SvsmError::Acpi)?;
let (raw_header, _) =
RawACPITableHeader::ref_from_prefix(raw_header).map_err(|_| SvsmError::Acpi)?;
let meta = ACPITableMeta::new(raw_header, offset);
self.tables.push(meta);
}

Expand Down Expand Up @@ -387,15 +373,15 @@ impl ACPITableBuffer {
const MADT_HEADER_SIZE: usize = 8;

/// Header of an entry within MADT
#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy, Debug, FromBytes, Immutable, KnownLayout)]
#[repr(C, packed)]
struct RawMADTEntryHeader {
entry_type: u8,
entry_len: u8,
}

/// Entry for a local APIC within MADT
#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy, Debug, FromBytes, Immutable, KnownLayout)]
#[repr(C, packed)]
struct RawMADTEntryLocalApic {
header: RawMADTEntryHeader,
Expand All @@ -405,7 +391,7 @@ struct RawMADTEntryLocalApic {
}

/// Entry for a local X2APIC within MADT
#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy, Debug, FromBytes, Immutable, KnownLayout)]
#[repr(C, packed)]
struct RawMADTEntryLocalX2Apic {
header: RawMADTEntryHeader,
Expand Down Expand Up @@ -488,38 +474,35 @@ pub fn load_acpi_cpu_info(fw_cfg: &FwCfg<'_>) -> Result<Vec<ACPICPUInfo>, SvsmEr
let entry_ptr = apic_table
.content_ptr::<RawMADTEntryHeader>(offset)
.ok_or(SvsmError::Acpi)?;
let (madt_type, entry_len) = unsafe { ((*entry_ptr).entry_type, (*entry_ptr).entry_len) };
let entry_len = usize::from(entry_len);
let entry_len = usize::from(entry_ptr.entry_len);

match madt_type {
match entry_ptr.entry_type {
0 if entry_len == mem::size_of::<RawMADTEntryLocalApic>() => {
let lapic_ptr = apic_table
.content_ptr::<RawMADTEntryLocalApic>(offset)
.ok_or(SvsmError::Acpi)?;
let (apic_id, flags) = unsafe { ((*lapic_ptr).apic_id as u32, (*lapic_ptr).flags) };
cpus.push(ACPICPUInfo {
apic_id,
enabled: (flags & 1) == 1,
apic_id: lapic_ptr.apic_id as u32,
enabled: (lapic_ptr.flags & 1) == 1,
});
}
9 if entry_len == mem::size_of::<RawMADTEntryLocalX2Apic>() => {
let x2apic_ptr = apic_table
.content_ptr::<RawMADTEntryLocalX2Apic>(offset)
.ok_or(SvsmError::Acpi)?;
let (apic_id, flags) = unsafe { ((*x2apic_ptr).apic_id, (*x2apic_ptr).flags) };
cpus.push(ACPICPUInfo {
apic_id,
enabled: (flags & 1) == 1,
apic_id: x2apic_ptr.apic_id,
enabled: (x2apic_ptr.flags & 1) == 1,
});
}
_ if entry_len == 0 => {
madt_type if entry_len == 0 => {
log::warn!(
"Found zero-length MADT entry with type {}, stopping",
madt_type
);
break;
}
_ => {
madt_type => {
log::info!("Ignoring MADT entry with type {}", madt_type);
}
}
Expand Down
6 changes: 6 additions & 0 deletions kernel/src/fw_cfg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ impl<'a> FwCfg<'a> {
self.driver.outw(FW_CFG_CTL, cfg);
}

pub fn read_bytes(&self, out: &mut [u8]) {
for byte in out.iter_mut() {
*byte = self.driver.inb(FW_CFG_DATA);
}
}

pub fn read_le<T>(&self) -> T
where
T: core::ops::Shl<usize, Output = T> + core::ops::BitOr<T, Output = T> + From<u8>,
Expand Down
75 changes: 15 additions & 60 deletions kernel/src/fw_meta.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ use crate::types::{PageSize, PAGE_SIZE};
use crate::utils::{zero_mem_region, MemoryRegion};
use alloc::vec::Vec;
use bootlib::kernel_launch::KernelLaunchInfo;
use zerocopy::{FromBytes, Immutable, KnownLayout};

use core::fmt;
use core::mem::{align_of, size_of, size_of_val};
use core::mem::{size_of, size_of_val};
use core::str::FromStr;

#[derive(Clone, Debug, Default)]
Expand Down Expand Up @@ -135,7 +136,7 @@ const OVMF_TABLE_FOOTER_GUID: &str = "96b582de-1fb2-45f7-baea-a366c55a082d";
const OVMF_SEV_META_DATA_GUID: &str = "dc886566-984a-4798-a75e-5585a7bf67cc";
const SVSM_INFO_GUID: &str = "a789a612-0597-4c4b-a49f-cbb1fe9d1ddd";

#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy, Debug, FromBytes, KnownLayout, Immutable)]
#[repr(C, packed)]
struct SevMetaDataHeader {
signature: [u8; 4],
Expand All @@ -144,7 +145,7 @@ struct SevMetaDataHeader {
num_desc: u32,
}

#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy, Debug, FromBytes, KnownLayout, Immutable)]
#[repr(C, packed)]
struct SevMetaDataDesc {
base: u32,
Expand All @@ -157,7 +158,7 @@ const SEV_META_DESC_TYPE_SECRETS: u32 = 2;
const SEV_META_DESC_TYPE_CPUID: u32 = 3;
const SEV_META_DESC_TYPE_CAA: u32 = 4;

#[derive(Debug, Clone, Copy)]
#[derive(Debug, Clone, Copy, FromBytes, KnownLayout, Immutable)]
#[repr(C, packed)]
struct RawMetaHeader {
len: u16,
Expand All @@ -171,7 +172,7 @@ impl RawMetaHeader {
}
}

#[derive(Debug, Clone, Copy)]
#[derive(Debug, Clone, Copy, FromBytes, KnownLayout, Immutable)]
#[repr(C, packed)]
struct RawMetaBuffer {
data: [u8; PAGE_SIZE - size_of::<RawMetaHeader>() - 32],
Expand All @@ -190,14 +191,7 @@ fn find_table<'a>(uuid: &Uuid, mem: &'a [u8]) -> Option<&'a [u8]> {

while idx != 0 {
let hdr_start = idx.checked_sub(size_of::<RawMetaHeader>())?;
let hdr_start_ptr = mem.get(hdr_start..idx)?.as_ptr().cast::<RawMetaHeader>();
if hdr_start_ptr.align_offset(align_of::<RawMetaHeader>()) != 0 {
log::error!("Misaligned firmware metadata table");
return None;
}

// Safety: we have checked the pointer is within bounds and aligned
let hdr = unsafe { hdr_start_ptr.read() };
let hdr = RawMetaHeader::ref_from_bytes(&mem[hdr_start..idx]).unwrap();

let data_len = hdr.data_len()?;
idx = hdr_start.checked_sub(data_len)?;
Expand All @@ -216,14 +210,7 @@ fn find_table<'a>(uuid: &Uuid, mem: &'a [u8]) -> Option<&'a [u8]> {
pub fn parse_fw_meta_data(mem: &[u8]) -> Result<SevFWMetaData, SvsmError> {
let mut meta_data = SevFWMetaData::new();

if mem.len() != size_of::<RawMetaBuffer>() {
return Err(SvsmError::Firmware);
}

// Safety: `RawMetaBuffer` has no invalid representations and is
// `repr(C, packed)`, which means there are no alignment requirements.
// We have also verified that the size of the slice matches.
let raw_meta = unsafe { &*mem.as_ptr().cast::<RawMetaBuffer>() };
let raw_meta = RawMetaBuffer::ref_from_bytes(mem).map_err(|_| SvsmError::Firmware)?;

// Check the UUID
let raw_uuid = raw_meta.header.uuid;
Expand Down Expand Up @@ -283,51 +270,19 @@ fn parse_sev_meta(
.ok_or(SvsmError::Firmware)?;
let sev_meta_end = sev_meta_start + size_of::<SevMetaDataHeader>();
// Bounds check the header and get a pointer to it
let sev_meta_hdr_ptr = raw_meta
let bytes = raw_meta
.data
.get(sev_meta_start..sev_meta_end)
.ok_or(SvsmError::Firmware)?
.as_ptr()
.cast::<SevMetaDataHeader>();

// Check that the header pointer is aligned. This also guarantees that
// descriptors down the line will be aligned. If the pointer was not
// aligned we would need to use ptr::read_unaligned(), so simply check
// beforehand and use ptr::read(), as there's no reason for the metadata
// to be misaligned.
if sev_meta_hdr_ptr.align_offset(align_of::<SevMetaDataHeader>()) != 0 {
log::error!("Misaligned SEV metadata header");
return Err(SvsmError::Firmware);
}
// Safety: we have checked the pointer is within bounds and aligned.
let sev_meta_hdr = unsafe { sev_meta_hdr_ptr.read() };
.ok_or(SvsmError::Firmware)?;
let sev_meta_hdr = SevMetaDataHeader::ref_from_bytes(bytes).map_err(|_| SvsmError::Firmware)?;

// Now find the descriptors
let bytes = &raw_meta.data[sev_meta_end..];
let num_desc = sev_meta_hdr.num_desc as usize;
let sev_descs_start = sev_meta_end;
let sev_descs_len = num_desc
.checked_mul(size_of::<SevMetaDataDesc>())
.ok_or(SvsmError::Firmware)?;
let sev_descs_end = sev_descs_start
.checked_add(sev_descs_len)
.ok_or(SvsmError::Firmware)?;
let (descs, _) = <[SevMetaDataDesc]>::ref_from_prefix_with_elems(bytes, num_desc)
.map_err(|_| SvsmError::Firmware)?;

// We have a variable number of descriptors following the header.
// Unfortunately flexible array members in Rust are not fully supported,
// so we cannot avoid using raw pointers.
let sev_descs_ptr = raw_meta
.data
.get(sev_descs_start..sev_descs_end)
.ok_or(SvsmError::Firmware)?
.as_ptr()
.cast::<SevMetaDataDesc>();

for i in 0..num_desc {
// Safety: We have checked that the descriptors are within bounds of
// the metadata memory. Since the descriptors follow the header, and
// the header is properly aligned, the descriptors must be so as
// well.
let desc = unsafe { sev_descs_ptr.add(i).read() };
for desc in descs {
let t = desc.t;
let base = PhysAddr::from(desc.base as usize);
let len = desc.len as usize;
Expand Down
Loading
Loading