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

Make map_to and update_flags unsafe #135

Merged
merged 5 commits into from
Apr 11, 2020
Merged
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
8 changes: 6 additions & 2 deletions src/structures/paging/frame_alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,21 @@ use core::ops::{Deref, DerefMut};
/// the `allocate_frame` method returns only unique unused frames.
pub unsafe trait FrameAllocator<S: PageSize> {
/// Allocate a frame of the appropriate size and return it if possible.
fn allocate_frame(&mut self) -> Option<UnusedPhysFrame<S>>;
fn allocate_frame(&mut self) -> Option<PhysFrame<S>>;
}

/// A trait for types that can deallocate a frame of memory.
pub trait FrameDeallocator<S: PageSize> {
/// Deallocate the given frame of memory.
fn deallocate_frame(&mut self, frame: UnusedPhysFrame<S>);
fn deallocate_frame(&mut self, frame: PhysFrame<S>);
}

/// Represents a physical frame that is not used for any mapping.
#[deprecated(note = "This wrapper type is no longer used. Use `PhysFrame` instead.")]
#[derive(Debug)]
pub struct UnusedPhysFrame<S: PageSize = Size4KiB>(PhysFrame<S>);

#[allow(deprecated)]
impl<S: PageSize> UnusedPhysFrame<S> {
/// Creates a new UnusedPhysFrame from the given frame.
///
Expand All @@ -39,6 +41,7 @@ impl<S: PageSize> UnusedPhysFrame<S> {
}
}

#[allow(deprecated)]
impl<S: PageSize> Deref for UnusedPhysFrame<S> {
type Target = PhysFrame<S>;

Expand All @@ -47,6 +50,7 @@ impl<S: PageSize> Deref for UnusedPhysFrame<S> {
}
}

