Skip to content

Commit 61d64a3

Browse files
mrutland-armwilldeacon
authored andcommitted
arm64: split EL0/EL1 UNDEF handlers
In general, exceptions taken from EL1 need to be handled separately from exceptions taken from EL0, as the logic to handle the two cases can be significantly divergent, and exceptions taken from EL1 typically have more stringent requirements on locking and instrumentation. Subsequent patches will rework the way EL1 UNDEFs are handled in order to address longstanding soundness issues with instrumentation and RCU. In preparation for that rework, this patch splits the existing do_undefinstr() handler into separate do_el0_undef() and do_el1_undef() handlers. Prior to this patch, do_undefinstr() was marked with NOKPROBE_SYMBOL(), preventing instrumentation via kprobes. However, do_undefinstr() invokes other code which can be instrumented, and: * For UNDEFINED exceptions taken from EL0, there is no risk of recursion within kprobes. Therefore it is safe for do_el0_undef to be instrumented with kprobes, and it does not need to be marked with NOKPROBE_SYMBOL(). * For UNDEFINED exceptions taken from EL1, either: (a) The exception is has been taken when manipulating SSBS; these cases are limited and do not occur within code that can be invoked recursively via kprobes. Hence, in these cases instrumentation with kprobes is benign. (b) The exception has been taken for an unknown reason, as other than manipulating SSBS we do not expect to take UNDEFINED exceptions from EL1. Any handling of these exception is best-effort. ... and in either case, marking do_el1_undef() with NOKPROBE_SYMBOL() isn't sufficient to prevent recursion via kprobes as functions it calls (including die()) are instrumentable via kprobes. Hence, it's not worthwhile to mark do_el1_undef() with NOKPROBE_SYMBOL(). The same applies to do_el1_bti() and do_el1_fpac(), so their NOKPROBE_SYMBOL() annotations are also removed. Aside from the new instrumentability, there should be no functional change as a result of this patch. Signed-off-by: Mark Rutland <[email protected]> Cc: Catalin Marinas <[email protected]> Cc: James Morse <[email protected]> Cc: Joey Gouly <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Will Deacon <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Will Deacon <[email protected]>
1 parent b3a0c01 commit 61d64a3

File tree

3 files changed

+16
-13
lines changed

3 files changed

+16
-13
lines changed

arch/arm64/include/asm/exception.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ asmlinkage void call_on_irq_stack(struct pt_regs *regs,
5858
asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);
5959

6060
void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
61-
void do_undefinstr(struct pt_regs *regs, unsigned long esr);
61+
void do_el0_undef(struct pt_regs *regs, unsigned long esr);
62+
void do_el1_undef(struct pt_regs *regs, unsigned long esr);
6263
void do_el0_bti(struct pt_regs *regs);
6364
void do_el1_bti(struct pt_regs *regs, unsigned long esr);
6465
void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,

arch/arm64/kernel/entry-common.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ static void noinstr el1_undef(struct pt_regs *regs, unsigned long esr)
384384
{
385385
enter_from_kernel_mode(regs);
386386
local_daif_inherit(regs);
387-
do_undefinstr(regs, esr);
387+
do_el1_undef(regs, esr);
388388
local_daif_mask();
389389
exit_to_kernel_mode(regs);
390390
}
@@ -599,7 +599,7 @@ static void noinstr el0_undef(struct pt_regs *regs, unsigned long esr)
599599
{
600600
enter_from_user_mode(regs);
601601
local_daif_restore(DAIF_PROCCTX);
602-
do_undefinstr(regs, esr);
602+
do_el0_undef(regs, esr);
603603
exit_to_user_mode(regs);
604604
}
605605

arch/arm64/kernel/traps.c

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ void arm64_notify_segfault(unsigned long addr)
486486
force_signal_inject(SIGSEGV, code, addr, 0);
487487
}
488488

489-
void do_undefinstr(struct pt_regs *regs, unsigned long esr)
489+
void do_el0_undef(struct pt_regs *regs, unsigned long esr)
490490
{
491491
/* check for AArch32 breakpoint instructions */
492492
if (!aarch32_break_handler(regs))
@@ -495,12 +495,16 @@ void do_undefinstr(struct pt_regs *regs, unsigned long esr)
495495
if (call_undef_hook(regs) == 0)
496496
return;
497497

498-
if (!user_mode(regs))
499-
die("Oops - Undefined instruction", regs, esr);
500-
501498
force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc, 0);
502499
}
503-
NOKPROBE_SYMBOL(do_undefinstr);
500+
501+
void do_el1_undef(struct pt_regs *regs, unsigned long esr)
502+
{
503+
if (call_undef_hook(regs) == 0)
504+
return;
505+
506+
die("Oops - Undefined instruction", regs, esr);
507+
}
504508

505509
void do_el0_bti(struct pt_regs *regs)
506510
{
@@ -511,7 +515,6 @@ void do_el1_bti(struct pt_regs *regs, unsigned long esr)
511515
{
512516
die("Oops - BTI", regs, esr);
513517
}
514-
NOKPROBE_SYMBOL(do_el1_bti);
515518

516519
void do_el0_fpac(struct pt_regs *regs, unsigned long esr)
517520
{
@@ -526,7 +529,6 @@ void do_el1_fpac(struct pt_regs *regs, unsigned long esr)
526529
*/
527530
die("Oops - FPAC", regs, esr);
528531
}
529-
NOKPROBE_SYMBOL(do_el1_fpac)
530532

531533
#define __user_cache_maint(insn, address, res) \
532534
if (address >= TASK_SIZE_MAX) { \
@@ -769,7 +771,7 @@ void do_el0_cp15(unsigned long esr, struct pt_regs *regs)
769771
hook_base = cp15_64_hooks;
770772
break;
771773
default:
772-
do_undefinstr(regs, esr);
774+
do_el0_undef(regs, esr);
773775
return;
774776
}
775777

@@ -784,7 +786,7 @@ void do_el0_cp15(unsigned long esr, struct pt_regs *regs)
784786
* EL0. Fall back to our usual undefined instruction handler
785787
* so that we handle these consistently.
786788
*/
787-
do_undefinstr(regs, esr);
789+
do_el0_undef(regs, esr);
788790
}
789791
#endif
790792

@@ -803,7 +805,7 @@ void do_el0_sys(unsigned long esr, struct pt_regs *regs)
803805
* back to our usual undefined instruction handler so that we handle
804806
* these consistently.
805807
*/
806-
do_undefinstr(regs, esr);
808+
do_el0_undef(regs, esr);
807809
}
808810

809811
static const char *esr_class_str[] = {

0 commit comments

Comments
 (0)