Skip to content

Commit 614da1d

Browse files
binxingtorvalds
authored andcommitted
x86: make page fault handling disable interrupts properly
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 ca4c6a9 ("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: ca4c6a9 ("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>
1 parent 0a80e38 commit 614da1d

1 file changed

Lines changed: 5 additions & 10 deletions

File tree

arch/x86/mm/fault.c

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -821,8 +821,6 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code,
821821
force_sig_pkuerr((void __user *)address, pkey);
822822
else
823823
force_sig_fault(SIGSEGV, si_code, (void __user *)address);
824-
825-
local_irq_disable();
826824
}
827825

828826
static noinline void
@@ -1474,15 +1472,12 @@ handle_page_fault(struct pt_regs *regs, unsigned long error_code,
14741472
do_kern_addr_fault(regs, error_code, address);
14751473
} else {
14761474
do_user_addr_fault(regs, error_code, address);
1477-
/*
1478-
* User address page fault handling might have reenabled
1479-
* interrupts. Fixing up all potential exit points of
1480-
* do_user_addr_fault() and its leaf functions is just not
1481-
* doable w/o creating an unholy mess or turning the code
1482-
* upside down.
1483-
*/
1484-
local_irq_disable();
14851475
}
1476+
/*
1477+
* page fault handling might have reenabled interrupts,
1478+
* make sure to disable them again.
1479+
*/
1480+
local_irq_disable();
14861481
}
14871482

14881483
DEFINE_IDTENTRY_RAW_ERRORCODE(exc_page_fault)

0 commit comments

Comments
 (0)