rust-osdev / x86_64

Library to program x86_64 hardware.
https://docs.rs/x86_64
Apache License 2.0
797 stars 132 forks source link

A `VirtualAddr` panic is triggered when the page table called `clean_up` #468

Closed GZTimeWalker closed 8 months ago

GZTimeWalker commented 8 months ago

I'm migrating to v0.15.0, found that a VirtualAddr panic is triggered when the page table called clean_up.

It caused by the new function introduced in https://github.com/rust-osdev/x86_64/pull/370.

trace!("Free page table: {:#x}", page_table.reg.addr.start_address());
mapper.clean_up(frame_deallocator);
[T] Clean up page table for sh#2
[T] Free segment: [0x111100000000 -> 0x111100001000) (2 pages)
[T] Free segment: [0x111100002000 -> 0x111100009000) (8 pages)
[T] Free segment: [0x11110000a000 -> 0x11110000b000) (2 pages)
[T] Free segment: [0x11110000c000 -> 0x11110000c000) (1 pages)
[T] Free page table: 0x163000
[E] pkg/kernel/src/utils/macros.rs @ 136

ERROR: panicked at .../x86_64-0.15.0/src/addr.rs @ 75:23

virtual address must be sign extended in bits 48 to 64

But since my kernel doesn't have backtrace, I am trying to find out why this is happening through step-by-step debugging.

GZTimeWalker commented 8 months ago

Some progress:

[#0] 0xffffff0000085a11 → x86_64::addr::VirtAddr::new(addr=<optimized out>, addr=<optimized out>)
[#1] 0xffffff0000085a11 → x86_64::addr::{impl#8}::add(rhs=<optimized out>, rhs=<optimized out>)
[#2] 0xffffff0000085a11 → x86_64::structures::paging::mapper::mapped_page_table::{impl#5}::clean_up_addr_range::clean_up<PhysOffset, ...>()
[#3] 0xffffff000006ee55 → x86_64::structures::paging::mapper::mapped_page_table::{impl#5}::clean_up_addr_range<PhysOffset, ..>(...)
Freax13 commented 8 months ago

Does this patch fix the issue?

diff --git a/src/structures/paging/mapper/mapped_page_table.rs b/src/structures/paging/mapper/mapped_page_table.rs
index 25e83e1..53fc869 100644
--- a/src/structures/paging/mapper/mapped_page_table.rs
+++ b/src/structures/paging/mapper/mapped_page_table.rs
@@ -646,7 +646,11 @@ impl<'a, P: PageTableFrameMapping> CleanUp for MappedPageTable<'a, P> {
                     .skip(usize::from(start))
                 {
                     if let Ok(page_table) = page_table_walker.next_table_mut(entry) {
-                        let start = table_addr + (offset_per_entry * (i as u64));
+                        let start = VirtAddr::forward_checked_impl(
+                            table_addr,
+                            (offset_per_entry as usize) * i,
+                        )
+                        .unwrap();
                         let end = start + (offset_per_entry - 1);
                         let start = Page::<Size4KiB>::containing_address(start);
                         let start = start.max(range.start);
GZTimeWalker commented 8 months ago

Does this patch fix the issue?

It will fix this issue in my case.