Re: [PATCH] powerpc/kprobes: Pass ppc_inst as a pointer to emulate_step() on ppc32
From: Naveen N. Rao <hidden>
Date: 2021-06-08 11:35:09
Christophe Leroy wrote:
Le 07/06/2021 à 19:36, Christophe Leroy a écrit :quoted
Le 07/06/2021 à 16:31, Christophe Leroy a écrit :quoted
Le 07/06/2021 à 13:34, Naveen N. Rao a écrit :quoted
Naveen N. Rao wrote:quoted
Trying to use a kprobe on ppc32 results in the below splat: BUG: Unable to handle kernel data access on read at 0x7c0802a6 Faulting instruction address: 0xc002e9f0 Oops: Kernel access of bad area, sig: 11 [#1] BE PAGE_SIZE=4K PowerPC 44x Platform Modules linked in: CPU: 0 PID: 89 Comm: sh Not tainted 5.13.0-rc1-01824-g3a81c0495fdb #7 NIP: c002e9f0 LR: c0011858 CTR: 00008a47 REGS: c292fd50 TRAP: 0300 Not tainted (5.13.0-rc1-01824-g3a81c0495fdb) MSR: 00009000 <EE,ME> CR: 24002002 XER: 20000000 DEAR: 7c0802a6 ESR: 00000000 <snip> NIP [c002e9f0] emulate_step+0x28/0x324 LR [c0011858] optinsn_slot+0x128/0x10000 Call Trace: opt_pre_handler+0x7c/0xb4 (unreliable) optinsn_slot+0x128/0x10000 ret_from_syscall+0x0/0x28 The offending instruction is: 81 24 00 00 lwz r9,0(r4) Here, we are trying to load the second argument to emulate_step(): struct ppc_inst, which is the instruction to be emulated. On ppc64, structures are passed in registers when passed by value. However, per the ppc32 ABI, structures are always passed to functions as pointers. This isn't being adhered to when setting up the call to emulate_step() in the optprobe trampoline. Fix the same. Fixes: eacf4c0202654a ("powerpc: Enable OPTPROBES on PPC32") Signed-off-by: Naveen N. Rao <redacted> --- arch/powerpc/kernel/optprobes.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)Christophe, Can you confirm if this patch works for you? It would be good if this can go in v5.13.I'm trying to use kprobes, but I must be missing something. I have tried to follow the exemple in kernel's documentation: # echo 'p:myprobe do_sys_open dfd=%r3' > /sys/kernel/debug/tracing/kprobe_events # echo 1 > /sys/kernel/debug/tracing/events/kprobes/myprobe/enable # cat /sys/kernel/debug/kprobes/list c00122e4 k kretprobe_trampoline+0x0 [OPTIMIZED] c018a1b4 k do_sys_open+0x0 [OPTIMIZED] # cat /sys/kernel/debug/tracing/tracing_on 1 # cat /sys/kernel/debug/tracing/trace # tracer: nop # # entries-in-buffer/entries-written: 0/0 #P:1 # # _-----=> irqs-off # / _----=> need-resched # | / _---=> hardirq/softirq # || / _--=> preempt-depth # ||| / delay # TASK-PID CPU# |||| TIMESTAMP FUNCTION # | | | |||| | | So it looks like I get no event. I can't believe that do_sys_open() is never hit. This is without your patch, so it should Oops ? Then it looks like something is locked up somewhere, because I can't do anything else: # echo 'p:myprobe2 do_sys_openat2 dfd=%r3' >/sys/kernel/debug/tracing/kprobe_events -sh: can't create /sys/kernel/debug/tracing/kprobe_events: Device or resource busy # echo '-:myprobe' > /sys/kernel/debug/tracing/kprobe_events -sh: can't create /sys/kernel/debug/tracing/kprobe_events: Device or resource busy # echo > /sys/kernel/debug/tracing/kprobe_events -sh: can't create /sys/kernel/debug/tracing/kprobe_events: Device or resource busy
These should work if you disable the event. See below...
quoted
quoted
Ok, did a new test. Seems like do_sys_open() is really never called. I set the test at do_sys_openat2 , it was not optimised and was working. I set the test at do_sys_openat2+0x10 , it was optimised and crashed. Now I'm going to test the patch. When I set an event, is that normal that it removes the previous one ? Then we can have only one event at a time ? And then when that event is enabled we get 'Device or resource busy' when trying to add a new one ?
You should append to kprobe_events (i.e., use '>>') when _adding_ an event, otherwise it is considered a write and it tries to remove the existing event, which can't be done if the event is enabled. kprobe_events allows events to be removed only after they are disabled.
quoted
I confirm it doesn't crash anymore and it now works with optimised probes. Tested-by: Christophe Leroy <redacted>
Thanks! - Naveen