Re: [PATCH] riscv/kprobe: fix kernel panic when invoking sys_read traced by kprobe

2021-04-08 Thread liaochang (A)


在 2021/4/8 19:20, Jisheng Zhang 写道:
> On Tue, 30 Mar 2021 16:18:48 +0800
> Liao Chang  wrote:
> 
> 
>>
>> The execution of sys_read end up hitting a BUG_ON() in __find_get_block
>> after installing kprobe at sys_read, the BUG message like the following:
>>
>> [   65.708663] [ cut here ]
>> [   65.709987] kernel BUG at fs/buffer.c:1251!
>> [   65.711283] Kernel BUG [#1]
>> [   65.712032] Modules linked in:
>> [   65.712925] CPU: 0 PID: 51 Comm: sh Not tainted 5.12.0-rc4 #1
>> [   65.714407] Hardware name: riscv-virtio,qemu (DT)
>> [   65.715696] epc : __find_get_block+0x218/0x2c8
>> [   65.716835]  ra : __getblk_gfp+0x1c/0x4a
>> [   65.717831] epc : ffe00019f11e ra : ffe00019f56a sp : 
>> ffe002437930
>> [   65.719553]  gp : ffe000f06030 tp : ffe0015abc00 t0 : 
>> ffe00191e038
>> [   65.721290]  t1 : ffe00191e038 t2 : 000a s0 : 
>> ffe002437960
>> [   65.723051]  s1 : ffe00160ad00 a0 : ffe00160ad00 a1 : 
>> 012a
>> [   65.724772]  a2 : 0400 a3 : 0008 a4 : 
>> 0040
>> [   65.726545]  a5 :  a6 : ffe00191e000 a7 : 
>> 
>> [   65.728308]  s2 : 012a s3 : 0400 s4 : 
>> 0008
>> [   65.730049]  s5 : 006c s6 : ffe00240f800 s7 : 
>> ffe000f080a8
>> [   65.731802]  s8 : 0001 s9 : 012a s10: 
>> 0008
>> [   65.733516]  s11: 0008 t3 : 03ff t4 : 
>> 000f
>> [   65.734434]  t5 : 03ff t6 : 0004
>> [   65.734613] status: 0100 badaddr:  cause: 
>> 0003
>> [   65.734901] Call Trace:
>> [   65.735076] [] __find_get_block+0x218/0x2c8
>> [   65.735417] [] __ext4_get_inode_loc+0xb2/0x2f6
>> [   65.735618] [] ext4_get_inode_loc+0x3a/0x8a
>> [   65.735802] [] ext4_reserve_inode_write+0x2e/0x8c
>> [   65.735999] [] __ext4_mark_inode_dirty+0x4c/0x18e
>> [   65.736208] [] ext4_dirty_inode+0x46/0x66
>> [   65.736387] [] __mark_inode_dirty+0x12c/0x3da
>> [   65.736576] [] touch_atime+0x146/0x150
>> [   65.736748] [] filemap_read+0x234/0x246
>> [   65.736920] [] generic_file_read_iter+0xc0/0x114
>> [   65.737114] [] ext4_file_read_iter+0x42/0xea
>> [   65.737310] [] new_sync_read+0xe2/0x15a
>> [   65.737483] [] vfs_read+0xca/0xf2
>> [   65.737641] [] ksys_read+0x5e/0xc8
>> [   65.737816] [] sys_read+0xe/0x16
>> [   65.737973] [] ret_from_syscall+0x0/0x2
>> [   65.738858] ---[ end trace fe93f985456c935d ]---
>>
>> A simple reproducer looks like:
>> echo 'p:myprobe sys_read fd=%a0 buf=%a1 count=%a2' > 
>> /sys/kernel/debug/tracing/kprobe_events
>> echo 1 > /sys/kernel/debug/tracing/events/kprobes/myprobe/enable
>> cat /sys/kernel/debug/tracing/trace
>>
> 
> I can't reproduce the BUG_ON with the above step, I may miss something.
> 
My test platform versions
Kernel: 0d02ec6b3136 Linux 5.12-rc4
QEMU: fdd76fecdd Update version for v5.0.0 release

>> Here's what happens to hit that BUG_ON():
>>
>> 1) After installing kprobe at entry of sys_read, the first instruction
>>is replaced by 'ebreak' instruction on riscv64 platform.
>>
>> 2) Once kernel reach the 'ebreak' instruction at the entry of sys_read,
>>it trap into the riscv breakpoint handler, where it do something to
>>setup for coming single-step of origin instruction, including backup
>>the 'sstatus' in pt_regs, followed by disable interrupt during single
>>stepping via clear 'SIE' bit of 'sstatus' in pt_regs.
>>
>> 3) Then kernel restore to the instruction slot contains two instructions,
>>one is original instruction at entry of sys_read, the other is 'ebreak'.
>>Here it trigger a 'Instruction page fault' exception (value at 'scause'
>>is '0xc'), if PF is not filled into PageTabe for that slot yet.
>>
>> 4) Again kernel trap into page fault exception handler, where it choose
>>different policy according to the state of running kprobe. Because
>>afte 2) the state is KPROBE_HIT_SS, so kernel reset the current kprobe
>>and 'pc' points back to the probe address.
>>
>> 5) Because 'epc' point back to 'ebreak' instrution at sys_read probe,
>>kernel trap into breakpoint handler again, and repeat the operations
>>at 2), however 'sstatus' without 'SIE' is keep at 4), it cause the
>>real 'sstatus' saved at 2) is overwritten by the one withou 'SIE'.
> 
> Is kprobe_single_step_handler() handled firstly this time? thus we won't
> enter kprobe_breakpoint_handler().
> 
No,because this time kcb->ss.ctx.match_addr points to the single-step slot,but
instruction_pointer(regs) points to the first instruction of sys_read('ebreak')
so the condition is not token eventually, then we enter 
kprobe_breakpoint_handler().

bool __kprobes
kprobe_single_step_handler(struct pt_regs *regs)
{
struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();

if ((kcb->ss_ctx.ss_pending)
   

Re: [PATCH] riscv: keep interrupts disabled for BREAKPOINT exception

2021-04-06 Thread liaochang (A)
Hi Jisheng,

在 2021/4/2 21:32, Jisheng Zhang 写道:
> On Thu, 1 Apr 2021 16:49:47 +0800
> "liaochang (A)"  wrote:
> 
>> Hi Jisheng,
> 
> Hi,
> 
>>
>> 在 2021/3/31 22:22, Jisheng Zhang 写道:
>>> On Tue, 30 Mar 2021 18:33:16 +0900
>>> Masami Hiramatsu  wrote:
>>>   
>>>> Hi Jisheng,  
>>>
>>> Hi Masami,
>>>   
>>>>
>>>> On Tue, 30 Mar 2021 02:16:24 +0800
>>>> Jisheng Zhang  wrote:
>>>>  
>>>>> From: Jisheng Zhang 
>>>>>
>>>>> Current riscv's kprobe handlers are run with both preemption and
>>>>> interrupt enabled, this violates kprobe requirements. Fix this issue
>>>>> by keeping interrupts disabled for BREAKPOINT exception.
>>>>
>>>> Not only while the breakpoint exception but also until the end of
>>>> the single step (maybe you are using __BUG_INSN_32 ??) need to be
>>>> disable interrupts. Can this do that?
>>>>  
>>>
>>> interrupt is disabled during "single step" by kprobes_save_local_irqflag()
>>> and kprobes_restore_local_irqflag(). The code flow looks like: 
>>>
>>> do_trap_break()   // for bp
>>>   kprobe_breakpoint_handler()
>>> setup_singlestep()
>>>   kprobes_restore_local_irqflag()
>>>
>>> do_trap_break()  // for ss
>>>   kprobe_single_step_handler()
>>> kprobes_restore_local_irqflag()  
>>
>> Recently, kernel hit BUG_ON() on QEMU after I install a probe at "sys_read" 
>> via kprobe,
> 
> TIPS: Each line should not exceed 80 chars
> 
>> accoriding to my debugging and analysis it looks like caused by the "irq 
>> disable" operation for single-stepping.
>>
>> I present a detailed description about this problem in the mail with title 
>> "[PATCH] riscv/kprobe: fix kernel panic when invoking sys_read traced by 
>> kprobe".
>> Looking forward to some feedback,Thanks.
>>
> 
> I will comment that patch.

Thanks for your reminder.

> 
> thanks
> 
> .
> 


Re: [PATCH] riscv: keep interrupts disabled for BREAKPOINT exception

2021-04-01 Thread liaochang (A)
Hi Jisheng,

在 2021/3/31 22:22, Jisheng Zhang 写道:
> On Tue, 30 Mar 2021 18:33:16 +0900
> Masami Hiramatsu  wrote:
> 
>> Hi Jisheng,
> 
> Hi Masami,
> 
>>
>> On Tue, 30 Mar 2021 02:16:24 +0800
>> Jisheng Zhang  wrote:
>>
>>> From: Jisheng Zhang 
>>>
>>> Current riscv's kprobe handlers are run with both preemption and
>>> interrupt enabled, this violates kprobe requirements. Fix this issue
>>> by keeping interrupts disabled for BREAKPOINT exception.  
>>
>> Not only while the breakpoint exception but also until the end of
>> the single step (maybe you are using __BUG_INSN_32 ??) need to be
>> disable interrupts. Can this do that?
>>
> 
> interrupt is disabled during "single step" by kprobes_save_local_irqflag()
> and kprobes_restore_local_irqflag(). The code flow looks like: 
> 
> do_trap_break()   // for bp
>   kprobe_breakpoint_handler()
> setup_singlestep()
>   kprobes_restore_local_irqflag()
> 
> do_trap_break()  // for ss
>   kprobe_single_step_handler()
> kprobes_restore_local_irqflag()

Recently, kernel hit BUG_ON() on QEMU after I install a probe at "sys_read" via 
kprobe,
accoriding to my debugging and analysis it looks like caused by the "irq 
disable" operation for single-stepping.

I present a detailed description about this problem in the mail with title 
"[PATCH] riscv/kprobe: fix kernel panic when invoking sys_read traced by 
kprobe".
Looking forward to some feedback,Thanks.

BR,
Liao Chang
> 
> Thanks
> 
> 
> ___
> linux-riscv mailing list
> linux-ri...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
> .
>