* [PATCH] powerpc/kprobes: Fix null pointer reference in arch_prepare_kprobe()
@ 2022-09-23 9:32 Li Huafei
2022-09-30 9:47 ` Naveen N. Rao
2022-10-04 13:26 ` Michael Ellerman
0 siblings, 2 replies; 5+ messages in thread
From: Li Huafei @ 2022-09-23 9:32 UTC (permalink / raw)
To: mpe, jniethe5
Cc: lihuafei1, peterz, npiggin, linux-kernel, rostedt, naveen.n.rao,
linuxppc-dev, mhiramat
I found a null pointer reference in arch_prepare_kprobe():
# echo 'p cmdline_proc_show' > kprobe_events
# echo 'p cmdline_proc_show+16' >> kprobe_events
[ 67.278533][ T122] Kernel attempted to read user page (0) - exploit attempt? (uid: 0)
[ 67.279326][ T122] BUG: Kernel NULL pointer dereference on read at 0x00000000
[ 67.279738][ T122] Faulting instruction address: 0xc000000000050bfc
[ 67.280486][ T122] Oops: Kernel access of bad area, sig: 11 [#1]
[ 67.280846][ T122] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
[ 67.281435][ T122] Modules linked in:
[ 67.281903][ T122] CPU: 0 PID: 122 Comm: sh Not tainted 6.0.0-rc3-00007-gdcf8e5633e2e #10
[ 67.282547][ T122] NIP: c000000000050bfc LR: c000000000050bec CTR: 0000000000005bdc
[ 67.282920][ T122] REGS: c0000000348475b0 TRAP: 0300 Not tainted (6.0.0-rc3-00007-gdcf8e5633e2e)
[ 67.283424][ T122] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 88002444 XER: 20040006
[ 67.284023][ T122] CFAR: c00000000022d100 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
[ 67.284023][ T122] GPR00: c000000000050bec c000000034847850 c0000000013f6100 c000000001fb7718
[ 67.284023][ T122] GPR04: c000000000515c10 c000000000e5fe08 c00000000133da60 c000000004839300
[ 67.284023][ T122] GPR08: c0000000014ffb98 0000000000000000 c000000000515c0c c000000000e18576
[ 67.284023][ T122] GPR12: c000000000e60170 c0000000015a0000 00000001155e0460 0000000000000000
[ 67.284023][ T122] GPR16: 0000000000000000 00007fffe8eeb3c8 0000000116320728 0000000000000000
[ 67.284023][ T122] GPR20: 0000000116320720 0000000000000000 c0000000012fa918 0000000000000006
[ 67.284023][ T122] GPR24: c0000000014ffb98 c0000000011ed360 0000000000000000 c000000001fb7928
[ 67.284023][ T122] GPR28: 0000000000000000 0000000000000000 000000007c0802a6 c000000001fb7918
[ 67.287799][ T122] NIP [c000000000050bfc] arch_prepare_kprobe+0x10c/0x2d0
[ 67.288490][ T122] LR [c000000000050bec] arch_prepare_kprobe+0xfc/0x2d0
[ 67.289025][ T122] Call Trace:
[ 67.289268][ T122] [c000000034847850] [c0000000012f77a0] 0xc0000000012f77a0 (unreliable)
[ 67.289999][ T122] [c0000000348478d0] [c000000000231320] register_kprobe+0x3c0/0x7a0
[ 67.290439][ T122] [c000000034847940] [c0000000002938c0] __register_trace_kprobe+0x140/0x1a0
[ 67.290898][ T122] [c0000000348479b0] [c0000000002944c4] __trace_kprobe_create+0x794/0x1040
[ 67.291330][ T122] [c000000034847b60] [c0000000002a1614] trace_probe_create+0xc4/0xe0
[ 67.291717][ T122] [c000000034847bb0] [c00000000029363c] create_or_delete_trace_kprobe+0x2c/0x80
[ 67.292158][ T122] [c000000034847bd0] [c000000000264420] trace_parse_run_command+0xf0/0x210
[ 67.292611][ T122] [c000000034847c70] [c0000000002934a0] probes_write+0x20/0x40
[ 67.292996][ T122] [c000000034847c90] [c00000000045e98c] vfs_write+0xfc/0x450
[ 67.293356][ T122] [c000000034847d50] [c00000000045eec4] ksys_write+0x84/0x140
[ 67.293716][ T122] [c000000034847da0] [c00000000002e4fc] system_call_exception+0x17c/0x3a0
[ 67.294186][ T122] [c000000034847e10] [c00000000000c0e8] system_call_vectored_common+0xe8/0x278
[ 67.294680][ T122] --- interrupt: 3000 at 0x7fffa5682de0
[ 67.294937][ T122] NIP: 00007fffa5682de0 LR: 0000000000000000 CTR: 0000000000000000
[ 67.295313][ T122] REGS: c000000034847e80 TRAP: 3000 Not tainted (6.0.0-rc3-00007-gdcf8e5633e2e)
[ 67.295725][ T122] MSR: 900000000280f033 <SF,HV,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 44002408 XER: 00000000
[ 67.296291][ T122] IRQMASK: 0
[ 67.296291][ T122] GPR00: 0000000000000004 00007fffe8eeaec0 00007fffa5757300 0000000000000001
[ 67.296291][ T122] GPR04: 0000000116329c60 0000000000000017 0000000000116329 0000000000000000
[ 67.296291][ T122] GPR08: 0000000000000006 0000000000000000 0000000000000000 0000000000000000
[ 67.296291][ T122] GPR12: 0000000000000000 00007fffa580ac60 00000001155e0460 0000000000000000
[ 67.296291][ T122] GPR16: 0000000000000000 00007fffe8eeb3c8 0000000116320728 0000000000000000
[ 67.296291][ T122] GPR20: 0000000116320720 0000000000000000 0000000000000000 0000000000000002
[ 67.296291][ T122] GPR24: 00000001163206f0 0000000000000020 00007fffe8eeafa0 0000000000000001
[ 67.296291][ T122] GPR28: 0000000000000000 0000000000000017 0000000116329c60 0000000000000001
[ 67.299570][ T122] NIP [00007fffa5682de0] 0x7fffa5682de0
[ 67.299837][ T122] LR [0000000000000000] 0x0
[ 67.300072][ T122] --- interrupt: 3000
[ 67.300447][ T122] Instruction dump:
[ 67.300736][ T122] 386319d8 481342f5 60000000 60000000 60000000 e87f0028 3863fffc 481dc4d1
[ 67.301230][ T122] 60000000 2c230000 41820018 e9230058 <81290000> 552936be 2c090001 4182018c
[ 67.302102][ T122] ---[ end trace 0000000000000000 ]---
[ 67.302496][ T122]
The address being probed has some special:
cmdline_proc_show: Probe based on ftrace
cmdline_proc_show+16: Probe for the next instruction at the ftrace location
The ftrace-based kprobe does not generate kprobe::ainsn::insn, it gets
set to NULL. In arch_prepare_kprobe() it will check for:
...
prev = get_kprobe(p->addr - 1);
preempt_enable_no_resched();
if (prev && ppc_inst_prefixed(ppc_inst_read(prev->ainsn.insn))) {
...
If prev is based on ftrace, 'ppc_inst_read(prev->ainsn.insn)' will occur
with a null pointer reference. At this point prev->addr will not be a
prefixed instruction, so the check can be skipped.
Check if prev is ftrace-based kprobe before reading 'prev->ainsn.insn'
to fix this problem.
Fixes: b4657f7650ba ("powerpc/kprobes: Don't allow breakpoints on suffixes")
Signed-off-by: Li Huafei <lihuafei1@huawei.com>
---
arch/powerpc/kernel/kprobes.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 912d4f8a13be..9f6cbbd56809 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -161,7 +161,12 @@ int arch_prepare_kprobe(struct kprobe *p)
preempt_disable();
prev = get_kprobe(p->addr - 1);
preempt_enable_no_resched();
- if (prev && ppc_inst_prefixed(ppc_inst_read(prev->ainsn.insn))) {
+ /*
+ * When prev is a ftrace-based kprobe, we don't have an insn, and it
+ * doesn't probe for prefixed instruction.
+ */
+ if (prev && !kprobe_ftrace(prev) &&
+ ppc_inst_prefixed(ppc_inst_read(prev->ainsn.insn))) {
printk("Cannot register a kprobe on the second word of prefixed instruction\n");
ret = -EINVAL;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc/kprobes: Fix null pointer reference in arch_prepare_kprobe()
2022-09-23 9:32 [PATCH] powerpc/kprobes: Fix null pointer reference in arch_prepare_kprobe() Li Huafei
@ 2022-09-30 9:47 ` Naveen N. Rao
2022-10-12 6:14 ` Li Huafei
2022-10-04 13:26 ` Michael Ellerman
1 sibling, 1 reply; 5+ messages in thread
From: Naveen N. Rao @ 2022-09-30 9:47 UTC (permalink / raw)
To: jniethe5, Li Huafei, mpe
Cc: peterz, linux-kernel, npiggin, rostedt, linuxppc-dev, mhiramat
Li Huafei wrote:
> I found a null pointer reference in arch_prepare_kprobe():
Good find!
>
> # echo 'p cmdline_proc_show' > kprobe_events
> # echo 'p cmdline_proc_show+16' >> kprobe_events
I think we should extend multiple_kprobes selftest to also place
contiguous probes to catch such errors.
> [ 67.278533][ T122] Kernel attempted to read user page (0) - exploit attempt? (uid: 0)
> [ 67.279326][ T122] BUG: Kernel NULL pointer dereference on read at 0x00000000
> [ 67.279738][ T122] Faulting instruction address: 0xc000000000050bfc
> [ 67.280486][ T122] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 67.280846][ T122] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
> [ 67.281435][ T122] Modules linked in:
> [ 67.281903][ T122] CPU: 0 PID: 122 Comm: sh Not tainted 6.0.0-rc3-00007-gdcf8e5633e2e #10
> [ 67.282547][ T122] NIP: c000000000050bfc LR: c000000000050bec CTR: 0000000000005bdc
> [ 67.282920][ T122] REGS: c0000000348475b0 TRAP: 0300 Not tainted (6.0.0-rc3-00007-gdcf8e5633e2e)
> [ 67.283424][ T122] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 88002444 XER: 20040006
> [ 67.284023][ T122] CFAR: c00000000022d100 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
> [ 67.284023][ T122] GPR00: c000000000050bec c000000034847850 c0000000013f6100 c000000001fb7718
> [ 67.284023][ T122] GPR04: c000000000515c10 c000000000e5fe08 c00000000133da60 c000000004839300
> [ 67.284023][ T122] GPR08: c0000000014ffb98 0000000000000000 c000000000515c0c c000000000e18576
> [ 67.284023][ T122] GPR12: c000000000e60170 c0000000015a0000 00000001155e0460 0000000000000000
> [ 67.284023][ T122] GPR16: 0000000000000000 00007fffe8eeb3c8 0000000116320728 0000000000000000
> [ 67.284023][ T122] GPR20: 0000000116320720 0000000000000000 c0000000012fa918 0000000000000006
> [ 67.284023][ T122] GPR24: c0000000014ffb98 c0000000011ed360 0000000000000000 c000000001fb7928
> [ 67.284023][ T122] GPR28: 0000000000000000 0000000000000000 000000007c0802a6 c000000001fb7918
> [ 67.287799][ T122] NIP [c000000000050bfc] arch_prepare_kprobe+0x10c/0x2d0
> [ 67.288490][ T122] LR [c000000000050bec] arch_prepare_kprobe+0xfc/0x2d0
> [ 67.289025][ T122] Call Trace:
> [ 67.289268][ T122] [c000000034847850] [c0000000012f77a0] 0xc0000000012f77a0 (unreliable)
> [ 67.289999][ T122] [c0000000348478d0] [c000000000231320] register_kprobe+0x3c0/0x7a0
> [ 67.290439][ T122] [c000000034847940] [c0000000002938c0] __register_trace_kprobe+0x140/0x1a0
> [ 67.290898][ T122] [c0000000348479b0] [c0000000002944c4] __trace_kprobe_create+0x794/0x1040
> [ 67.291330][ T122] [c000000034847b60] [c0000000002a1614] trace_probe_create+0xc4/0xe0
> [ 67.291717][ T122] [c000000034847bb0] [c00000000029363c] create_or_delete_trace_kprobe+0x2c/0x80
> [ 67.292158][ T122] [c000000034847bd0] [c000000000264420] trace_parse_run_command+0xf0/0x210
> [ 67.292611][ T122] [c000000034847c70] [c0000000002934a0] probes_write+0x20/0x40
> [ 67.292996][ T122] [c000000034847c90] [c00000000045e98c] vfs_write+0xfc/0x450
> [ 67.293356][ T122] [c000000034847d50] [c00000000045eec4] ksys_write+0x84/0x140
> [ 67.293716][ T122] [c000000034847da0] [c00000000002e4fc] system_call_exception+0x17c/0x3a0
> [ 67.294186][ T122] [c000000034847e10] [c00000000000c0e8] system_call_vectored_common+0xe8/0x278
> [ 67.294680][ T122] --- interrupt: 3000 at 0x7fffa5682de0
> [ 67.294937][ T122] NIP: 00007fffa5682de0 LR: 0000000000000000 CTR: 0000000000000000
> [ 67.295313][ T122] REGS: c000000034847e80 TRAP: 3000 Not tainted (6.0.0-rc3-00007-gdcf8e5633e2e)
> [ 67.295725][ T122] MSR: 900000000280f033 <SF,HV,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 44002408 XER: 00000000
> [ 67.296291][ T122] IRQMASK: 0
> [ 67.296291][ T122] GPR00: 0000000000000004 00007fffe8eeaec0 00007fffa5757300 0000000000000001
> [ 67.296291][ T122] GPR04: 0000000116329c60 0000000000000017 0000000000116329 0000000000000000
> [ 67.296291][ T122] GPR08: 0000000000000006 0000000000000000 0000000000000000 0000000000000000
> [ 67.296291][ T122] GPR12: 0000000000000000 00007fffa580ac60 00000001155e0460 0000000000000000
> [ 67.296291][ T122] GPR16: 0000000000000000 00007fffe8eeb3c8 0000000116320728 0000000000000000
> [ 67.296291][ T122] GPR20: 0000000116320720 0000000000000000 0000000000000000 0000000000000002
> [ 67.296291][ T122] GPR24: 00000001163206f0 0000000000000020 00007fffe8eeafa0 0000000000000001
> [ 67.296291][ T122] GPR28: 0000000000000000 0000000000000017 0000000116329c60 0000000000000001
> [ 67.299570][ T122] NIP [00007fffa5682de0] 0x7fffa5682de0
> [ 67.299837][ T122] LR [0000000000000000] 0x0
> [ 67.300072][ T122] --- interrupt: 3000
> [ 67.300447][ T122] Instruction dump:
> [ 67.300736][ T122] 386319d8 481342f5 60000000 60000000 60000000 e87f0028 3863fffc 481dc4d1
> [ 67.301230][ T122] 60000000 2c230000 41820018 e9230058 <81290000> 552936be 2c090001 4182018c
> [ 67.302102][ T122] ---[ end trace 0000000000000000 ]---
> [ 67.302496][ T122]
Please consider trimming the backtrace to only capture the necessary
information:
https://docs.kernel.org/process/submitting-patches.html#backtraces-in-commit-mesages
>
> The address being probed has some special:
>
> cmdline_proc_show: Probe based on ftrace
> cmdline_proc_show+16: Probe for the next instruction at the ftrace location
>
> The ftrace-based kprobe does not generate kprobe::ainsn::insn, it gets
> set to NULL. In arch_prepare_kprobe() it will check for:
>
> ...
> prev = get_kprobe(p->addr - 1);
> preempt_enable_no_resched();
> if (prev && ppc_inst_prefixed(ppc_inst_read(prev->ainsn.insn))) {
> ...
>
> If prev is based on ftrace, 'ppc_inst_read(prev->ainsn.insn)' will occur
> with a null pointer reference. At this point prev->addr will not be a
> prefixed instruction, so the check can be skipped.
>
> Check if prev is ftrace-based kprobe before reading 'prev->ainsn.insn'
> to fix this problem.
>
> Fixes: b4657f7650ba ("powerpc/kprobes: Don't allow breakpoints on suffixes")
> Signed-off-by: Li Huafei <lihuafei1@huawei.com>
> ---
> arch/powerpc/kernel/kprobes.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 912d4f8a13be..9f6cbbd56809 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -161,7 +161,12 @@ int arch_prepare_kprobe(struct kprobe *p)
> preempt_disable();
> prev = get_kprobe(p->addr - 1);
> preempt_enable_no_resched();
> - if (prev && ppc_inst_prefixed(ppc_inst_read(prev->ainsn.insn))) {
> + /*
> + * When prev is a ftrace-based kprobe, we don't have an insn, and it
> + * doesn't probe for prefixed instruction.
> + */
> + if (prev && !kprobe_ftrace(prev) &&
> + ppc_inst_prefixed(ppc_inst_read(prev->ainsn.insn))) {
> printk("Cannot register a kprobe on the second word of prefixed instruction\n");
> ret = -EINVAL;
> }
It's fine to keep the if condition on a single line.
Other than that, thanks for the fix!
Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
- Naveen
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc/kprobes: Fix null pointer reference in arch_prepare_kprobe()
2022-09-23 9:32 [PATCH] powerpc/kprobes: Fix null pointer reference in arch_prepare_kprobe() Li Huafei
2022-09-30 9:47 ` Naveen N. Rao
@ 2022-10-04 13:26 ` Michael Ellerman
1 sibling, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2022-10-04 13:26 UTC (permalink / raw)
To: jniethe5, Li Huafei, mpe
Cc: peterz, npiggin, linux-kernel, rostedt, mhiramat, naveen.n.rao,
linuxppc-dev
On Fri, 23 Sep 2022 17:32:53 +0800, Li Huafei wrote:
> I found a null pointer reference in arch_prepare_kprobe():
>
> # echo 'p cmdline_proc_show' > kprobe_events
> # echo 'p cmdline_proc_show+16' >> kprobe_events
> [ 67.278533][ T122] Kernel attempted to read user page (0) - exploit attempt? (uid: 0)
> [ 67.279326][ T122] BUG: Kernel NULL pointer dereference on read at 0x00000000
> [ 67.279738][ T122] Faulting instruction address: 0xc000000000050bfc
> [ 67.280486][ T122] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 67.280846][ T122] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
> [ 67.281435][ T122] Modules linked in:
> [ 67.281903][ T122] CPU: 0 PID: 122 Comm: sh Not tainted 6.0.0-rc3-00007-gdcf8e5633e2e #10
> [ 67.282547][ T122] NIP: c000000000050bfc LR: c000000000050bec CTR: 0000000000005bdc
> [ 67.282920][ T122] REGS: c0000000348475b0 TRAP: 0300 Not tainted (6.0.0-rc3-00007-gdcf8e5633e2e)
> [ 67.283424][ T122] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 88002444 XER: 20040006
> [ 67.284023][ T122] CFAR: c00000000022d100 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
> [ 67.284023][ T122] GPR00: c000000000050bec c000000034847850 c0000000013f6100 c000000001fb7718
> [ 67.284023][ T122] GPR04: c000000000515c10 c000000000e5fe08 c00000000133da60 c000000004839300
> [ 67.284023][ T122] GPR08: c0000000014ffb98 0000000000000000 c000000000515c0c c000000000e18576
> [ 67.284023][ T122] GPR12: c000000000e60170 c0000000015a0000 00000001155e0460 0000000000000000
> [ 67.284023][ T122] GPR16: 0000000000000000 00007fffe8eeb3c8 0000000116320728 0000000000000000
> [ 67.284023][ T122] GPR20: 0000000116320720 0000000000000000 c0000000012fa918 0000000000000006
> [ 67.284023][ T122] GPR24: c0000000014ffb98 c0000000011ed360 0000000000000000 c000000001fb7928
> [ 67.284023][ T122] GPR28: 0000000000000000 0000000000000000 000000007c0802a6 c000000001fb7918
> [ 67.287799][ T122] NIP [c000000000050bfc] arch_prepare_kprobe+0x10c/0x2d0
> [ 67.288490][ T122] LR [c000000000050bec] arch_prepare_kprobe+0xfc/0x2d0
> [ 67.289025][ T122] Call Trace:
> [ 67.289268][ T122] [c000000034847850] [c0000000012f77a0] 0xc0000000012f77a0 (unreliable)
> [ 67.289999][ T122] [c0000000348478d0] [c000000000231320] register_kprobe+0x3c0/0x7a0
> [ 67.290439][ T122] [c000000034847940] [c0000000002938c0] __register_trace_kprobe+0x140/0x1a0
> [ 67.290898][ T122] [c0000000348479b0] [c0000000002944c4] __trace_kprobe_create+0x794/0x1040
> [ 67.291330][ T122] [c000000034847b60] [c0000000002a1614] trace_probe_create+0xc4/0xe0
> [ 67.291717][ T122] [c000000034847bb0] [c00000000029363c] create_or_delete_trace_kprobe+0x2c/0x80
> [ 67.292158][ T122] [c000000034847bd0] [c000000000264420] trace_parse_run_command+0xf0/0x210
> [ 67.292611][ T122] [c000000034847c70] [c0000000002934a0] probes_write+0x20/0x40
> [ 67.292996][ T122] [c000000034847c90] [c00000000045e98c] vfs_write+0xfc/0x450
> [ 67.293356][ T122] [c000000034847d50] [c00000000045eec4] ksys_write+0x84/0x140
> [ 67.293716][ T122] [c000000034847da0] [c00000000002e4fc] system_call_exception+0x17c/0x3a0
> [ 67.294186][ T122] [c000000034847e10] [c00000000000c0e8] system_call_vectored_common+0xe8/0x278
> [ 67.294680][ T122] --- interrupt: 3000 at 0x7fffa5682de0
> [ 67.294937][ T122] NIP: 00007fffa5682de0 LR: 0000000000000000 CTR: 0000000000000000
> [ 67.295313][ T122] REGS: c000000034847e80 TRAP: 3000 Not tainted (6.0.0-rc3-00007-gdcf8e5633e2e)
> [ 67.295725][ T122] MSR: 900000000280f033 <SF,HV,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 44002408 XER: 00000000
> [ 67.296291][ T122] IRQMASK: 0
> [ 67.296291][ T122] GPR00: 0000000000000004 00007fffe8eeaec0 00007fffa5757300 0000000000000001
> [ 67.296291][ T122] GPR04: 0000000116329c60 0000000000000017 0000000000116329 0000000000000000
> [ 67.296291][ T122] GPR08: 0000000000000006 0000000000000000 0000000000000000 0000000000000000
> [ 67.296291][ T122] GPR12: 0000000000000000 00007fffa580ac60 00000001155e0460 0000000000000000
> [ 67.296291][ T122] GPR16: 0000000000000000 00007fffe8eeb3c8 0000000116320728 0000000000000000
> [ 67.296291][ T122] GPR20: 0000000116320720 0000000000000000 0000000000000000 0000000000000002
> [ 67.296291][ T122] GPR24: 00000001163206f0 0000000000000020 00007fffe8eeafa0 0000000000000001
> [ 67.296291][ T122] GPR28: 0000000000000000 0000000000000017 0000000116329c60 0000000000000001
> [ 67.299570][ T122] NIP [00007fffa5682de0] 0x7fffa5682de0
> [ 67.299837][ T122] LR [0000000000000000] 0x0
> [ 67.300072][ T122] --- interrupt: 3000
> [ 67.300447][ T122] Instruction dump:
> [ 67.300736][ T122] 386319d8 481342f5 60000000 60000000 60000000 e87f0028 3863fffc 481dc4d1
> [ 67.301230][ T122] 60000000 2c230000 41820018 e9230058 <81290000> 552936be 2c090001 4182018c
> [ 67.302102][ T122] ---[ end trace 0000000000000000 ]---
> [ 67.302496][ T122]
>
> [...]
Applied to powerpc/next.
[1/1] powerpc/kprobes: Fix null pointer reference in arch_prepare_kprobe()
https://git.kernel.org/powerpc/c/97f88a3d723162781d6cbfdc7b9617eefab55b19
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc/kprobes: Fix null pointer reference in arch_prepare_kprobe()
2022-09-30 9:47 ` Naveen N. Rao
@ 2022-10-12 6:14 ` Li Huafei
2022-10-12 9:52 ` Naveen N. Rao
0 siblings, 1 reply; 5+ messages in thread
From: Li Huafei @ 2022-10-12 6:14 UTC (permalink / raw)
To: Naveen N. Rao, jniethe5, mpe
Cc: peterz, linux-kernel, npiggin, rostedt, linuxppc-dev, mhiramat
On 2022/9/30 17:47, Naveen N. Rao wrote:
> Li Huafei wrote:
>> I found a null pointer reference in arch_prepare_kprobe():
>
> Good find!
>
Hi Naveen,
Thank you for the review.
>>
>> # echo 'p cmdline_proc_show' > kprobe_events
>> # echo 'p cmdline_proc_show+16' >> kprobe_events
>
> I think we should extend multiple_kprobes selftest to also place
> contiguous probes to catch such errors.
>
Yes. But each architecture implementation is different and it looks a
little difficult to decide which offsets need to be tested.
>> [ 67.278533][ T122] Kernel attempted to read user page (0) -
>> exploitattempt? (uid: 0)
>> [ 67.279326][ T122] BUG: Kernel NULL pointer dereference on read
>> at 0x00000000
>> [ 67.279738][ T122] Faulting instruction address: 0xc000000000050bfc
>> [ 67.280486][ T122] Oops: Kernel access of bad area, sig: 11 [#1]
>> [ 67.280846][ T122] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048
>> NUMA PowerNV
>> [ 67.281435][ T122] Modules linked in:
>> [ 67.281903][ T122] CPU: 0 PID: 122 Comm: sh Not tainted
>> 6.0.0-rc3-00007-gdcf8e5633e2e #10
>> [ 67.282547][ T122] NIP: c000000000050bfc LR: c000000000050bec
>> CTR:0000000000005bdc
>> [ 67.282920][ T122] REGS: c0000000348475b0 TRAP: 0300 Not
>> tainted (6.0.0-rc3-00007-gdcf8e5633e2e)
>> [ 67.283424][ T122] MSR: 9000000000009033
>> <SF,HV,EE,ME,IR,DR,RI,LE> CR: 88002444 XER: 20040006
>> [ 67.284023][ T122] CFAR: c00000000022d100 DAR: 0000000000000000
>> DSISR: 40000000 IRQMASK: 0
>> [ 67.284023][ T122] GPR00: c000000000050bec c000000034847850
>> c0000000013f6100 c000000001fb7718
>> [ 67.284023][ T122] GPR04: c000000000515c10 c000000000e5fe08
>> c00000000133da60 c000000004839300
>> [ 67.284023][ T122] GPR08: c0000000014ffb98 0000000000000000
>> c000000000515c0c c000000000e18576
>> [ 67.284023][ T122] GPR12: c000000000e60170 c0000000015a0000
>> 00000001155e0460 0000000000000000
>> [ 67.284023][ T122] GPR16: 0000000000000000 00007fffe8eeb3c8
>> 0000000116320728 0000000000000000
>> [ 67.284023][ T122] GPR20: 0000000116320720 0000000000000000
>> c0000000012fa918 0000000000000006
>> [ 67.284023][ T122] GPR24: c0000000014ffb98 c0000000011ed360
>> 0000000000000000 c000000001fb7928
>> [ 67.284023][ T122] GPR28: 0000000000000000 0000000000000000
>> 000000007c0802a6 c000000001fb7918
>> [ 67.287799][ T122] NIP [c000000000050bfc]
>> arch_prepare_kprobe+0x10c/0x2d0
>> [ 67.288490][ T122] LR [c000000000050bec]
>> arch_prepare_kprobe+0xfc/0x2d0
>> [ 67.289025][ T122] Call Trace:
>> [ 67.289268][ T122] [c000000034847850] [c0000000012f77a0]
>> 0xc0000000012f77a0 (unreliable)
>> [ 67.289999][ T122] [c0000000348478d0] [c000000000231320]
>> register_kprobe+0x3c0/0x7a0
>> [ 67.290439][ T122] [c000000034847940] [c0000000002938c0]
>> __register_trace_kprobe+0x140/0x1a0
>> [ 67.290898][ T122] [c0000000348479b0] [c0000000002944c4]
>> __trace_kprobe_create+0x794/0x1040
>> [ 67.291330][ T122] [c000000034847b60] [c0000000002a1614]
>> trace_probe_create+0xc4/0xe0
>> [ 67.291717][ T122] [c000000034847bb0] [c00000000029363c]
>> create_or_delete_trace_kprobe+0x2c/0x80
>> [ 67.292158][ T122] [c000000034847bd0] [c000000000264420]
>> trace_parse_run_command+0xf0/0x210
>> [ 67.292611][ T122] [c000000034847c70] [c0000000002934a0]
>> probes_write+0x20/0x40
>> [ 67.292996][ T122] [c000000034847c90] [c00000000045e98c]
>> vfs_write+0xfc/0x450
>> [ 67.293356][ T122] [c000000034847d50] [c00000000045eec4]
>> ksys_write+0x84/0x140
>> [ 67.293716][ T122] [c000000034847da0] [c00000000002e4fc]
>> system_call_exception+0x17c/0x3a0
>> [ 67.294186][ T122] [c000000034847e10] [c00000000000c0e8]
>> system_call_vectored_common+0xe8/0x278
>> [ 67.294680][ T122] --- interrupt: 3000 at 0x7fffa5682de0
>> [ 67.294937][ T122] NIP: 00007fffa5682de0 LR: 0000000000000000
>> CTR:0000000000000000
>> [ 67.295313][ T122] REGS: c000000034847e80 TRAP: 3000 Not
>> tainted (6.0.0-rc3-00007-gdcf8e5633e2e)
>> [ 67.295725][ T122] MSR: 900000000280f033
>> <SF,HV,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 44002408 XER: 00000000
>> [ 67.296291][ T122] IRQMASK: 0
>> [ 67.296291][ T122] GPR00: 0000000000000004 00007fffe8eeaec0
>> 00007fffa5757300 0000000000000001
>> [ 67.296291][ T122] GPR04: 0000000116329c60 0000000000000017
>> 0000000000116329 0000000000000000
>> [ 67.296291][ T122] GPR08: 0000000000000006 0000000000000000
>> 0000000000000000 0000000000000000
>> [ 67.296291][ T122] GPR12: 0000000000000000 00007fffa580ac60
>> 00000001155e0460 0000000000000000
>> [ 67.296291][ T122] GPR16: 0000000000000000 00007fffe8eeb3c8
>> 0000000116320728 0000000000000000
>> [ 67.296291][ T122] GPR20: 0000000116320720 0000000000000000
>> 0000000000000000 0000000000000002
>> [ 67.296291][ T122] GPR24: 00000001163206f0 0000000000000020
>> 00007fffe8eeafa0 0000000000000001
>> [ 67.296291][ T122] GPR28: 0000000000000000 0000000000000017
>> 0000000116329c60 0000000000000001
>> [ 67.299570][ T122] NIP [00007fffa5682de0] 0x7fffa5682de0
>> [ 67.299837][ T122] LR [0000000000000000] 0x0
>> [ 67.300072][ T122] --- interrupt: 3000
>> [ 67.300447][ T122] Instruction dump:
>> [ 67.300736][ T122] 386319d8 481342f5 60000000 60000000 60000000
>> e87f0028 3863fffc 481dc4d1
>> [ 67.301230][ T122] 60000000 2c230000 41820018 e9230058
>> <81290000> 552936be 2c090001 4182018c
>> [ 67.302102][ T122] ---[ end trace 0000000000000000 ]---
>> [ 67.302496][ T122]
>
> Please consider trimming the backtrace to only capture the necessary
> information:
> https://docs.kernel.org/process/submitting-patches.html#backtraces-in-commit-mesages
>
>
Thank you for the reminder. Michael has modified it when merging in,
thanks Michael.
>>
>> The address being probed has some special:
>>
>> cmdline_proc_show: Probe based on ftrace
>> cmdline_proc_show+16: Probe for the next instruction at the ftrace
>> location
>>
>> The ftrace-based kprobe does not generate kprobe::ainsn::insn, it gets
>> set to NULL. In arch_prepare_kprobe() it will check for:
>>
>> ...
>> prev = get_kprobe(p->addr - 1);
>> preempt_enable_no_resched();
>> if (prev && ppc_inst_prefixed(ppc_inst_read(prev->ainsn.insn))) {
>> ...
>>
>> If prev is based on ftrace, 'ppc_inst_read(prev->ainsn.insn)' will occur
>> with a null pointer reference. At this point prev->addr will not be a
>> prefixed instruction, so the check can be skipped.
>>
>> Check if prev is ftrace-based kprobe before reading 'prev->ainsn.insn'
>> to fix this problem.
>>
>> Fixes: b4657f7650ba ("powerpc/kprobes: Don't allow breakpoints on
>> suffixes")
>> Signed-off-by: Li Huafei <lihuafei1@huawei.com>
>> ---
>> arch/powerpc/kernel/kprobes.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kernel/kprobes.c
>> b/arch/powerpc/kernel/kprobes.c
>> index 912d4f8a13be..9f6cbbd56809 100644
>> --- a/arch/powerpc/kernel/kprobes.c
>> +++ b/arch/powerpc/kernel/kprobes.c
>> @@ -161,7 +161,12 @@ int arch_prepare_kprobe(struct kprobe *p)
>> preempt_disable();
>> prev = get_kprobe(p->addr - 1);
>> preempt_enable_no_resched();
>> - if (prev && ppc_inst_prefixed(ppc_inst_read(prev->ainsn.insn))) {
>> + /*
>> + * When prev is a ftrace-based kprobe, we don't have an insn, and it
>> + * doesn't probe for prefixed instruction.
>> + */
>> + if (prev && !kprobe_ftrace(prev) &&
>> + ppc_inst_prefixed(ppc_inst_read(prev->ainsn.insn))) {
>> printk("Cannot register a kprobe on the second word of
>> prefixed instruction\n");
>> ret = -EINVAL;
>> }
>
> It's fine to keep the if condition on a single line.
>
> Other than that, thanks for the fix!
> Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>
>
> - Naveen
>
> .
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc/kprobes: Fix null pointer reference in arch_prepare_kprobe()
2022-10-12 6:14 ` Li Huafei
@ 2022-10-12 9:52 ` Naveen N. Rao
0 siblings, 0 replies; 5+ messages in thread
From: Naveen N. Rao @ 2022-10-12 9:52 UTC (permalink / raw)
To: jniethe5, Li Huafei, mpe
Cc: peterz, linux-kernel, npiggin, rostedt, linuxppc-dev, mhiramat
Li Huafei wrote:
>>>
>>> # echo 'p cmdline_proc_show' > kprobe_events
>>> # echo 'p cmdline_proc_show+16' >> kprobe_events
>>
>> I think we should extend multiple_kprobes selftest to also place
>> contiguous probes to catch such errors.
>>
> Yes. But each architecture implementation is different and it looks a
> little difficult to decide which offsets need to be tested.
I don't think we need to be accurate here. A test to simply try putting
a probe at every byte within the first 256 bytes of a kernel function
should help catch many such issues. Some of those probes will be
rejected, but we can ignore errors.
- Naveen
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-12 9:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-23 9:32 [PATCH] powerpc/kprobes: Fix null pointer reference in arch_prepare_kprobe() Li Huafei
2022-09-30 9:47 ` Naveen N. Rao
2022-10-12 6:14 ` Li Huafei
2022-10-12 9:52 ` Naveen N. Rao
2022-10-04 13:26 ` Michael Ellerman
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).