bugfix for race demand-page access.

Author: gjz010

crate/memory/src/memory_set/handler/delay.rs

@@ -49,10 +49,20 @@ impl<T: FrameAllocator> MemoryHandler for Delay<T> {
         }
     }
 
-    fn handle_page_fault(&self, pt: &mut dyn PageTable, addr: VirtAddr) -> bool {
+    fn handle_page_fault_ext(
+        &self,
+        pt: &mut dyn PageTable,
+        addr: VirtAddr,
+        access: super::AccessType,
+    ) -> bool {
         let entry = pt.get_entry(addr).expect("failed to get entry");
         if entry.present() {
-            // not a delay case
+            // permission check.
+            if access.check_access(entry) {
+                return true;
+            }
+            // permisison check failed.
+            error!("Permission check failed at 0x{:x}.", addr);
             return false;
         }
         let frame = self.allocator.alloc().expect("failed to alloc frame");

crate/memory/src/memory_set/handler/file.rs

@@ -58,10 +58,24 @@ impl<F: Read, T: FrameAllocator> MemoryHandler for File<F, T> {
         }
     }
 
-    fn handle_page_fault(&self, pt: &mut dyn PageTable, addr: usize) -> bool {
+    fn handle_page_fault_ext(
+        &self,
+        pt: &mut dyn PageTable,
+        addr: usize,
+        access: super::AccessType,
+    ) -> bool {
         let addr = addr & !(PAGE_SIZE - 1);
         let entry = pt.get_entry(addr).expect("failed to get entry");
         if entry.present() {
+            // permission check.
+            if access.check_access(entry) {
+                return true;
+            }
+            // permisison check failed.
+            error!(
+                "Permission check failed at 0x{:x}, access = {:?}.",
+                addr, access
+            );
             return false;
         }
         let execute = entry.execute();

crate/memory/src/memory_set/handler/mod.rs

@@ -1,5 +1,45 @@
 use super::*;
-
+#[derive(Copy, Clone, Debug)]
+pub struct AccessType {
+    pub write: bool,
+    pub execute: bool,
+    pub user: bool,
+}
+impl AccessType {
+    pub fn unknown() -> Self {
+        AccessType {
+            write: true,
+            execute: true,
+            user: true,
+        }
+    }
+    pub fn read(user: bool) -> Self {
+        AccessType {
+            write: false,
+            execute: false,
+            user,
+        }
+    }
+    pub fn write(user: bool) -> Self {
+        AccessType {
+            write: true,
+            execute: false,
+            user,
+        }
+    }
+    pub fn execute(user: bool) -> Self {
+        AccessType {
+            write: false,
+            execute: true,
+            user,
+        }
+    }
+    pub fn check_access(self, entry: &dyn paging::Entry) -> bool {
+        ((!self.write) || entry.writable())
+            && ((!self.execute) || entry.execute())
+            && ((!self.user) || entry.user())
+    }
+}
 // here may be a interesting part for lab
 pub trait MemoryHandler: Debug + Send + Sync + 'static {
     fn box_clone(&self) -> Box<dyn MemoryHandler>;
@@ -22,7 +62,20 @@ pub trait MemoryHandler: Debug + Send + Sync + 'static {
 
     /// Handle page fault on `addr`
     /// Return true if success, false if error
-    fn handle_page_fault(&self, pt: &mut dyn PageTable, addr: VirtAddr) -> bool;
+    fn handle_page_fault(&self, pt: &mut dyn PageTable, addr: VirtAddr) -> bool {
+        self.handle_page_fault_ext(pt, addr, AccessType::unknown())
+    }
+
+    /// Handle page fault on `addr` and access type `access`
+    /// Return true if success (or should-retry), false if error
+    fn handle_page_fault_ext(
+        &self,
+        pt: &mut dyn PageTable,
+        addr: VirtAddr,
+        _access: AccessType,
+    ) -> bool {
+        self.handle_page_fault(pt, addr)
+    }
 }
 
 impl Clone for Box<dyn MemoryHandler> {

crate/memory/src/memory_set/handler/shared.rs

@@ -35,7 +35,7 @@ impl<T: FrameAllocator> SharedGuard<T> {
         self.target.insert(virt_addr, phys_addr);
         Some(phys_addr)
     }
-    
+
     pub fn dealloc(&mut self, virt_addr: usize) {
         let phys_addr = self.target.get(&virt_addr).unwrap().clone();
         self.allocator.dealloc(phys_addr);

crate/memory/src/memory_set/mod.rs

@@ -376,6 +376,15 @@ impl<T: PageTableExt> MemorySet<T> {
         &mut self.page_table
     }
 
+    pub fn handle_page_fault_ext(&mut self, addr: VirtAddr, access: handler::AccessType) -> bool {
+        let area = self.areas.iter().find(|area| area.contains(addr));
+        match area {
+            Some(area) => area
+                .handler
+                .handle_page_fault_ext(&mut self.page_table, addr, access),
+            None => false,
+        }
+    }
     pub fn handle_page_fault(&mut self, addr: VirtAddr) -> bool {
         let area = self.areas.iter().find(|area| area.contains(addr));
         match area {

kernel/src/arch/riscv/interrupt/consts.rs

@@ -14,6 +14,15 @@ pub fn is_page_fault(trap: usize) -> bool {
     trap == InstructionPageFault || trap == LoadPageFault || trap == StorePageFault
 }
 
+pub fn is_execute_page_fault(trap: usize) -> bool {
+    trap == InstructionPageFault
+}
+pub fn is_read_page_fault(trap: usize) -> bool {
+    trap == LoadPageFault
+}
+pub fn is_write_page_fault(trap: usize) -> bool {
+    trap == StorePageFault
+}
 pub fn is_syscall(trap: usize) -> bool {
     trap == Syscall
 }

kernel/src/arch/riscv/interrupt/mod.rs

@@ -39,20 +39,27 @@ pub extern "C" fn trap_handler(tf: &mut TrapFrame) {
     trap_handler_no_frame(&mut tf.sepc);
 }
 
+use crate::memory::AccessType;
 #[inline]
 pub fn trap_handler_no_frame(sepc: &mut usize) {
     use self::scause::{Exception as E, Interrupt as I, Trap};
     let scause = scause::read();
     let stval = stval::read();
+    let is_user = false;
     trace!("Interrupt @ CPU{}: {:?} ", super::cpu::id(), scause.cause());
     match scause.cause() {
         Trap::Interrupt(I::SupervisorExternal) => external(),
         Trap::Interrupt(I::SupervisorSoft) => ipi(),
         Trap::Interrupt(I::SupervisorTimer) => timer(),
-        Trap::Exception(E::LoadPageFault) => page_fault(stval, sepc),
-        Trap::Exception(E::StorePageFault) => page_fault(stval, sepc),
-        Trap::Exception(E::InstructionPageFault) => page_fault(stval, sepc),
-        _ => panic!("unhandled trap {:?}", scause.cause()),
+        Trap::Exception(E::LoadPageFault) => page_fault(stval, sepc, AccessType::read(is_user)),
+        Trap::Exception(E::StorePageFault) => page_fault(stval, sepc, AccessType::write(is_user)),
+        Trap::Exception(E::InstructionPageFault) => {
+            page_fault(stval, sepc, AccessType::execute(is_user))
+        }
+        _ => {
+            let bits = scause.bits();
+            panic!("unhandled trap {:?} ({})", scause.cause(), bits);
+        }
     }
     trace!("Interrupt end");
 }
@@ -62,7 +69,6 @@ fn external() {
     unsafe {
         super::board::handle_external_interrupt();
     }
-
     IRQ_MANAGER
         .read()
         .try_handle_interrupt(Some(SupervisorExternal));
@@ -78,11 +84,11 @@ pub fn timer() {
     crate::trap::timer();
 }
 
-fn page_fault(stval: usize, sepc: &mut usize) {
+fn page_fault(stval: usize, sepc: &mut usize, access: AccessType) {
     let addr = stval;
     info!("\nEXCEPTION: Page Fault @ {:#x}", addr);
 
-    if crate::memory::handle_page_fault(addr) {
+    if crate::memory::handle_page_fault_ext(addr, access) {
         return;
     }
     extern "C" {
@@ -94,6 +100,7 @@ fn page_fault(stval: usize, sepc: &mut usize) {
         *sepc = crate::memory::read_user_fixup as usize;
         return;
     }
+    error!("unhandled page fault {:#x} from {:#x}", addr, sepc);
     panic!("unhandled page fault");
 }
 
@@ -121,8 +128,8 @@ pub fn wait_for_interrupt() {
     }
 }
 
-pub fn handle_user_page_fault(thread: &Arc<Thread>, addr: usize) -> bool {
-    thread.vm.lock().handle_page_fault(addr)
+pub fn handle_user_page_fault_ext(thread: &Arc<Thread>, addr: usize, access: AccessType) -> bool {
+    thread.vm.lock().handle_page_fault_ext(addr, access)
 }
 
 pub fn handle_reserved_inst(tf: &mut UserContext) -> bool {

kernel/src/process/thread.rs

@@ -5,7 +5,7 @@ use super::{
 use crate::arch::interrupt::consts::{
     is_intr, is_page_fault, is_reserved_inst, is_syscall, is_timer_intr,
 };
-use crate::arch::interrupt::{get_trap_num, handle_reserved_inst, handle_user_page_fault};
+use crate::arch::interrupt::{get_trap_num, handle_reserved_inst};
 use crate::arch::{
     cpu,
     fp::FpState,
@@ -505,10 +505,8 @@ pub fn spawn(thread: Arc<Thread>) {
             thread_context.fp.restore();
             cx.run();
             thread_context.fp.save();
-
             let trap_num = get_trap_num(&cx);
             trace!("back from user: {:#x?} trap_num {:#x}", cx, trap_num);
-
             let mut exit = false;
             let mut do_yield = false;
             match trap_num {
@@ -517,10 +515,36 @@ pub fn spawn(thread: Arc<Thread>) {
                     // page fault
                     let addr = get_page_fault_addr();
                     info!("page fault from user @ {:#x}", addr);
-
-                    if !handle_user_page_fault(&thread, addr) {
-                        // TODO: SIGSEGV
-                        panic!("page fault handle failed");
+                    #[cfg(any(target_arch = "riscv32", target_arch = "riscv64"))]
+                    {
+                        use crate::arch::interrupt::consts::{
+                            is_execute_page_fault, is_read_page_fault, is_write_page_fault,
+                        };
+                        use crate::arch::interrupt::handle_user_page_fault_ext;
+                        let access_type = match trap_num {
+                            _ if is_execute_page_fault(trap_num) => {
+                                crate::memory::AccessType::execute(true)
+                            }
+                            _ if is_read_page_fault(trap_num) => {
+                                crate::memory::AccessType::read(true)
+                            }
+                            _ if is_write_page_fault(trap_num) => {
+                                crate::memory::AccessType::write(true)
+                            }
+                            _ => unreachable!(),
+                        };
+                        if !handle_user_page_fault_ext(&thread, addr, access_type) {
+                            // TODO: SIGSEGV
+                            panic!("page fault handle failed");
+                        }
+                    }
+                    #[cfg(not(any(target_arch = "riscv32", target_arch = "riscv64")))]
+                    {
+                        use crate::arch::interrupt::handle_user_page_fault;
+                        if !handle_user_page_fault(&thread, addr) {
+                            // TODO: SIGSEGV
+                            panic!("page fault handle failed");
+                        }
                     }
                 }
                 _ if is_syscall(trap_num) => exit = handle_syscall(&thread, cx).await,