Re: [PATCH] powerpc/kprobes: Pass ppc_inst as a pointer to emulate_step() on ppc32

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 07/06/2021 à 16:31, Christophe Leroy a écrit :
> 
> 
> Le 07/06/2021 à 13:34, Naveen N. Rao a écrit :
>> Naveen N. Rao wrote:
>>> 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 <naveen.n.rao@linux.vnet.ibm.com>
>>> ---
>>>  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
> 
> 

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 ?

Christophe