From 1047728f3542e55048a19fe4ecf6d1c40dbfae3f Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Fri, 6 Aug 2021 15:41:07 +0200 Subject: [PATCH] Only set process regs for faults from ring 3. This fixes a deadlock that might occur if a page fault is triggered while a lock to the current context is held. --- Cargo.lock | 4 +++- src/arch/x86_64/interrupt/handler.rs | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0d6c719..706ed5e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,5 +1,7 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. +version = 3 + [[package]] name = "autocfg" version = "1.0.1" @@ -165,7 +167,7 @@ dependencies = [ [[package]] name = "redox_syscall" -version = "0.2.9" +version = "0.2.10" dependencies = [ "bitflags", ] diff --git a/src/arch/x86_64/interrupt/handler.rs b/src/arch/x86_64/interrupt/handler.rs index 877792d..d3921f0 100644 --- a/src/arch/x86_64/interrupt/handler.rs +++ b/src/arch/x86_64/interrupt/handler.rs @@ -331,6 +331,9 @@ macro_rules! interrupt_stack { paste::item! { #[no_mangle] unsafe extern "C" fn [<__interrupt_ $name>]($stack: &mut $crate::arch::x86_64::interrupt::InterruptStack) { + // Deadlock safety: interrupts are not normally enabled in the kernel, except in + // kmain. However, no locks for context list nor even individual context locks, are + // ever meant to be acquired there. let _guard = $crate::ptrace::set_process_regs($stack); // TODO: Force the declarations to specify unsafe? @@ -410,7 +413,18 @@ macro_rules! interrupt_error { paste::item! { #[no_mangle] unsafe extern "C" fn [<__interrupt_ $name>]($stack: &mut $crate::arch::x86_64::interrupt::handler::InterruptErrorStack) { - let _guard = $crate::ptrace::set_process_regs(&mut $stack.inner); + let _guard; + + // Only set_ptrace_process_regs if this error occured from userspace. If this fault + // originated from kernel mode, we have no idea what it might have locked (and + // kernel mode faults are never meant to occur unless something is wrong, and will + // not context switch anyway, rendering that statement useless in such a case + // anyway). + // + // Check the privilege level of CS against ring 3. + if $stack.inner.iret.cs & 0b11 == 0b11 { + _guard = $crate::ptrace::set_process_regs(&mut $stack.inner); + } #[allow(unused_unsafe)] unsafe {