Skip to contentSkip to conversation

parisc: Avoid kernel panic triggered by invalid kprobe

PATCH In review Helge Deller 2 participants 3 messages Jul 16, 2019 PA-RISC architecture

lore
Share & tools
lore

Helge Deller

author maintainer
#0 lore raw

PATCH 1/1 parisc: Avoid kernel panic triggered by invalid kprobe

When running gdb I was able to trigger this kernel panic:

Kernel Fault: Code=26 (Data memory access rights trap) at addr 0000000000000060

 CPU: 0 PID: 1401 Comm: gdb-crash Not tainted 5.2.0-rc7-64bit+ #1053

YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
PSW: 00001000000001000000000000001111 Not tainted

 r00-03  000000000804000f 0000000040dee1a0 0000000040c78cf0 00000000b8d50160
 r04-07  0000000040d2b1a0 000000004360a098 00000000bbbe87b8 0000000000000003
 r08-11  00000000fac20a70 00000000fac24160 00000000fac1bbe0 0000000000000000
 r12-15  00000000fabfb79a 00000000fac244a4 0000000000010000 0000000000000001
 r16-19  00000000bbbe87b8 00000000f8f02910 0000000000010034 0000000000000000
 r20-23  00000000fac24630 00000000fac24630 000000006474e552 00000000fac1aa52
 r24-27  0000000000000028 00000000bbbe87b8 00000000bbbe87b8 0000000040d2b1a0
 r28-31  0000000000000000 00000000b8d501c0 00000000b8d501f0 0000000003424000

sr00-03 0000000000423000 0000000000000000 0000000000000000 0000000000423000 sr04-07 0000000000000000 0000000000000000 0000000000000000 0000000000000000

IASQ: 0000000000000000 0000000000000000 IAOQ: 0000000040c78cf0 0000000040c78cf4
IIR: 539f00c0 ISR: 0000000000000000 IOR: 0000000000000060
CPU: 0 CR30: 00000000b8d50000 CR31: 00000000d22345e2
ORIG_R28: 0000000040250798
IAOQ[0]: parisc_kprobe_ss_handler+0x58/0x170
IAOQ[1]: parisc_kprobe_ss_handler+0x5c/0x170
RP(r2): parisc_kprobe_ss_handler+0x58/0x170

 Backtrace:
  [<0000000040206ff8>] handle_interruption+0x178/0xbb8
 Kernel panic - not syncing: Kernel Fault

Avoid this panic by checking the return value of kprobe_running() and skip kprobe if none is currently active.

Signed-off-by Helge Deller <[email protected]>
Cc <[email protected]> # v5.2
arch/parisc/kernel/kprobes.c +3 -0
index d58960b33bda..0385a8fd74aa 100644
@@ -133,6 +133,9 @@ int __kprobes parisc_kprobe_ss_handler(struct pt_regs *regs)
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 	struct kprobe *p = kprobe_running();
 
+	if (!p)
+		return 0;
+
 	if (regs->iaoq[0] != (unsigned long)p->ainsn.insn+4)
 		return 0;
Acked-by on v1 1/1
#1 lore raw

Hi Helge,

Quoted 14 lines · Helge Deller

When running gdb I was able to trigger this kernel panic:
[OOps]

Avoid this panic by checking the return value of kprobe_running() and skip kprobe if none is currently active.

Signed-off-by Helge Deller <[email protected]>
Cc <[email protected]> # v5.2
arch/parisc/kernel/kprobes.c +0 -0
index d58960b33bda..0385a8fd74aa 100644

arch/parisc/kernel/kprobes.c int __kprobes parisc_kprobe_ss_handler(struct pt_regs *regs) · line 141

Quoted 9 lines
arch/parisc/kernel/kprobes.c +3 -0
@@ -133,6 +133,9 @@ int __kprobes parisc_kprobe_ss_handler(struct pt_regs *regs)
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 	struct kprobe *p = kprobe_running();
 
+	if (!p)
+		return 0;
+
 	if (regs->iaoq[0] != (unsigned long)p->ainsn.insn+4)
 		return 0;

Looks ok to me. I assume this happened during single-stepping?

Acked-by Sven Schnelle <[email protected]>
author maintainer
#2 lore raw
Quoted 20 lines · Sven Schnelle

Hi Helge,

Quoted 17 lines · Helge Deller

When running gdb I was able to trigger this kernel panic:
[OOps]

Avoid this panic by checking the return value of kprobe_running() and skip kprobe if none is currently active.

Signed-off-by Helge Deller <[email protected]>
Cc <[email protected]> # v5.2
arch/parisc/kernel/kprobes.c +0 -0
index d58960b33bda..0385a8fd74aa 100644
@@ -133,6 +133,9 @@ int __kprobes parisc_kprobe_ss_handler(struct pt_regs *regs)
  	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
  	struct kprobe *p = kprobe_running();
Quoted 8 lines

arch/parisc/kernel/kprobes.c int __kprobes parisc_kprobe_ss_handler(struct pt_regs *regs) · line 141

Quoted 6 lines
arch/parisc/kernel/kprobes.c +3 -0
+	if (!p)
+		return 0;
+
  	if (regs->iaoq[0] != (unsigned long)p->ainsn.insn+4)
  		return 0;

Looks ok to me. I assume this happened during single-stepping?

Yes.
Can be reproduced with the testcase in this bug report:
https://bugs.gentoo.org/481768

Quoted 1 line
Acked-by Sven Schnelle <[email protected]>

Thanks!
Helge