linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] arm64: report EL1 exceptions better
@ 2022-09-13 10:17 Mark Rutland
  2022-09-13 10:17 ` [PATCH v2 1/5] arm64: report EL1 UNDEFs better Mark Rutland
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Mark Rutland @ 2022-09-13 10:17 UTC (permalink / raw)
  To: linux-arm-kernel, catalin.marinas
  Cc: alexandru.elisei, amit.kachhap, anshuman.khandual, broonie,
	james.morse, mark.rutland, will

We treat BTI, FPAC, and UNDEFINED exceptions as fatal when taken from
EL1, but due to the way we make these fatal, we end up throwing away
information that could be used to diagnose the cause of the exception.

We have an anti-pattern where in the handler for the exception we do:

	BUG_ON(!user_mode(regs));

... and consequently, the BUG handler logs the context of the exception
handler which contained the BUG_ON(), rather than the context from which
the fatal exception was taken.

For example, today we report UNDEFINED exceptions as:

| kernel BUG at arch/arm64/kernel/traps.c:497!
| Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
| Modules linked in:
| CPU: 0 PID: 0 Comm: swapper Not tainted 5.19.0-rc3-00127-geff044f1b04e-dirty #3
| Hardware name: linux,dummy-virt (DT)
| pstate: 000000c5 (nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : do_undefinstr+0x28c/0x2ac
| lr : do_undefinstr+0x298/0x2ac
| sp : ffff800009f63bc0
| x29: ffff800009f63bc0 x28: ffff800009f73c00 x27: ffff800009644a70
| x26: ffff8000096778a8 x25: 0000000000000040 x24: 0000000000000000
| x23: 00000000800000c5 x22: ffff800009894060 x21: ffff800009f63d90
| x20: 0000000000000000 x19: ffff800009f63c40 x18: 0000000000000006
| x17: 0000000000403000 x16: 00000000bfbfd000 x15: ffff800009f63830
| x14: ffffffffffffffff x13: 0000000000000000 x12: 0000000000000019
| x11: 0101010101010101 x10: 0000000000161b98 x9 : 0000000000000000
| x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
| x5 : ffff800009f761d0 x4 : 0000000000000000 x3 : ffff80000a2b80f8
| x2 : 0000000000000000 x1 : ffff800009f73c00 x0 : 00000000800000c5
| Call trace:
|  do_undefinstr+0x28c/0x2ac
|  el1_undef+0x2c/0x4c
|  el1h_64_sync_handler+0x84/0xd0
|  el1h_64_sync+0x64/0x68
|  setup_arch+0x550/0x598
|  start_kernel+0x88/0x6ac
|  __primary_switched+0xb8/0xc0
| Code: 17ffff95 a9425bf5 17ffffb8 a9025bf5 (d4210000)

... where the code dump is for the BRK instruction in the BUG_ON(), and
the register contents are for the innards of do_undefinstr(). The
instruction which triggered the UNDEFINED exception is nowhere in the
dump.

This series reworks the handling of these exceptions to be more useful,
reporting the original context of the exception, and calling die()
directly rather than using BUG_ON().

For example, as of this series being applied, UNDEFINED exceptions are
reported as:

| Internal error: Oops - Undefined instruction: 0 [#1] PREEMPT SMP
| Modules linked in:
| CPU: 0 PID: 0 Comm: swapper Not tainted 5.19.0-rc3-00128-gf27cfcc80e52-dirty #5
| Hardware name: linux,dummy-virt (DT)
| pstate: 800000c5 (Nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : setup_arch+0x550/0x598
| lr : setup_arch+0x50c/0x598
| sp : ffff800009f63d90
| x29: ffff800009f63d90 x28: 0000000081000200 x27: ffff800009644a70
| x26: ffff8000096778c8 x25: 0000000000000040 x24: 0000000000000000
| x23: 0000000000000100 x22: ffff800009f69a58 x21: ffff80000a2b80b8
| x20: 0000000000000000 x19: 0000000000000000 x18: 0000000000000006
| x17: 0000000000403000 x16: 00000000bfbfd000 x15: ffff800009f63830
| x14: ffffffffffffffff x13: 0000000000000000 x12: 0000000000000019
| x11: 0101010101010101 x10: 0000000000161b98 x9 : 0000000000000000
| x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
| x5 : 0000000000000008 x4 : 0000000000000010 x3 : 0000000000000000
| x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
| Call trace:
|  setup_arch+0x550/0x598
|  start_kernel+0x88/0x6ac
|  __primary_switched+0xb8/0xc0
| Code: b4000080 90ffed80 912ac000 97db745f (00000000)

... where the code dump includes the instruction which triggered the
exception, and the register contents are for the context the instruction
was executed in.

Since v1 [1]:
* Rebase (trivial) to v6.0-rc3
* Fix typos
* Consistently pass ESR to die()
* Add Reviewed-by tags

[1] https://lore.kernel.org/linux-arm-kernel/20220712161452.4142444-1-mark.rutland@arm.com/

Mark.

Mark Rutland (5):
  arm64: report EL1 UNDEFs better
  arm64: die(): pass 'err' as long
  arm64: consistently pass ESR_ELx to die()
  arm64: rework FPAC exception handling
  arm64: rework BTI exception handling

 arch/arm64/include/asm/exception.h   |  8 +++--
 arch/arm64/include/asm/system_misc.h |  2 +-
 arch/arm64/kernel/entry-common.c     | 32 +++++++++++++------
 arch/arm64/kernel/traps.c            | 48 +++++++++++++++++-----------
 4 files changed, 57 insertions(+), 33 deletions(-)

-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/5] arm64: report EL1 UNDEFs better
  2022-09-13 10:17 [PATCH v2 0/5] arm64: report EL1 exceptions better Mark Rutland
@ 2022-09-13 10:17 ` Mark Rutland
  2022-09-14  5:32   ` Anshuman Khandual
  2022-09-13 10:17 ` [PATCH v2 2/5] arm64: die(): pass 'err' as long Mark Rutland
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Mark Rutland @ 2022-09-13 10:17 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: alexandru.elisei, amit.kachhap, anshuman.khandual, broonie,
	catalin.marinas, james.morse, mark.rutland, will

If an UNDEFINED exception is taken from EL1, and do_undefinstr() doesn't
find any suitable undef_hook, it will call:

	BUG_ON(!user_mode(regs))

... and the kernel will report a failure witin do_undefinstr() rather
than reporting the original context that the UNDEFINED exception was
taken from. The pt_regs and ESR value reported within the BUG() handler
will be from within do_undefinstr() and the code dump will be for the
BRK in BUG_ON(), which isn't sufficient to debug the cause of the
original exception.

This patch makes the reporting better by having do_undefinstr() call
die() directly in this case to report the original context from which
the UNDEFINED exception was taken.

Prior to this patch, an undefined instruction is reported as:

| kernel BUG at arch/arm64/kernel/traps.c:497!
| Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
| Modules linked in:
| CPU: 0 PID: 0 Comm: swapper Not tainted 5.19.0-rc3-00127-geff044f1b04e-dirty #3
| Hardware name: linux,dummy-virt (DT)
| pstate: 000000c5 (nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : do_undefinstr+0x28c/0x2ac
| lr : do_undefinstr+0x298/0x2ac
| sp : ffff800009f63bc0
| x29: ffff800009f63bc0 x28: ffff800009f73c00 x27: ffff800009644a70
| x26: ffff8000096778a8 x25: 0000000000000040 x24: 0000000000000000
| x23: 00000000800000c5 x22: ffff800009894060 x21: ffff800009f63d90
| x20: 0000000000000000 x19: ffff800009f63c40 x18: 0000000000000006
| x17: 0000000000403000 x16: 00000000bfbfd000 x15: ffff800009f63830
| x14: ffffffffffffffff x13: 0000000000000000 x12: 0000000000000019
| x11: 0101010101010101 x10: 0000000000161b98 x9 : 0000000000000000
| x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
| x5 : ffff800009f761d0 x4 : 0000000000000000 x3 : ffff80000a2b80f8
| x2 : 0000000000000000 x1 : ffff800009f73c00 x0 : 00000000800000c5
| Call trace:
|  do_undefinstr+0x28c/0x2ac
|  el1_undef+0x2c/0x4c
|  el1h_64_sync_handler+0x84/0xd0
|  el1h_64_sync+0x64/0x68
|  setup_arch+0x550/0x598
|  start_kernel+0x88/0x6ac
|  __primary_switched+0xb8/0xc0
| Code: 17ffff95 a9425bf5 17ffffb8 a9025bf5 (d4210000)

With this patch applied, an undefined instruction is reported as:

| Internal error: Oops - Undefined instruction: 0 [#1] PREEMPT SMP
| Modules linked in:
| CPU: 0 PID: 0 Comm: swapper Not tainted 5.19.0-rc3-00128-gf27cfcc80e52-dirty #5
| Hardware name: linux,dummy-virt (DT)
| pstate: 800000c5 (Nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : setup_arch+0x550/0x598
| lr : setup_arch+0x50c/0x598
| sp : ffff800009f63d90
| x29: ffff800009f63d90 x28: 0000000081000200 x27: ffff800009644a70
| x26: ffff8000096778c8 x25: 0000000000000040 x24: 0000000000000000
| x23: 0000000000000100 x22: ffff800009f69a58 x21: ffff80000a2b80b8
| x20: 0000000000000000 x19: 0000000000000000 x18: 0000000000000006
| x17: 0000000000403000 x16: 00000000bfbfd000 x15: ffff800009f63830
| x14: ffffffffffffffff x13: 0000000000000000 x12: 0000000000000019
| x11: 0101010101010101 x10: 0000000000161b98 x9 : 0000000000000000
| x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
| x5 : 0000000000000008 x4 : 0000000000000010 x3 : 0000000000000000
| x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
| Call trace:
|  setup_arch+0x550/0x598
|  start_kernel+0x88/0x6ac
|  __primary_switched+0xb8/0xc0
| Code: b4000080 90ffed80 912ac000 97db745f (00000000)

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/traps.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index b7fed33981f7b..eac4f7a831750 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -494,7 +494,9 @@ void do_undefinstr(struct pt_regs *regs)
 	if (call_undef_hook(regs) == 0)
 		return;
 
-	BUG_ON(!user_mode(regs));
+	if (!user_mode(regs))
+		die("Oops - Undefined instruction", regs, 0);
+
 	force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc, 0);
 }
 NOKPROBE_SYMBOL(do_undefinstr);
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/5] arm64: die(): pass 'err' as long
  2022-09-13 10:17 [PATCH v2 0/5] arm64: report EL1 exceptions better Mark Rutland
  2022-09-13 10:17 ` [PATCH v2 1/5] arm64: report EL1 UNDEFs better Mark Rutland
@ 2022-09-13 10:17 ` Mark Rutland
  2022-09-13 10:17 ` [PATCH v2 3/5] arm64: consistently pass ESR_ELx to die() Mark Rutland
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2022-09-13 10:17 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: alexandru.elisei, amit.kachhap, anshuman.khandual, broonie,
	catalin.marinas, james.morse, mark.rutland, will

Recently, we reworked a lot of code to consistentlt pass ESR_ELx as a
64-bit quantity. However, we missed that this can be passed into die()
and __die() as the 'err' parameter where it is truncated to a 32-bit
int.

As notify_die() already takes 'err' as a long, this patch changes die()
and __die() to also take 'err' as a long, ensuring that the full value
of ESR_ELx is retained.

At the same time, die() is updated to consistently log 'err' as a
zero-padded 64-bit quantity.

Subsequent patches will pass the ESR_ELx value to die() for a number of
exceptions.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/system_misc.h | 2 +-
 arch/arm64/kernel/traps.c            | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
index 0eb7709422e29..c343442567625 100644
--- a/arch/arm64/include/asm/system_misc.h
+++ b/arch/arm64/include/asm/system_misc.h
@@ -18,7 +18,7 @@
 
 struct pt_regs;
 
-void die(const char *msg, struct pt_regs *regs, int err);
+void die(const char *msg, struct pt_regs *regs, long err);
 
 struct siginfo;
 void arm64_notify_die(const char *str, struct pt_regs *regs,
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index eac4f7a831750..da8ffef6f7c58 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -180,12 +180,12 @@ static void dump_kernel_instr(const char *lvl, struct pt_regs *regs)
 
 #define S_SMP " SMP"
 
-static int __die(const char *str, int err, struct pt_regs *regs)
+static int __die(const char *str, long err, struct pt_regs *regs)
 {
 	static int die_counter;
 	int ret;
 
-	pr_emerg("Internal error: %s: %x [#%d]" S_PREEMPT S_SMP "\n",
+	pr_emerg("Internal error: %s: %016lx [#%d]" S_PREEMPT S_SMP "\n",
 		 str, err, ++die_counter);
 
 	/* trap and error numbers are mostly meaningless on ARM */
@@ -206,7 +206,7 @@ static DEFINE_RAW_SPINLOCK(die_lock);
 /*
  * This function is protected against re-entrancy.
  */
-void die(const char *str, struct pt_regs *regs, int err)
+void die(const char *str, struct pt_regs *regs, long err)
 {
 	int ret;
 	unsigned long flags;
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/5] arm64: consistently pass ESR_ELx to die()
  2022-09-13 10:17 [PATCH v2 0/5] arm64: report EL1 exceptions better Mark Rutland
  2022-09-13 10:17 ` [PATCH v2 1/5] arm64: report EL1 UNDEFs better Mark Rutland
  2022-09-13 10:17 ` [PATCH v2 2/5] arm64: die(): pass 'err' as long Mark Rutland
@ 2022-09-13 10:17 ` Mark Rutland
  2022-09-13 11:48   ` Mark Brown
  2022-09-14  5:52   ` Anshuman Khandual
  2022-09-13 10:17 ` [PATCH v2 4/5] arm64: rework FPAC exception handling Mark Rutland
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 10+ messages in thread
From: Mark Rutland @ 2022-09-13 10:17 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: alexandru.elisei, amit.kachhap, anshuman.khandual, broonie,
	catalin.marinas, james.morse, mark.rutland, will

Currently, bug_handler() and kasan_handler() call die() with '0' as the
'err' value, whereas die_kernel_fault() passes the ESR_ELx value.

For consistency, this patch ensures we always pass the ESR_ELx value to
die(). As this is only called for exceptions taken from kernel mode,
there should be no user-visible change as a result of this patch.

For UNDEFINED exceptions, I've had to modify do_undefinstr() and its
callers to pass the ESR_ELx value. In all cases the ESR_ELx value had
already been read and was available.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/exception.h |  2 +-
 arch/arm64/kernel/entry-common.c   | 14 +++++++-------
 arch/arm64/kernel/traps.c          | 14 +++++++-------
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index d94aecff96902..28c0275666e12 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -58,7 +58,7 @@ asmlinkage void call_on_irq_stack(struct pt_regs *regs,
 asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);
 
 void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
-void do_undefinstr(struct pt_regs *regs);
+void do_undefinstr(struct pt_regs *regs, unsigned long esr);
 void do_bti(struct pt_regs *regs);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,
 			struct pt_regs *regs);
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index c75ca36b4a491..3dd17ef6d6d8b 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -379,11 +379,11 @@ static void noinstr el1_pc(struct pt_regs *regs, unsigned long esr)
 	exit_to_kernel_mode(regs);
 }
 
-static void noinstr el1_undef(struct pt_regs *regs)
+static void noinstr el1_undef(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_kernel_mode(regs);
 	local_daif_inherit(regs);
-	do_undefinstr(regs);
+	do_undefinstr(regs, esr);
 	local_daif_mask();
 	exit_to_kernel_mode(regs);
 }
@@ -425,7 +425,7 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
 		break;
 	case ESR_ELx_EC_SYS64:
 	case ESR_ELx_EC_UNKNOWN:
-		el1_undef(regs);
+		el1_undef(regs, esr);
 		break;
 	case ESR_ELx_EC_BREAKPT_CUR:
 	case ESR_ELx_EC_SOFTSTP_CUR:
@@ -582,11 +582,11 @@ static void noinstr el0_sp(struct pt_regs *regs, unsigned long esr)
 	exit_to_user_mode(regs);
 }
 
-static void noinstr el0_undef(struct pt_regs *regs)
+static void noinstr el0_undef(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
 	local_daif_restore(DAIF_PROCCTX);
-	do_undefinstr(regs);
+	do_undefinstr(regs, esr);
 	exit_to_user_mode(regs);
 }
 
@@ -670,7 +670,7 @@ asmlinkage void noinstr el0t_64_sync_handler(struct pt_regs *regs)
 		el0_pc(regs, esr);
 		break;
 	case ESR_ELx_EC_UNKNOWN:
-		el0_undef(regs);
+		el0_undef(regs, esr);
 		break;
 	case ESR_ELx_EC_BTI:
 		el0_bti(regs);
@@ -788,7 +788,7 @@ asmlinkage void noinstr el0t_32_sync_handler(struct pt_regs *regs)
 	case ESR_ELx_EC_CP14_MR:
 	case ESR_ELx_EC_CP14_LS:
 	case ESR_ELx_EC_CP14_64:
-		el0_undef(regs);
+		el0_undef(regs, esr);
 		break;
 	case ESR_ELx_EC_CP15_32:
 	case ESR_ELx_EC_CP15_64:
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index da8ffef6f7c58..05999cde870a9 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -485,7 +485,7 @@ void arm64_notify_segfault(unsigned long addr)
 	force_signal_inject(SIGSEGV, code, addr, 0);
 }
 
-void do_undefinstr(struct pt_regs *regs)
+void do_undefinstr(struct pt_regs *regs, unsigned long esr)
 {
 	/* check for AArch32 breakpoint instructions */
 	if (!aarch32_break_handler(regs))
@@ -495,7 +495,7 @@ void do_undefinstr(struct pt_regs *regs)
 		return;
 
 	if (!user_mode(regs))
-		die("Oops - Undefined instruction", regs, 0);
+		die("Oops - Undefined instruction", regs, esr);
 
 	force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc, 0);
 }
@@ -760,7 +760,7 @@ void do_cp15instr(unsigned long esr, struct pt_regs *regs)
 		hook_base = cp15_64_hooks;
 		break;
 	default:
-		do_undefinstr(regs);
+		do_undefinstr(regs, esr);
 		return;
 	}
 
@@ -775,7 +775,7 @@ void do_cp15instr(unsigned long esr, struct pt_regs *regs)
 	 * EL0. Fall back to our usual undefined instruction handler
 	 * so that we handle these consistently.
 	 */
-	do_undefinstr(regs);
+	do_undefinstr(regs, esr);
 }
 NOKPROBE_SYMBOL(do_cp15instr);
 #endif
@@ -795,7 +795,7 @@ void do_sysinstr(unsigned long esr, struct pt_regs *regs)
 	 * back to our usual undefined instruction handler so that we handle
 	 * these consistently.
 	 */
-	do_undefinstr(regs);
+	do_undefinstr(regs, esr);
 }
 NOKPROBE_SYMBOL(do_sysinstr);
 
@@ -972,7 +972,7 @@ static int bug_handler(struct pt_regs *regs, unsigned long esr)
 {
 	switch (report_bug(regs->pc, regs)) {
 	case BUG_TRAP_TYPE_BUG:
-		die("Oops - BUG", regs, 0);
+		die("Oops - BUG", regs, esr);
 		break;
 
 	case BUG_TRAP_TYPE_WARN:
@@ -1040,7 +1040,7 @@ static int kasan_handler(struct pt_regs *regs, unsigned long esr)
 	 * This is something that might be fixed at some point in the future.
 	 */
 	if (!recover)
-		die("Oops - KASAN", regs, 0);
+		die("Oops - KASAN", regs, esr);
 
 	/* If thread survives, skip over the brk instruction and continue: */
 	arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/5] arm64: rework FPAC exception handling
  2022-09-13 10:17 [PATCH v2 0/5] arm64: report EL1 exceptions better Mark Rutland
                   ` (2 preceding siblings ...)
  2022-09-13 10:17 ` [PATCH v2 3/5] arm64: consistently pass ESR_ELx to die() Mark Rutland
@ 2022-09-13 10:17 ` Mark Rutland
  2022-09-13 10:17 ` [PATCH v2 5/5] arm64: rework BTI " Mark Rutland
  2022-09-16 17:46 ` [PATCH v2 0/5] arm64: report EL1 exceptions better Catalin Marinas
  5 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2022-09-13 10:17 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: alexandru.elisei, amit.kachhap, anshuman.khandual, broonie,
	catalin.marinas, james.morse, mark.rutland, will

If an FPAC exception is taken from EL1, the entry code will call
do_ptrauth_fault(), where due to:

	BUG_ON(!user_mode(regs))

... the kernel will report a problem within do_ptrauth_fault() rather
than reporting the original context the FPAC exception was taken from.
The pt_regs and ESR value reported will be from within
do_ptrauth_fault() and the code dump will be for the BRK in BUG_ON(),
which isn't sufficient to debug the cause of the original exception.

This patch makes the reporting better by having separate EL0 and EL1
FPAC exception handlers, with the latter calling die() directly to
report the original context the FPAC exception was taken from.

Note that we only need to prevent kprobes of the EL1 FPAC handler, since
the EL0 FPAC handler cannot be called recursively.

For consistency with do_el0_svc*(), I've named the split functions
do_el{0,1}_fpac() rather than do_el{0,1}_ptrauth_fault(). I've also
clarified the comment to not imply there are casues other than FPAC
exceptions.

Prior to this patch FPAC exceptions are reported as:

| kernel BUG at arch/arm64/kernel/traps.c:517!
| Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
| Modules linked in:
| CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc3-00130-g9c8a180a1cdf-dirty #12
| Hardware name: FVP Base RevC (DT)
| pstate: 00400009 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : do_ptrauth_fault+0x3c/0x40
| lr : el1_fpac+0x34/0x54
| sp : ffff80000a3bbc80
| x29: ffff80000a3bbc80 x28: ffff0008001d8000 x27: 0000000000000000
| x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
| x23: 0000000020400009 x22: ffff800008f70fa4 x21: ffff80000a3bbe00
| x20: 0000000072000000 x19: ffff80000a3bbcb0 x18: fffffbfffda37000
| x17: 3120676e696d7573 x16: 7361202c6e6f6974 x15: 0000000081a90000
| x14: 0040000000000041 x13: 0040000000000001 x12: ffff000001a90000
| x11: fffffbfffda37480 x10: 0068000000000703 x9 : 0001000080000000
| x8 : 0000000000090000 x7 : 0068000000000f03 x6 : 0060000000000783
| x5 : ffff80000a3bbcb0 x4 : ffff0008001d8000 x3 : 0000000072000000
| x2 : 0000000000000000 x1 : 0000000020400009 x0 : ffff80000a3bbcb0
| Call trace:
|  do_ptrauth_fault+0x3c/0x40
|  el1h_64_sync_handler+0xc4/0xd0
|  el1h_64_sync+0x64/0x68
|  test_pac+0x8/0x10
|  smp_init+0x7c/0x8c
|  kernel_init_freeable+0x128/0x28c
|  kernel_init+0x28/0x13c
|  ret_from_fork+0x10/0x20
| Code: 97fffe5e a8c17bfd d50323bf d65f03c0 (d4210000)

With this patch applied FPAC exceptions are reported as:

| Internal error: Oops - FPAC: 0000000072000000 [#1] PREEMPT SMP
| Modules linked in:
| CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc3-00132-g78846e1c4757-dirty #11
| Hardware name: FVP Base RevC (DT)
| pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : test_pac+0x8/0x10
| lr : 0x0
| sp : ffff80000a3bbe00
| x29: ffff80000a3bbe00 x28: 0000000000000000 x27: 0000000000000000
| x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
| x23: ffff80000a2c8000 x22: 0000000000000000 x21: 0000000000000000
| x20: ffff8000099fa5b0 x19: ffff80000a007000 x18: fffffbfffda37000
| x17: 3120676e696d7573 x16: 7361202c6e6f6974 x15: 0000000081a90000
| x14: 0040000000000041 x13: 0040000000000001 x12: ffff000001a90000
| x11: fffffbfffda37480 x10: 0068000000000703 x9 : 0001000080000000
| x8 : 0000000000090000 x7 : 0068000000000f03 x6 : 0060000000000783
| x5 : ffff80000a2c6000 x4 : ffff0008001d8000 x3 : ffff800009f88378
| x2 : 0000000000000000 x1 : 0000000080210000 x0 : ffff000001a90000
| Call trace:
|  test_pac+0x8/0x10
|  smp_init+0x7c/0x8c
|  kernel_init_freeable+0x128/0x28c
|  kernel_init+0x28/0x13c
|  ret_from_fork+0x10/0x20
| Code: d50323bf d65f03c0 d503233f aa1f03fe (d50323bf)

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/exception.h |  3 ++-
 arch/arm64/kernel/entry-common.c   |  4 ++--
 arch/arm64/kernel/traps.c          | 16 ++++++++++------
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 28c0275666e12..3f5141dc33412 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -72,7 +72,8 @@ void bad_el0_sync(struct pt_regs *regs, int reason, unsigned long esr);
 void do_cp15instr(unsigned long esr, struct pt_regs *regs);
 void do_el0_svc(struct pt_regs *regs);
 void do_el0_svc_compat(struct pt_regs *regs);
-void do_ptrauth_fault(struct pt_regs *regs, unsigned long esr);
+void do_el0_fpac(struct pt_regs *regs, unsigned long esr);
+void do_el1_fpac(struct pt_regs *regs, unsigned long esr);
 void do_serror(struct pt_regs *regs, unsigned long esr);
 void do_notify_resume(struct pt_regs *regs, unsigned long thread_flags);
 
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 3dd17ef6d6d8b..f334951d38e3e 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -402,7 +402,7 @@ static void noinstr el1_fpac(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_kernel_mode(regs);
 	local_daif_inherit(regs);
-	do_ptrauth_fault(regs, esr);
+	do_el1_fpac(regs, esr);
 	local_daif_mask();
 	exit_to_kernel_mode(regs);
 }
@@ -629,7 +629,7 @@ static void noinstr el0_fpac(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
 	local_daif_restore(DAIF_PROCCTX);
-	do_ptrauth_fault(regs, esr);
+	do_el0_fpac(regs, esr);
 	exit_to_user_mode(regs);
 }
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 05999cde870a9..6cec65fbf8b86 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -508,16 +508,20 @@ void do_bti(struct pt_regs *regs)
 }
 NOKPROBE_SYMBOL(do_bti);
 
-void do_ptrauth_fault(struct pt_regs *regs, unsigned long esr)
+void do_el0_fpac(struct pt_regs *regs, unsigned long esr)
+{
+	force_signal_inject(SIGILL, ILL_ILLOPN, regs->pc, esr);
+}
+
+void do_el1_fpac(struct pt_regs *regs, unsigned long esr)
 {
 	/*
-	 * Unexpected FPAC exception or pointer authentication failure in
-	 * the kernel: kill the task before it does any more harm.
+	 * Unexpected FPAC exception in the kernel: kill the task before it
+	 * does any more harm.
 	 */
-	BUG_ON(!user_mode(regs));
-	force_signal_inject(SIGILL, ILL_ILLOPN, regs->pc, esr);
+	die("Oops - FPAC", regs, esr);
 }
-NOKPROBE_SYMBOL(do_ptrauth_fault);
+NOKPROBE_SYMBOL(do_el1_fpac)
 
 #define __user_cache_maint(insn, address, res)			\
 	if (address >= TASK_SIZE_MAX) {				\
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 5/5] arm64: rework BTI exception handling
  2022-09-13 10:17 [PATCH v2 0/5] arm64: report EL1 exceptions better Mark Rutland
                   ` (3 preceding siblings ...)
  2022-09-13 10:17 ` [PATCH v2 4/5] arm64: rework FPAC exception handling Mark Rutland
@ 2022-09-13 10:17 ` Mark Rutland
  2022-09-16 17:46 ` [PATCH v2 0/5] arm64: report EL1 exceptions better Catalin Marinas
  5 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2022-09-13 10:17 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: alexandru.elisei, amit.kachhap, anshuman.khandual, broonie,
	catalin.marinas, james.morse, mark.rutland, will

If a BTI exception is taken from EL1, the entry code will treat this as
an unhandled exception and will panic() the kernel. This is inconsistent
with the way we handle FPAC exceptions, which have a dedicated handler
and only necessarily kill the thread from which the exception was taken
from, and we don't log all the information that could be relevant to
debug the issue.

The code in do_bti() has:

	BUG_ON(!user_mode(regs));

... and it seems like the intent was to call this for EL1 BTI
exceptions, as with FPAC, but this was omitted due to an oversight.

This patch adds separate EL0 and EL1 BTI exception handlers, with the
latter calling die() directly to report the original context the BTI
exception was taken from. This matches our handling of FPAC exceptions.

Prior to this patch, a BTI failure is reported as:

| Unhandled 64-bit el1h sync exception on CPU0, ESR 0x0000000034000002 -- BTI
| CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc3-00131-g7d937ff0221d-dirty #9
| Hardware name: linux,dummy-virt (DT)
| pstate: 20400809 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=-c)
| pc : test_bti_callee+0x4/0x10
| lr : test_bti_caller+0x1c/0x28
| sp : ffff80000800bdf0
| x29: ffff80000800bdf0 x28: 0000000000000000 x27: 0000000000000000
| x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
| x23: ffff80000a2b8000 x22: 0000000000000000 x21: 0000000000000000
| x20: ffff8000099fa5b0 x19: ffff800009ff7000 x18: fffffbfffda37000
| x17: 3120676e696d7573 x16: 7361202c6e6f6974 x15: 0000000041a90000
| x14: 0040000000000041 x13: 0040000000000001 x12: ffff000001a90000
| x11: fffffbfffda37480 x10: 0068000000000703 x9 : 0001000040000000
| x8 : 0000000000090000 x7 : 0068000000000f03 x6 : 0060000000000f83
| x5 : ffff80000a2b6000 x4 : ffff0000028d0000 x3 : ffff800009f78378
| x2 : 0000000000000000 x1 : 0000000040210000 x0 : ffff8000080257e4
| Kernel panic - not syncing: Unhandled exception
| CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc3-00131-g7d937ff0221d-dirty #9
| Hardware name: linux,dummy-virt (DT)
| Call trace:
|  dump_backtrace.part.0+0xcc/0xe0
|  show_stack+0x18/0x5c
|  dump_stack_lvl+0x64/0x80
|  dump_stack+0x18/0x34
|  panic+0x170/0x360
|  arm64_exit_nmi.isra.0+0x0/0x80
|  el1h_64_sync_handler+0x64/0xd0
|  el1h_64_sync+0x64/0x68
|  test_bti_callee+0x4/0x10
|  smp_cpus_done+0xb0/0xbc
|  smp_init+0x7c/0x8c
|  kernel_init_freeable+0x128/0x28c
|  kernel_init+0x28/0x13c
|  ret_from_fork+0x10/0x20

With this patch applied, a BTI failure is reported as:

| Internal error: Oops - BTI: 0000000034000002 [#1] PREEMPT SMP
| Modules linked in:
| CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc3-00132-g0ad98265d582-dirty #8
| Hardware name: linux,dummy-virt (DT)
| pstate: 20400809 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=-c)
| pc : test_bti_callee+0x4/0x10
| lr : test_bti_caller+0x1c/0x28
| sp : ffff80000800bdf0
| x29: ffff80000800bdf0 x28: 0000000000000000 x27: 0000000000000000
| x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
| x23: ffff80000a2b8000 x22: 0000000000000000 x21: 0000000000000000
| x20: ffff8000099fa5b0 x19: ffff800009ff7000 x18: fffffbfffda37000
| x17: 3120676e696d7573 x16: 7361202c6e6f6974 x15: 0000000041a90000
| x14: 0040000000000041 x13: 0040000000000001 x12: ffff000001a90000
| x11: fffffbfffda37480 x10: 0068000000000703 x9 : 0001000040000000
| x8 : 0000000000090000 x7 : 0068000000000f03 x6 : 0060000000000f83
| x5 : ffff80000a2b6000 x4 : ffff0000028d0000 x3 : ffff800009f78378
| x2 : 0000000000000000 x1 : 0000000040210000 x0 : ffff800008025804
| Call trace:
|  test_bti_callee+0x4/0x10
|  smp_cpus_done+0xb0/0xbc
|  smp_init+0x7c/0x8c
|  kernel_init_freeable+0x128/0x28c
|  kernel_init+0x28/0x13c
|  ret_from_fork+0x10/0x20
| Code: d50323bf d53cd040 d65f03c0 d503233f (d50323bf)

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/exception.h |  3 ++-
 arch/arm64/kernel/entry-common.c   | 14 +++++++++++++-
 arch/arm64/kernel/traps.c          | 10 +++++++---
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 3f5141dc33412..0278a58abe69a 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -59,7 +59,8 @@ asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);
 
 void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
 void do_undefinstr(struct pt_regs *regs, unsigned long esr);
-void do_bti(struct pt_regs *regs);
+void do_el0_bti(struct pt_regs *regs);
+void do_el1_bti(struct pt_regs *regs, unsigned long esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,
 			struct pt_regs *regs);
 void do_fpsimd_acc(unsigned long esr, struct pt_regs *regs);
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index f334951d38e3e..9173fad279af9 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -388,6 +388,15 @@ static void noinstr el1_undef(struct pt_regs *regs, unsigned long esr)
 	exit_to_kernel_mode(regs);
 }
 
+static void noinstr el1_bti(struct pt_regs *regs, unsigned long esr)
+{
+	enter_from_kernel_mode(regs);
+	local_daif_inherit(regs);
+	do_el1_bti(regs, esr);
+	local_daif_mask();
+	exit_to_kernel_mode(regs);
+}
+
 static void noinstr el1_dbg(struct pt_regs *regs, unsigned long esr)
 {
 	unsigned long far = read_sysreg(far_el1);
@@ -427,6 +436,9 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
 	case ESR_ELx_EC_UNKNOWN:
 		el1_undef(regs, esr);
 		break;
+	case ESR_ELx_EC_BTI:
+		el1_bti(regs, esr);
+		break;
 	case ESR_ELx_EC_BREAKPT_CUR:
 	case ESR_ELx_EC_SOFTSTP_CUR:
 	case ESR_ELx_EC_WATCHPT_CUR:
@@ -594,7 +606,7 @@ static void noinstr el0_bti(struct pt_regs *regs)
 {
 	enter_from_user_mode(regs);
 	local_daif_restore(DAIF_PROCCTX);
-	do_bti(regs);
+	do_el0_bti(regs);
 	exit_to_user_mode(regs);
 }
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 6cec65fbf8b86..54b5ba135b97a 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -501,12 +501,16 @@ void do_undefinstr(struct pt_regs *regs, unsigned long esr)
 }
 NOKPROBE_SYMBOL(do_undefinstr);
 
-void do_bti(struct pt_regs *regs)
+void do_el0_bti(struct pt_regs *regs)
 {
-	BUG_ON(!user_mode(regs));
 	force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc, 0);
 }
-NOKPROBE_SYMBOL(do_bti);
+
+void do_el1_bti(struct pt_regs *regs, unsigned long esr)
+{
+	die("Oops - BTI", regs, esr);
+}
+NOKPROBE_SYMBOL(do_el1_bti);
 
 void do_el0_fpac(struct pt_regs *regs, unsigned long esr)
 {
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/5] arm64: consistently pass ESR_ELx to die()
  2022-09-13 10:17 ` [PATCH v2 3/5] arm64: consistently pass ESR_ELx to die() Mark Rutland
@ 2022-09-13 11:48   ` Mark Brown
  2022-09-14  5:52   ` Anshuman Khandual
  1 sibling, 0 replies; 10+ messages in thread
From: Mark Brown @ 2022-09-13 11:48 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel, alexandru.elisei, amit.kachhap,
	anshuman.khandual, catalin.marinas, james.morse, will


[-- Attachment #1.1: Type: text/plain, Size: 475 bytes --]

On Tue, Sep 13, 2022 at 11:17:30AM +0100, Mark Rutland wrote:
> Currently, bug_handler() and kasan_handler() call die() with '0' as the
> 'err' value, whereas die_kernel_fault() passes the ESR_ELx value.
> 
> For consistency, this patch ensures we always pass the ESR_ELx value to
> die(). As this is only called for exceptions taken from kernel mode,
> there should be no user-visible change as a result of this patch.

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/5] arm64: report EL1 UNDEFs better
  2022-09-13 10:17 ` [PATCH v2 1/5] arm64: report EL1 UNDEFs better Mark Rutland
@ 2022-09-14  5:32   ` Anshuman Khandual
  0 siblings, 0 replies; 10+ messages in thread
From: Anshuman Khandual @ 2022-09-14  5:32 UTC (permalink / raw)
  To: Mark Rutland, linux-arm-kernel
  Cc: alexandru.elisei, amit.kachhap, broonie, catalin.marinas,
	james.morse, will

On 9/13/22 15:47, Mark Rutland wrote:
> If an UNDEFINED exception is taken from EL1, and do_undefinstr() doesn't
> find any suitable undef_hook, it will call:
> 
> 	BUG_ON(!user_mode(regs))
> 
> ... and the kernel will report a failure witin do_undefinstr() rather
> than reporting the original context that the UNDEFINED exception was
> taken from. The pt_regs and ESR value reported within the BUG() handler
> will be from within do_undefinstr() and the code dump will be for the
> BRK in BUG_ON(), which isn't sufficient to debug the cause of the
> original exception.
> 
> This patch makes the reporting better by having do_undefinstr() call
> die() directly in this case to report the original context from which
> the UNDEFINED exception was taken.
> 
> Prior to this patch, an undefined instruction is reported as:
> 
> | kernel BUG at arch/arm64/kernel/traps.c:497!
> | Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> | Modules linked in:
> | CPU: 0 PID: 0 Comm: swapper Not tainted 5.19.0-rc3-00127-geff044f1b04e-dirty #3
> | Hardware name: linux,dummy-virt (DT)
> | pstate: 000000c5 (nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> | pc : do_undefinstr+0x28c/0x2ac
> | lr : do_undefinstr+0x298/0x2ac
> | sp : ffff800009f63bc0
> | x29: ffff800009f63bc0 x28: ffff800009f73c00 x27: ffff800009644a70
> | x26: ffff8000096778a8 x25: 0000000000000040 x24: 0000000000000000
> | x23: 00000000800000c5 x22: ffff800009894060 x21: ffff800009f63d90
> | x20: 0000000000000000 x19: ffff800009f63c40 x18: 0000000000000006
> | x17: 0000000000403000 x16: 00000000bfbfd000 x15: ffff800009f63830
> | x14: ffffffffffffffff x13: 0000000000000000 x12: 0000000000000019
> | x11: 0101010101010101 x10: 0000000000161b98 x9 : 0000000000000000
> | x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
> | x5 : ffff800009f761d0 x4 : 0000000000000000 x3 : ffff80000a2b80f8
> | x2 : 0000000000000000 x1 : ffff800009f73c00 x0 : 00000000800000c5
> | Call trace:
> |  do_undefinstr+0x28c/0x2ac
> |  el1_undef+0x2c/0x4c
> |  el1h_64_sync_handler+0x84/0xd0
> |  el1h_64_sync+0x64/0x68
> |  setup_arch+0x550/0x598
> |  start_kernel+0x88/0x6ac
> |  __primary_switched+0xb8/0xc0
> | Code: 17ffff95 a9425bf5 17ffffb8 a9025bf5 (d4210000)
> 
> With this patch applied, an undefined instruction is reported as:
> 
> | Internal error: Oops - Undefined instruction: 0 [#1] PREEMPT SMP
> | Modules linked in:
> | CPU: 0 PID: 0 Comm: swapper Not tainted 5.19.0-rc3-00128-gf27cfcc80e52-dirty #5
> | Hardware name: linux,dummy-virt (DT)
> | pstate: 800000c5 (Nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> | pc : setup_arch+0x550/0x598
> | lr : setup_arch+0x50c/0x598
> | sp : ffff800009f63d90
> | x29: ffff800009f63d90 x28: 0000000081000200 x27: ffff800009644a70
> | x26: ffff8000096778c8 x25: 0000000000000040 x24: 0000000000000000
> | x23: 0000000000000100 x22: ffff800009f69a58 x21: ffff80000a2b80b8
> | x20: 0000000000000000 x19: 0000000000000000 x18: 0000000000000006
> | x17: 0000000000403000 x16: 00000000bfbfd000 x15: ffff800009f63830
> | x14: ffffffffffffffff x13: 0000000000000000 x12: 0000000000000019
> | x11: 0101010101010101 x10: 0000000000161b98 x9 : 0000000000000000
> | x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
> | x5 : 0000000000000008 x4 : 0000000000000010 x3 : 0000000000000000
> | x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
> | Call trace:
> |  setup_arch+0x550/0x598
> |  start_kernel+0x88/0x6ac
> |  __primary_switched+0xb8/0xc0
> | Code: b4000080 90ffed80 912ac000 97db745f (00000000)
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Reviewed-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/kernel/traps.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index b7fed33981f7b..eac4f7a831750 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -494,7 +494,9 @@ void do_undefinstr(struct pt_regs *regs)
>  	if (call_undef_hook(regs) == 0)
>  		return;
>  
> -	BUG_ON(!user_mode(regs));
> +	if (!user_mode(regs))
> +		die("Oops - Undefined instruction", regs, 0);
> +
>  	force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc, 0);
>  }
>  NOKPROBE_SYMBOL(do_undefinstr);

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/5] arm64: consistently pass ESR_ELx to die()
  2022-09-13 10:17 ` [PATCH v2 3/5] arm64: consistently pass ESR_ELx to die() Mark Rutland
  2022-09-13 11:48   ` Mark Brown
@ 2022-09-14  5:52   ` Anshuman Khandual
  1 sibling, 0 replies; 10+ messages in thread
From: Anshuman Khandual @ 2022-09-14  5:52 UTC (permalink / raw)
  To: Mark Rutland, linux-arm-kernel
  Cc: alexandru.elisei, amit.kachhap, broonie, catalin.marinas,
	james.morse, will



On 9/13/22 15:47, Mark Rutland wrote:
> Currently, bug_handler() and kasan_handler() call die() with '0' as the
> 'err' value, whereas die_kernel_fault() passes the ESR_ELx value.
> 
> For consistency, this patch ensures we always pass the ESR_ELx value to
> die(). As this is only called for exceptions taken from kernel mode,
> there should be no user-visible change as a result of this patch.
> 
> For UNDEFINED exceptions, I've had to modify do_undefinstr() and its
> callers to pass the ESR_ELx value. In all cases the ESR_ELx value had
> already been read and was available.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Will Deacon <will@kernel.org>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  arch/arm64/include/asm/exception.h |  2 +-
>  arch/arm64/kernel/entry-common.c   | 14 +++++++-------
>  arch/arm64/kernel/traps.c          | 14 +++++++-------
>  3 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
> index d94aecff96902..28c0275666e12 100644
> --- a/arch/arm64/include/asm/exception.h
> +++ b/arch/arm64/include/asm/exception.h
> @@ -58,7 +58,7 @@ asmlinkage void call_on_irq_stack(struct pt_regs *regs,
>  asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);
>  
>  void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
> -void do_undefinstr(struct pt_regs *regs);
> +void do_undefinstr(struct pt_regs *regs, unsigned long esr);
>  void do_bti(struct pt_regs *regs);
>  void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,
>  			struct pt_regs *regs);
> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> index c75ca36b4a491..3dd17ef6d6d8b 100644
> --- a/arch/arm64/kernel/entry-common.c
> +++ b/arch/arm64/kernel/entry-common.c
> @@ -379,11 +379,11 @@ static void noinstr el1_pc(struct pt_regs *regs, unsigned long esr)
>  	exit_to_kernel_mode(regs);
>  }
>  
> -static void noinstr el1_undef(struct pt_regs *regs)
> +static void noinstr el1_undef(struct pt_regs *regs, unsigned long esr)
>  {
>  	enter_from_kernel_mode(regs);
>  	local_daif_inherit(regs);
> -	do_undefinstr(regs);
> +	do_undefinstr(regs, esr);
>  	local_daif_mask();
>  	exit_to_kernel_mode(regs);
>  }
> @@ -425,7 +425,7 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
>  		break;
>  	case ESR_ELx_EC_SYS64:
>  	case ESR_ELx_EC_UNKNOWN:
> -		el1_undef(regs);
> +		el1_undef(regs, esr);
>  		break;
>  	case ESR_ELx_EC_BREAKPT_CUR:
>  	case ESR_ELx_EC_SOFTSTP_CUR:
> @@ -582,11 +582,11 @@ static void noinstr el0_sp(struct pt_regs *regs, unsigned long esr)
>  	exit_to_user_mode(regs);
>  }
>  
> -static void noinstr el0_undef(struct pt_regs *regs)
> +static void noinstr el0_undef(struct pt_regs *regs, unsigned long esr)
>  {
>  	enter_from_user_mode(regs);
>  	local_daif_restore(DAIF_PROCCTX);
> -	do_undefinstr(regs);
> +	do_undefinstr(regs, esr);
>  	exit_to_user_mode(regs);
>  }
>  
> @@ -670,7 +670,7 @@ asmlinkage void noinstr el0t_64_sync_handler(struct pt_regs *regs)
>  		el0_pc(regs, esr);
>  		break;
>  	case ESR_ELx_EC_UNKNOWN:
> -		el0_undef(regs);
> +		el0_undef(regs, esr);
>  		break;
>  	case ESR_ELx_EC_BTI:
>  		el0_bti(regs);
> @@ -788,7 +788,7 @@ asmlinkage void noinstr el0t_32_sync_handler(struct pt_regs *regs)
>  	case ESR_ELx_EC_CP14_MR:
>  	case ESR_ELx_EC_CP14_LS:
>  	case ESR_ELx_EC_CP14_64:
> -		el0_undef(regs);
> +		el0_undef(regs, esr);
>  		break;
>  	case ESR_ELx_EC_CP15_32:
>  	case ESR_ELx_EC_CP15_64:
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index da8ffef6f7c58..05999cde870a9 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -485,7 +485,7 @@ void arm64_notify_segfault(unsigned long addr)
>  	force_signal_inject(SIGSEGV, code, addr, 0);
>  }
>  
> -void do_undefinstr(struct pt_regs *regs)
> +void do_undefinstr(struct pt_regs *regs, unsigned long esr)
>  {
>  	/* check for AArch32 breakpoint instructions */
>  	if (!aarch32_break_handler(regs))
> @@ -495,7 +495,7 @@ void do_undefinstr(struct pt_regs *regs)
>  		return;
>  
>  	if (!user_mode(regs))
> -		die("Oops - Undefined instruction", regs, 0);
> +		die("Oops - Undefined instruction", regs, esr);
>  
>  	force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc, 0);
>  }
> @@ -760,7 +760,7 @@ void do_cp15instr(unsigned long esr, struct pt_regs *regs)
>  		hook_base = cp15_64_hooks;
>  		break;
>  	default:
> -		do_undefinstr(regs);
> +		do_undefinstr(regs, esr);
>  		return;
>  	}
>  
> @@ -775,7 +775,7 @@ void do_cp15instr(unsigned long esr, struct pt_regs *regs)
>  	 * EL0. Fall back to our usual undefined instruction handler
>  	 * so that we handle these consistently.
>  	 */
> -	do_undefinstr(regs);
> +	do_undefinstr(regs, esr);
>  }
>  NOKPROBE_SYMBOL(do_cp15instr);
>  #endif
> @@ -795,7 +795,7 @@ void do_sysinstr(unsigned long esr, struct pt_regs *regs)
>  	 * back to our usual undefined instruction handler so that we handle
>  	 * these consistently.
>  	 */
> -	do_undefinstr(regs);
> +	do_undefinstr(regs, esr);
>  }
>  NOKPROBE_SYMBOL(do_sysinstr);
>  
> @@ -972,7 +972,7 @@ static int bug_handler(struct pt_regs *regs, unsigned long esr)
>  {
>  	switch (report_bug(regs->pc, regs)) {
>  	case BUG_TRAP_TYPE_BUG:
> -		die("Oops - BUG", regs, 0);
> +		die("Oops - BUG", regs, esr);
>  		break;
>  
>  	case BUG_TRAP_TYPE_WARN:
> @@ -1040,7 +1040,7 @@ static int kasan_handler(struct pt_regs *regs, unsigned long esr)
>  	 * This is something that might be fixed at some point in the future.
>  	 */
>  	if (!recover)
> -		die("Oops - KASAN", regs, 0);
> +		die("Oops - KASAN", regs, esr);
>  
>  	/* If thread survives, skip over the brk instruction and continue: */
>  	arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/5] arm64: report EL1 exceptions better
  2022-09-13 10:17 [PATCH v2 0/5] arm64: report EL1 exceptions better Mark Rutland
                   ` (4 preceding siblings ...)
  2022-09-13 10:17 ` [PATCH v2 5/5] arm64: rework BTI " Mark Rutland
@ 2022-09-16 17:46 ` Catalin Marinas
  5 siblings, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2022-09-16 17:46 UTC (permalink / raw)
  To: linux-arm-kernel, Mark Rutland
  Cc: Will Deacon, alexandru.elisei, anshuman.khandual, james.morse,
	amit.kachhap, broonie

On Tue, 13 Sep 2022 11:17:27 +0100, Mark Rutland wrote:
> We treat BTI, FPAC, and UNDEFINED exceptions as fatal when taken from
> EL1, but due to the way we make these fatal, we end up throwing away
> information that could be used to diagnose the cause of the exception.
> 
> We have an anti-pattern where in the handler for the exception we do:
> 
> 	BUG_ON(!user_mode(regs));
> 
> [...]

Applied to arm64 (for-next/el1-exceptions), thanks!

[1/5] arm64: report EL1 UNDEFs better
      https://git.kernel.org/arm64/c/b502c87d2a26
[2/5] arm64: die(): pass 'err' as long
      https://git.kernel.org/arm64/c/18906ff9af65
[3/5] arm64: consistently pass ESR_ELx to die()
      https://git.kernel.org/arm64/c/0f2cb928a154
[4/5] arm64: rework FPAC exception handling
      https://git.kernel.org/arm64/c/a1fafa3b24a7
[5/5] arm64: rework BTI exception handling
      https://git.kernel.org/arm64/c/830a2a4d853f

-- 
Catalin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-09-16 17:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-13 10:17 [PATCH v2 0/5] arm64: report EL1 exceptions better Mark Rutland
2022-09-13 10:17 ` [PATCH v2 1/5] arm64: report EL1 UNDEFs better Mark Rutland
2022-09-14  5:32   ` Anshuman Khandual
2022-09-13 10:17 ` [PATCH v2 2/5] arm64: die(): pass 'err' as long Mark Rutland
2022-09-13 10:17 ` [PATCH v2 3/5] arm64: consistently pass ESR_ELx to die() Mark Rutland
2022-09-13 11:48   ` Mark Brown
2022-09-14  5:52   ` Anshuman Khandual
2022-09-13 10:17 ` [PATCH v2 4/5] arm64: rework FPAC exception handling Mark Rutland
2022-09-13 10:17 ` [PATCH v2 5/5] arm64: rework BTI " Mark Rutland
2022-09-16 17:46 ` [PATCH v2 0/5] arm64: report EL1 exceptions better Catalin Marinas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).