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

add workaround for recursive page tables with recursive index 511 #425

Merged
merged 4 commits into from
Jun 19, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/structures/paging/mapper/recursive_page_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@
/// - The page table must be active, i.e. the CR3 register must contain its physical address.
///
/// Otherwise `Err(())` is returned.
///
/// ## Safety
///
/// Creating a recursive page table with recursive index 511 is unsound
/// because calculating the end ptr of the structure causes an overflow.
Freax13 marked this conversation as resolved.
Show resolved Hide resolved
#[inline]
pub fn new(table: &'a mut PageTable) -> Result<Self, InvalidPageTable> {
let page = Page::containing_address(VirtAddr::new(table as *const _ as u64));
Expand Down Expand Up @@ -85,7 +90,7 @@

/// Returns a mutable reference to the wrapped level 4 `PageTable` instance.
pub fn level_4_table(&mut self) -> &mut PageTable {
&mut self.p4

Check warning on line 93 in src/structures/paging/mapper/recursive_page_table.rs

View workflow job for this annotation

GitHub Actions / Clippy

this expression creates a reference which is immediately dereferenced by the compiler
}

/// Internal helper function to create the page table of the next level if needed.
Expand Down Expand Up @@ -774,7 +779,7 @@
if p3_entry.flags().contains(PageTableFlags::HUGE_PAGE) {
let entry = &p3[addr.p3_index()];
let frame = PhysFrame::containing_address(entry.addr());
let offset = addr.as_u64() & 0o_777_777_7777;

Check warning on line 782 in src/structures/paging/mapper/recursive_page_table.rs

View workflow job for this annotation

GitHub Actions / Clippy

digits of hex, binary or octal literal not in groups of equal size
let flags = entry.flags();
return TranslateResult::Mapped {
frame: MappedFrame::Size1GiB(frame),
Expand All @@ -791,7 +796,7 @@
if p2_entry.flags().contains(PageTableFlags::HUGE_PAGE) {
let entry = &p2[addr.p2_index()];
let frame = PhysFrame::containing_address(entry.addr());
let offset = addr.as_u64() & 0o_777_7777;

Check warning on line 799 in src/structures/paging/mapper/recursive_page_table.rs

View workflow job for this annotation

GitHub Actions / Clippy

digits of hex, binary or octal literal not in groups of equal size
let flags = entry.flags();
return TranslateResult::Mapped {
frame: MappedFrame::Size2MiB(frame),
Expand Down
25 changes: 21 additions & 4 deletions src/structures/paging/page_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,27 +200,44 @@ impl PageTable {
/// Clears all entries.
#[inline]
pub fn zero(&mut self) {
for entry in self.entries.iter_mut() {
for entry in self.iter_mut() {
entry.set_unused();
}
}

/// Returns an iterator over the entries of the page table.
#[inline]
pub fn iter(&self) -> impl Iterator<Item = &PageTableEntry> {
self.entries.iter()
// Note that we intentionally don't just return `self.entries.iter()`:
// Some users may choose to create a reference to a page table at
// `0xffff_ffff_ffff_f000`. This causes problems because calculating
// the end pointer of the page tables causes an overflow. Therefore
// creating page tables at that address is unsound and must be avoided.
// Unfortunately creating such page tables is quite common when
// recursive page tables are used, so we try to avoid calculating the
// end pointer if possible. `core::slice::Iter` calculates the end
// pointer to determine when it should stop yielding elements. Because
// we want to avoid calculating the end pointer, we don't use
// `core::slice::Iter`, we implement our own iterator that doesn't
// calculate the end pointer. This doesn't make creating page tables at
// that address sound, but it avoids some easy to trigger
// miscompilations.
let ptr = self.entries.as_ptr();
(0..512).map(move |i| unsafe { &*ptr.add(i) })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried whether self.entries[i] works? This way we might be able to avoid the unsafe..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work for iter, but not iter_mut.

}

/// Returns an iterator that allows modifying the entries of the page table.
#[inline]
pub fn iter_mut(&mut self) -> impl Iterator<Item = &mut PageTableEntry> {
self.entries.iter_mut()
// See `Self::iter`.
let ptr = self.entries.as_mut_ptr();
(0..512).map(move |i| unsafe { &mut *ptr.add(i) })
}

/// Checks if the page table is empty (all entries are zero).
#[inline]
pub fn is_empty(&self) -> bool {
self.entries.iter().all(|entry| entry.is_unused())
self.iter().all(|entry| entry.is_unused())
}
}

Expand Down