#[allow(deprecated)]
impl<S: PageSize> DerefMut for UnusedPhysFrame<S> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
Expand Down
31 changes: 14 additions & 17 deletions src/structures/paging/mapper/mapped_page_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl<'a, P: PhysToVirt> MappedPageTable<'a, P> {
fn map_to_1gib<A>(
&mut self,
page: Page<Size1GiB>,
frame: UnusedPhysFrame<Size1GiB>,
frame: PhysFrame<Size1GiB>,
flags: PageTableFlags,
allocator: &mut A,
) -> Result<MapperFlush<Size1GiB>, MapToError<Size1GiB>>
Expand All @@ -66,7 +66,7 @@ impl<'a, P: PhysToVirt> MappedPageTable<'a, P> {
fn map_to_2mib<A>(
&mut self,
page: Page<Size2MiB>,
frame: UnusedPhysFrame<Size2MiB>,
frame: PhysFrame<Size2MiB>,
flags: PageTableFlags,
allocator: &mut A,
) -> Result<MapperFlush<Size2MiB>, MapToError<Size2MiB>>
Expand Down Expand Up @@ -94,7 +94,7 @@ impl<'a, P: PhysToVirt> MappedPageTable<'a, P> {
fn map_to_4kib<A>(
&mut self,
page: Page<Size4KiB>,
frame: UnusedPhysFrame<Size4KiB>,
frame: PhysFrame<Size4KiB>,
flags: PageTableFlags,
allocator: &mut A,
) -> Result<MapperFlush<Size4KiB>, MapToError<Size4KiB>>
Expand All @@ -115,17 +115,17 @@ impl<'a, P: PhysToVirt> MappedPageTable<'a, P> {
if !p1[page.p1_index()].is_unused() {
return Err(MapToError::PageAlreadyMapped(frame));
}
p1[page.p1_index()].set_frame(frame.frame(), flags);
p1[page.p1_index()].set_frame(frame, flags);

Ok(MapperFlush::new(page))
}
}

impl<'a, P: PhysToVirt> Mapper<Size1GiB> for MappedPageTable<'a, P> {
fn map_to<A>(
unsafe fn map_to<A>(
&mut self,
page: Page<Size1GiB>,
frame: UnusedPhysFrame<Size1GiB>,
frame: PhysFrame<Size1GiB>,
flags: PageTableFlags,
allocator: &mut A,
) -> Result<MapperFlush<Size1GiB>, MapToError<Size1GiB>>
Expand Down Expand Up @@ -161,7 +161,7 @@ impl<'a, P: PhysToVirt> Mapper<Size1GiB> for MappedPageTable<'a, P> {
Ok((frame, MapperFlush::new(page)))
}

fn update_flags(
unsafe fn update_flags(
&mut self,
page: Page<Size1GiB>,
flags: PageTableFlags,
Expand Down Expand Up @@ -195,10 +195,10 @@ impl<'a, P: PhysToVirt> Mapper<Size1GiB> for MappedPageTable<'a, P> {
}

impl<'a, P: PhysToVirt> Mapper<Size2MiB> for MappedPageTable<'a, P> {
fn map_to<A>(
unsafe fn map_to<A>(
&mut self,
page: Page<Size2MiB>,
frame: UnusedPhysFrame<Size2MiB>,
frame: PhysFrame<Size2MiB>,
flags: PageTableFlags,
allocator: &mut A,
) -> Result<MapperFlush<Size2MiB>, MapToError<Size2MiB>>
Expand Down Expand Up @@ -237,7 +237,7 @@ impl<'a, P: PhysToVirt> Mapper<Size2MiB> for MappedPageTable<'a, P> {
Ok((frame, MapperFlush::new(page)))
}

fn update_flags(
unsafe fn update_flags(
&mut self,
page: Page<Size2MiB>,
flags: PageTableFlags,
Expand Down Expand Up @@ -276,10 +276,10 @@ impl<'a, P: PhysToVirt> Mapper<Size2MiB> for MappedPageTable<'a, P> {
}

impl<'a, P: PhysToVirt> Mapper<Size4KiB> for MappedPageTable<'a, P> {
fn map_to<A>(
unsafe fn map_to<A>(
&mut self,
page: Page<Size4KiB>,
frame: UnusedPhysFrame<Size4KiB>,
frame: PhysFrame<Size4KiB>,
flags: PageTableFlags,
allocator: &mut A,
) -> Result<MapperFlush<Size4KiB>, MapToError<Size4KiB>>
Expand Down Expand Up @@ -315,7 +315,7 @@ impl<'a, P: PhysToVirt> Mapper<Size4KiB> for MappedPageTable<'a, P> {
Ok((frame, MapperFlush::new(page)))
}

fn update_flags(
unsafe fn update_flags(
&mut self,
page: Page<Size4KiB>,
flags: PageTableFlags,
Expand Down Expand Up @@ -465,10 +465,7 @@ impl<P: PhysToVirt> PageTableWalker<P> {

if entry.is_unused() {
if let Some(frame) = allocator.allocate_frame() {
entry.set_frame(
frame.frame(),
PageTableFlags::PRESENT | PageTableFlags::WRITABLE,
);
entry.set_frame(frame, PageTableFlags::PRESENT | PageTableFlags::WRITABLE);
created = true;
} else {
return Err(PageTableCreateError::FrameAllocationFailed);
Expand Down
55 changes: 46 additions & 9 deletions src/structures/paging/mapper/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ pub use self::mapped_page_table::{MappedPageTable, PhysToVirt};
pub use self::{offset_page_table::OffsetPageTable, recursive_page_table::RecursivePageTable};

use crate::structures::paging::{
frame_alloc::{FrameAllocator, UnusedPhysFrame},
page_table::PageTableFlags,
Page, PageSize, PhysFrame, Size1GiB, Size2MiB, Size4KiB,
frame_alloc::FrameAllocator, page_table::PageTableFlags, Page, PageSize, PhysFrame, Size1GiB,
Size2MiB, Size4KiB,
};
use crate::{PhysAddr, VirtAddr};

Expand Down Expand Up @@ -83,10 +82,39 @@ pub trait Mapper<S: PageSize> {
///
/// This function might need additional physical frames to create new page tables. These
/// frames are allocated from the `allocator` argument. At most three frames are required.
fn map_to<A>(
///
/// ## Safety
///
/// Creating page table mappings is a fundamentally unsafe operation because
/// there are various ways to break memory safety through it. For example,
/// re-mapping an in-use page to a different frame changes and invalidates
/// all values stored in that page, resulting in undefined behavior on the
/// next use.
///
/// The caller must ensure that no undefined behavior or memory safety
/// violations can occur through the new mapping. Among other things, the
/// caller must prevent the following:
///
/// - Aliasing of `&mut` references, i.e. two `&mut` references that point to
/// the same physical address. This is undefined behavior in Rust.
/// - This can be ensured by mapping each page to an individual physical
/// frame that is not mapped anywhere else.
/// - Creating uninitalized or invalid values: Rust requires that all values
/// have a correct memory layout. For example, a `bool` must be either a 0
/// or a 1 in memory, but not a 3 or 4. An exception is the `MaybeUninit`
/// wrapper type, which abstracts over possibly uninitialized memory.
/// - This is only a problem when re-mapping pages to different physical
/// frames. Mapping a page that is not in use yet is fine.
///
/// Special care must be taken when sharing pages with other address spaces,
/// e.g. by setting the `GLOBAL` flag. For example, a global mapping must be
/// the same in all address spaces, otherwise undefined behavior can occur
/// because of TLB races. It's worth noting that all the above requirements
/// also apply to shared mappings, including the aliasing requirements.
unsafe fn map_to<A>(
&mut self,
page: Page<S>,
frame: UnusedPhysFrame<S>,
frame: PhysFrame<S>,
flags: PageTableFlags,
frame_allocator: &mut A,
) -> Result<MapperFlush<S>, MapToError<S>>
Expand All @@ -100,7 +128,15 @@ pub trait Mapper<S: PageSize> {
fn unmap(&mut self, page: Page<S>) -> Result<(PhysFrame<S>, MapperFlush<S>), UnmapError>;

/// Updates the flags of an existing mapping.
fn update_flags(
///
/// ## Safety
///
/// This method is unsafe because changing the flags of a mapping
/// might result in undefined behavior. For example, setting the
/// `GLOBAL` and `MUTABLE` flags for a page might result in the corruption
/// of values stored in that page from processes running in other address
/// spaces.
unsafe fn update_flags(
&mut self,
page: Page<S>,
flags: PageTableFlags,
Expand All @@ -116,11 +152,12 @@ pub trait Mapper<S: PageSize> {
///
/// ## Safety
///
/// TODO: Should this function be safe?
/// This is a convencience function that invokes [`map_to`] internally, so
/// all safety requirements of it also apply for this function.
#[inline]
unsafe fn identity_map<A>(
&mut self,
frame: UnusedPhysFrame<S>,
frame: PhysFrame<S>,
flags: PageTableFlags,
frame_allocator: &mut A,
) -> Result<MapperFlush<S>, MapToError<S>>
Expand Down Expand Up @@ -172,7 +209,7 @@ pub enum MapToError<S: PageSize> {
/// given page is part of an already mapped huge page.
ParentEntryHugePage,
/// The given page is already mapped to a physical frame.
PageAlreadyMapped(UnusedPhysFrame<S>),
PageAlreadyMapped(PhysFrame<S>),
}

/// An error indicating that an `unmap` call failed.
Expand Down
18 changes: 9 additions & 9 deletions src/structures/paging/mapper/offset_page_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ impl PhysToVirt for PhysOffset {
// delegate all trait implementations to inner

impl<'a> Mapper<Size1GiB> for OffsetPageTable<'a> {
fn map_to<A>(
unsafe fn map_to<A>(
&mut self,
page: Page<Size1GiB>,
frame: UnusedPhysFrame<Size1GiB>,
frame: PhysFrame<Size1GiB>,
flags: PageTableFlags,
allocator: &mut A,
) -> Result<MapperFlush<Size1GiB>, MapToError<Size1GiB>>
Expand All @@ -75,7 +75,7 @@ impl<'a> Mapper<Size1GiB> for OffsetPageTable<'a> {
}

#[inline]
fn update_flags(
unsafe fn update_flags(
&mut self,
page: Page<Size1GiB>,
flags: PageTableFlags,
Expand All @@ -91,10 +91,10 @@ impl<'a> Mapper<Size1GiB> for OffsetPageTable<'a> {

impl<'a> Mapper<Size2MiB> for OffsetPageTable<'a> {
#[inline]
fn map_to<A>(
unsafe fn map_to<A>(
&mut self,
page: Page<Size2MiB>,
frame: UnusedPhysFrame<Size2MiB>,
frame: PhysFrame<Size2MiB>,
flags: PageTableFlags,
allocator: &mut A,
) -> Result<MapperFlush<Size2MiB>, MapToError<Size2MiB>>
Expand All @@ -113,7 +113,7 @@ impl<'a> Mapper<Size2MiB> for OffsetPageTable<'a> {
}

#[inline]
fn update_flags(
unsafe fn update_flags(
&mut self,
page: Page<Size2MiB>,
flags: PageTableFlags,
Expand All @@ -129,10 +129,10 @@ impl<'a> Mapper<Size2MiB> for OffsetPageTable<'a> {

impl<'a> Mapper<Size4KiB> for OffsetPageTable<'a> {
#[inline]
fn map_to<A>(
unsafe fn map_to<A>(
&mut self,
page: Page<Size4KiB>,
frame: UnusedPhysFrame<Size4KiB>,
frame: PhysFrame<Size4KiB>,
flags: PageTableFlags,
allocator: &mut A,
) -> Result<MapperFlush<Size4KiB>, MapToError<Size4KiB>>
Expand All @@ -151,7 +151,7 @@ impl<'a> Mapper<Size4KiB> for OffsetPageTable<'a> {
}

#[inline]
fn update_flags(
unsafe fn update_flags(
&mut self,
page: Page<Size4KiB>,
flags: PageTableFlags,
Expand Down
Loading