| From db17a20431c5ab640018f9ab395a3e7846a1ac30 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Thu, 22 Jan 2026 18:39:15 -0600 |
| Subject: x86: make page fault handling disable interrupts properly |
| |
| From: Cedric Xing <cedric.xing@intel.com> |
| |
| [ Upstream commit 614da1d3d4cdbd6e41aea06bc97ec15aacff6daf ] |
| |
| There's a big comment in the x86 do_page_fault() about our interrupt |
| disabling code: |
| |
| * User address page fault handling might have reenabled |
| * interrupts. Fixing up all potential exit points of |
| * do_user_addr_fault() and its leaf functions is just not |
| * doable w/o creating an unholy mess or turning the code |
| * upside down. |
| |
| but it turns out that comment is subtly wrong, and the code as a result |
| is also wrong. |
| |
| Because it's certainly true that we may have re-enabled interrupts when |
| handling user page faults. And it's most certainly true that we don't |
| want to bother fixing up all the cases. |
| |
| But what isn't true is that it's limited to user address page faults. |
| |
| The confusion stems from the fact that we have logic here that depends |
| on the address range of the access, but other code then depends on the |
| _context_ the access was done in. The two are not related, even though |
| both of them are about user-vs-kernel. |
| |
| In other words, both user and kernel addresses can cause interrupts to |
| have been enabled (eg when __bad_area_nosemaphore() gets called for user |
| accesses to kernel addresses). As a result we should make sure to |
| disable interrupts again regardless of the address range before |
| returning to the low-level fault handling code. |
| |
| The __bad_area_nosemaphore() code actually did disable interrupts again |
| after enabling them, just not consistently. Ironically, as noted in the |
| original comment, fixing up all the cases is just not worth it, when the |
| simple solution is to just do it unconditionally in one single place. |
| |
| So remove the incomplete case that unsuccessfully tried to do what the |
| comment said was "not doable" in commit ca4c6a9858c2 ("x86/traps: Make |
| interrupt enable/disable symmetric in C code"), and just make it do the |
| simple and straightforward thing. |
| |
| Signed-off-by: Cedric Xing <cedric.xing@intel.com> |
| Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com> |
| Fixes: ca4c6a9858c2 ("x86/traps: Make interrupt enable/disable symmetric in C code") |
| Cc: Peter Zijlstra <peterz@infradead.org> |
| Cc: Thomas Gleixner <tglx@linutronix.de> |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| arch/x86/mm/fault.c | 15 +++++---------- |
| 1 file changed, 5 insertions(+), 10 deletions(-) |
| |
| diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c |
| index 998bd807fc7ba..b83a06739b511 100644 |
| --- a/arch/x86/mm/fault.c |
| +++ b/arch/x86/mm/fault.c |
| @@ -821,8 +821,6 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, |
| force_sig_pkuerr((void __user *)address, pkey); |
| else |
| force_sig_fault(SIGSEGV, si_code, (void __user *)address); |
| - |
| - local_irq_disable(); |
| } |
| |
| static noinline void |
| @@ -1474,15 +1472,12 @@ handle_page_fault(struct pt_regs *regs, unsigned long error_code, |
| do_kern_addr_fault(regs, error_code, address); |
| } else { |
| do_user_addr_fault(regs, error_code, address); |
| - /* |
| - * User address page fault handling might have reenabled |
| - * interrupts. Fixing up all potential exit points of |
| - * do_user_addr_fault() and its leaf functions is just not |
| - * doable w/o creating an unholy mess or turning the code |
| - * upside down. |
| - */ |
| - local_irq_disable(); |
| } |
| + /* |
| + * page fault handling might have reenabled interrupts, |
| + * make sure to disable them again. |
| + */ |
| + local_irq_disable(); |
| } |
| |
| DEFINE_IDTENTRY_RAW_ERRORCODE(exc_page_fault) |
| -- |
| 2.51.0 |
| |