From 365fa05a8256d1ba44cff5d05c44dda5021571e9 Mon Sep 17 00:00:00 2001 From: Tom Date: Mon, 14 Sep 2020 13:10:49 -0600 Subject: [PATCH] Kernel: Handle safe_memcpy/safe_memset/safe_strnlen faults in irq handlers Fix gracefully failing these calls if used within IRQ handlers. If we're handling IRQs, we need to handle these failures first, because we can't really resolve page faults in a meaningful way. But if we know that it was one of these functions that failed, then we can gracefully handle the situation. This solves a crash where the Scheduler attempts to produce backtraces in the timer irq, some of which cause faults. Fixes #3492 --- Kernel/Arch/i386/CPU.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/Kernel/Arch/i386/CPU.cpp b/Kernel/Arch/i386/CPU.cpp index d1a9ccb136..ca5425cffd 100644 --- a/Kernel/Arch/i386/CPU.cpp +++ b/Kernel/Arch/i386/CPU.cpp @@ -401,9 +401,23 @@ void page_fault_handler(TrapFrame* trap) dump(regs); #endif - bool faulted_in_userspace = (regs.cs & 3) == 3; + bool faulted_in_kernel = !(regs.cs & 3); + + if (faulted_in_kernel && Processor::current().in_irq()) { + // If we're faulting in an IRQ handler, first check if we failed + // due to safe_memcpy, safe_strnlen, or safe_memset. If we did, + // gracefully continue immediately. Because we're in an IRQ handler + // we can't really try to resolve the page fault in a meaningful + // way, so we need to do this before calling into + // MemoryManager::handle_page_fault, which would just bail and + // request a crash + if (handle_safe_access_fault(regs, fault_address)) + return; + } + + auto current_thread = Thread::current(); - if (faulted_in_userspace && !MM.validate_user_stack(current_thread->process(), VirtualAddress(regs.userspace_esp))) { + if (!faulted_in_kernel && !MM.validate_user_stack(current_thread->process(), VirtualAddress(regs.userspace_esp))) { dbg() << "Invalid stack pointer: " << VirtualAddress(regs.userspace_esp); handle_crash(regs, "Bad stack on page fault", SIGSTKFLT); ASSERT_NOT_REACHED(); @@ -412,7 +426,7 @@ void page_fault_handler(TrapFrame* trap) auto response = MM.handle_page_fault(PageFault(regs.exception_code, VirtualAddress(fault_address))); if (response == PageFaultResponse::ShouldCrash || response == PageFaultResponse::OutOfMemory) { - if (!(regs.cs & 3) && handle_safe_access_fault(regs, fault_address)) { + if (faulted_in_kernel && handle_safe_access_fault(regs, fault_address)) { // If this would be a ring0 (kernel) fault and the fault was triggered by // safe_memcpy, safe_strnlen, or safe_memset then we resume execution at // the appropriate _fault label rather than crashing