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 à 19:36, Christophe Leroy a écrit :
> 
> 
> 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 ?
> 

I confirm it doesn't crash anymore and it now works with optimised probes.

Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>