Messages in this thread Patch in this message | | | Date | Wed, 3 Feb 2021 21:07:29 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH 09/11] x86/fault: Rename no_context() to kernelmode_fixup_or_oops() |
| |
On Wed, Feb 03, 2021 at 11:53:03AM -0800, Andy Lutomirski wrote: > I feel like that would be more obfuscated — then the function would > return without fixing anything for usermode faults, return after > fixing it for kernel mode faults, or oops.
You practically pretty much have it already with the WARN_ON_ONCE. And you can make the thing return 1 to denote it was in user_mode() and 0 otherwise. IINMSO, something like this:
--- diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index 08f5f74cf989..2b86d541b181 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -692,11 +692,12 @@ page_fault_oops(struct pt_regs *regs, unsigned long error_code, oops_end(flags, regs, sig); } -static noinline void +static noinline int kernelmode_fixup_or_oops(struct pt_regs *regs, unsigned long error_code, unsigned long address, int signal, int si_code) { - WARN_ON_ONCE(user_mode(regs)); + if (WARN_ON_ONCE(user_mode(regs))) + return 1; /* Are we prepared to handle this kernel fault? */ if (fixup_exception(regs, X86_TRAP_PF, error_code, address)) { @@ -706,7 +707,7 @@ kernelmode_fixup_or_oops(struct pt_regs *regs, unsigned long error_code, * task context. */ if (in_interrupt()) - return; + return 0; /* * Per the above we're !in_interrupt(), aka. task context. @@ -726,7 +727,7 @@ kernelmode_fixup_or_oops(struct pt_regs *regs, unsigned long error_code, /* * Barring that, we can do the fixup and be happy. */ - return; + return 0; } /* @@ -734,9 +735,11 @@ kernelmode_fixup_or_oops(struct pt_regs *regs, unsigned long error_code, * instruction. */ if (is_prefetch(regs, error_code, address)) - return; + return 0; page_fault_oops(regs, error_code, address); + + return 0; } /* @@ -781,10 +784,8 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, { struct task_struct *tsk = current; - if (!user_mode(regs)) { - kernelmode_fixup_or_oops(regs, error_code, address, pkey, si_code); + if (!kernelmode_fixup_or_oops(regs, error_code, address, pkey, si_code)) return; - } if (!(error_code & X86_PF_USER)) { /* Implicit user access to kernel memory -- just oops */ @@ -913,10 +914,8 @@ do_sigbus(struct pt_regs *regs, unsigned long error_code, unsigned long address, vm_fault_t fault) { /* Kernel mode? Handle exceptions or die: */ - if (!user_mode(regs)) { - kernelmode_fixup_or_oops(regs, error_code, address, SIGBUS, BUS_ADRERR); + if (!kernelmode_fixup_or_oops(regs, error_code, address, SIGBUS, BUS_ADRERR)) return; - } /* User-space => ok to do another page fault: */ if (is_prefetch(regs, error_code, address)) @@ -1378,10 +1377,8 @@ void do_user_addr_fault(struct pt_regs *regs, * Quick path to respond to signals. The core mm code * has unlocked the mm for us if we get here. */ - if (!user_mode(regs)) - kernelmode_fixup_or_oops(regs, error_code, address, - SIGBUS, BUS_ADRERR); - return; + if (!kernelmode_fixup_or_oops(regs, error_code, address, SIGBUS, BUS_ADRERR)) + return; } /* @@ -1399,18 +1396,15 @@ void do_user_addr_fault(struct pt_regs *regs, if (likely(!(fault & VM_FAULT_ERROR))) return; - if (fatal_signal_pending(current) && !user_mode(regs)) { - kernelmode_fixup_or_oops(regs, error_code, address, 0, 0); - return; + if (fatal_signal_pending(current)) { + if (!kernelmode_fixup_or_oops(regs, error_code, address, 0, 0)) + return; } if (fault & VM_FAULT_OOM) { /* Kernel mode? Handle exceptions or die: */ - if (!user_mode(regs)) { - kernelmode_fixup_or_oops(regs, error_code, address, - SIGSEGV, SEGV_MAPERR); + if (!kernelmode_fixup_or_oops(regs, error_code, address, SIGSEGV, SEGV_MAPERR)) return; - } /* * We ran out of memory, call the OOM killer, and return the -- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |