diff mbox series

arm64: traps: attempt to dump all instructions

Message ID 20230127121256.2141368-1-mark.rutland@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: traps: attempt to dump all instructions | expand

Commit Message

Mark Rutland Jan. 27, 2023, 12:12 p.m. UTC
Currently dump_kernel_instr() dumps a few instructions around the
pt_regs::pc value, dumping 4 instructions before the PC before dumping
the instruction at the PC. If an attempt to read an instruction fails,
it gives up and does not attempt to dump any subsequent instructions.

This is unfortunate when the pt_regs::pc value points to the start of a
page with a leading guard page, where the instruction at the PC can be
read, but prior instructions cannot.

This patch makes dump_kernel_instr() attempt to dump each instruction
regardless of whether reading a prior instruction could be read, which
gives a more useful code dump in such cases. When an instruction cannot
be read, it is reported as "????????", which cannot be confused with a
hex value,

For example, with a `UDF #0` (AKA 0x00000000) early in the kexec control
page, we'll now get the following code dump:

| Internal error: Oops - Undefined instruction: 0000000002000000 [#1] SMP
| Modules linked in:
| CPU: 0 PID: 261 Comm: kexec Not tainted 6.2.0-rc5+ #26
| Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
| pstate: 604003c5 (nZCv DAIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : 0x48c00000
| lr : machine_kexec+0x190/0x200
| sp : ffff80000d36ba80
| x29: ffff80000d36ba80 x28: ffff000002dfc380 x27: 0000000000000000
| x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
| x23: ffff80000a9f7858 x22: 000000004c460000 x21: 0000000000000010
| x20: 00000000ad821000 x19: ffff000000aa0000 x18: 0000000000000006
| x17: ffff8000758a2000 x16: ffff800008000000 x15: ffff80000d36b568
| x14: 0000000000000000 x13: ffff80000d36b707 x12: ffff80000a9bf6e0
| x11: 00000000ffffdfff x10: ffff80000aaaf8e0 x9 : ffff80000815eff8
| x8 : 000000000002ffe8 x7 : c0000000ffffdfff x6 : 00000000000affa8
| x5 : 0000000000001fff x4 : 0000000000000001 x3 : ffff80000a263008
| x2 : ffff80000a9e20f8 x1 : 0000000048c00000 x0 : ffff000000aa0000
| Call trace:
|  0x48c00000
|  kernel_kexec+0x88/0x138
|  __do_sys_reboot+0x108/0x288
|  __arm64_sys_reboot+0x2c/0x40
|  invoke_syscall+0x78/0x140
|  el0_svc_common.constprop.0+0x4c/0x100
|  do_el0_svc+0x34/0x80
|  el0_svc+0x34/0x140
|  el0t_64_sync_handler+0xf4/0x140
|  el0t_64_sync+0x194/0x1c0
| Code: ???????? ???????? ???????? ???????? (00000000)
| ---[ end trace 0000000000000000 ]---
| Kernel panic - not syncing: Oops - Undefined instruction: Fatal exception
| Kernel Offset: disabled
| CPU features: 0x002000,00050108,c8004203
| Memory Limit: none

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
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 | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Ard Biesheuvel Jan. 27, 2023, 12:30 p.m. UTC | #1
On Fri, 27 Jan 2023 at 13:13, Mark Rutland <mark.rutland@arm.com> wrote:
>
> Currently dump_kernel_instr() dumps a few instructions around the
> pt_regs::pc value, dumping 4 instructions before the PC before dumping
> the instruction at the PC. If an attempt to read an instruction fails,
> it gives up and does not attempt to dump any subsequent instructions.
>
> This is unfortunate when the pt_regs::pc value points to the start of a
> page with a leading guard page, where the instruction at the PC can be
> read, but prior instructions cannot.
>
> This patch makes dump_kernel_instr() attempt to dump each instruction
> regardless of whether reading a prior instruction could be read, which
> gives a more useful code dump in such cases. When an instruction cannot
> be read, it is reported as "????????", which cannot be confused with a
> hex value,
>
> For example, with a `UDF #0` (AKA 0x00000000) early in the kexec control
> page, we'll now get the following code dump:
>
> | Internal error: Oops - Undefined instruction: 0000000002000000 [#1] SMP
> | Modules linked in:
> | CPU: 0 PID: 261 Comm: kexec Not tainted 6.2.0-rc5+ #26
> | Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
> | pstate: 604003c5 (nZCv DAIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> | pc : 0x48c00000
> | lr : machine_kexec+0x190/0x200
> | sp : ffff80000d36ba80
> | x29: ffff80000d36ba80 x28: ffff000002dfc380 x27: 0000000000000000
> | x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
> | x23: ffff80000a9f7858 x22: 000000004c460000 x21: 0000000000000010
> | x20: 00000000ad821000 x19: ffff000000aa0000 x18: 0000000000000006
> | x17: ffff8000758a2000 x16: ffff800008000000 x15: ffff80000d36b568
> | x14: 0000000000000000 x13: ffff80000d36b707 x12: ffff80000a9bf6e0
> | x11: 00000000ffffdfff x10: ffff80000aaaf8e0 x9 : ffff80000815eff8
> | x8 : 000000000002ffe8 x7 : c0000000ffffdfff x6 : 00000000000affa8
> | x5 : 0000000000001fff x4 : 0000000000000001 x3 : ffff80000a263008
> | x2 : ffff80000a9e20f8 x1 : 0000000048c00000 x0 : ffff000000aa0000
> | Call trace:
> |  0x48c00000
> |  kernel_kexec+0x88/0x138
> |  __do_sys_reboot+0x108/0x288
> |  __arm64_sys_reboot+0x2c/0x40
> |  invoke_syscall+0x78/0x140
> |  el0_svc_common.constprop.0+0x4c/0x100
> |  do_el0_svc+0x34/0x80
> |  el0_svc+0x34/0x140
> |  el0t_64_sync_handler+0xf4/0x140
> |  el0t_64_sync+0x194/0x1c0
> | Code: ???????? ???????? ???????? ???????? (00000000)
> | ---[ end trace 0000000000000000 ]---
> | Kernel panic - not syncing: Oops - Undefined instruction: Fatal exception
> | Kernel Offset: disabled
> | CPU features: 0x002000,00050108,c8004203
> | Memory Limit: none
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Will Deacon <will@kernel.org>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  arch/arm64/kernel/traps.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 4c0caa589e12d..0ccc063daccb8 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -162,10 +162,8 @@ static void dump_kernel_instr(const char *lvl, struct pt_regs *regs)
>
>                 if (!bad)
>                         p += sprintf(p, i == 0 ? "(%08x) " : "%08x ", val);
> -               else {
> -                       p += sprintf(p, "bad PC value");
> -                       break;
> -               }
> +               else
> +                       p += sprintf(p, i == 0 ? "(????????) " : "???????? ");
>         }
>
>         printk("%sCode: %s\n", lvl, str);
> --
> 2.30.2
>
Catalin Marinas Jan. 27, 2023, 5:51 p.m. UTC | #2
On Fri, 27 Jan 2023 12:12:56 +0000, Mark Rutland wrote:
> Currently dump_kernel_instr() dumps a few instructions around the
> pt_regs::pc value, dumping 4 instructions before the PC before dumping
> the instruction at the PC. If an attempt to read an instruction fails,
> it gives up and does not attempt to dump any subsequent instructions.
> 
> This is unfortunate when the pt_regs::pc value points to the start of a
> page with a leading guard page, where the instruction at the PC can be
> read, but prior instructions cannot.
> 
> [...]

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

[1/1] arm64: traps: attempt to dump all instructions
      https://git.kernel.org/arm64/c/a873bb493fb1
diff mbox series

Patch

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 4c0caa589e12d..0ccc063daccb8 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -162,10 +162,8 @@  static void dump_kernel_instr(const char *lvl, struct pt_regs *regs)
 
 		if (!bad)
 			p += sprintf(p, i == 0 ? "(%08x) " : "%08x ", val);
-		else {
-			p += sprintf(p, "bad PC value");
-			break;
-		}
+		else
+			p += sprintf(p, i == 0 ? "(????????) " : "???????? ");
 	}
 
 	printk("%sCode: %s\n", lvl, str);