Merge pull request #425 from Freax13/fix/overflow

add workaround for recursive page tables with recursive index 511

Author: Freax13

src/structures/paging/mapper/recursive_page_table.rs

@@ -47,6 +47,14 @@ impl<'a> RecursivePageTable<'a> {
     /// - The page table must be active, i.e. the CR3 register must contain its physical address.
     ///
     /// Otherwise `Err(())` is returned.
+    ///
+    /// ## Safety
+    ///
+    /// Note that creating a `PageTable` with recursive index 511 is unsound
+    /// because allocating the last byte of the address space can lead to pointer
+    /// overflows and undefined behavior. For more details, see the discussions
+    /// [on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/end-of-address-space)
+    /// and [in the `unsafe-code-guidelines ` repo]https://github.com/rust-lang/unsafe-code-guidelines/issues/420).
     #[inline]
     pub fn new(table: &'a mut PageTable) -> Result<Self, InvalidPageTable> {
         let page = Page::containing_address(VirtAddr::new(table as *const _ as u64));

src/structures/paging/page_table.rs

@@ -209,27 +209,42 @@ 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()
+        (0..512).map(move |i| &self.entries[i])
     }
 
     /// 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()
+        // 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_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())
     }
 }