On 2015/1/19 20:45, Masami Hiramatsu wrote: > (2015/01/19 20:21), Wang Nan wrote: >> On 2015/1/19 17:05, Masami Hiramatsu wrote: >>> Hi Wang, >>> >>> I've found a problem on this patch, since kprobes calls unoptioize_kprobe >>> with kprobes_all_disarmed=true when trying to disable all kprobes, this >>> cause a serious problem. >>> >>> Moreover, I couldn't reproduce your reported bug on my 3.19-rc4 kernel. >>> Could you test it again? >>> >> >> I tested it again based on 3.19-rc5 and found that the problem still exists. >> My testing is based on QEMU. >> >> First I tested my kprobeopt for ARM, then on x86_64. The test results are >> pasted >> at the bottom of this mail. Commands after 'gdb attaches to QEMU' is my >> actions >> on a gdb console attached to QEMU; commands after 'inside virtual machine' is >> what I do in Linux run under QEMU. > > Thank you for the reporting. > So, now I know what happened, the problem is "debugfs/kprobes/enabled doesn't > work > correctly on optimized kprobes". Please make update the patch description. > > I also reproduced the bug without gdb. > Here is the log. > > ---- > [root@localhost ~]# cd /sys/kernel/debug/tracing/ > [root@localhost tracing]# echo p do_fork+5 > kprobe_events # setup new > event > [root@localhost tracing]# echo $$ > set_ftrace_pid # trace only > this process > [root@localhost tracing]# echo 1 > events/kprobes/p_do_fork_5/enable # > enable it > [root@localhost tracing]# cat trace # check the > trace data > # tracer: nop > # > # entries-in-buffer/entries-written: 1/1 #P:8 > # > # _-----=> irqs-off > # / _----=> need-resched > # | / _---=> hardirq/softirq > # || / _--=> preempt-depth > # ||| / delay > # TASK-PID CPU# |||| TIMESTAMP FUNCTION > # | | | |||| | | > bash-3883 [006] d... 279.799023: p_do_fork_5: > (do_fork+0x5/0x360) # OK, now tracing > [root@localhost tracing]# cat ../kprobes/list > ffffffff810bc1c5 k do_fork+0x5 [OPTIMIZED] # and > it is actually optimized > [root@localhost tracing]# echo 0 > ../kprobes/enabled # disable *ALL* > kprobes > [root@localhost tracing]# echo > trace # clear > events > [root@localhost tracing]# cat trace # this should > show empty buffer > # tracer: nop > # > # entries-in-buffer/entries-written: 1/1 #P:8 > # > # _-----=> irqs-off > # / _----=> need-resched > # | / _---=> hardirq/softirq > # || / _--=> preempt-depth > # ||| / delay > # TASK-PID CPU# |||| TIMESTAMP FUNCTION > # | | | |||| | | > bash-3883 [006] d... 337.770785: p_do_fork_5: > (do_fork+0x5/0x360) # But still tracing! > [root@localhost tracing]# cat trace # Check again > # tracer: nop > # > # entries-in-buffer/entries-written: 2/2 #P:8 > # > # _-----=> irqs-off > # / _----=> need-resched > # | / _---=> hardirq/softirq > # || / _--=> preempt-depth > # ||| / delay > # TASK-PID CPU# |||| TIMESTAMP FUNCTION > # | | | |||| | | > bash-3883 [006] d... 337.770785: p_do_fork_5: > (do_fork+0x5/0x360) > bash-3883 [006] d... 345.592178: p_do_fork_5: > (do_fork+0x5/0x360) # We are tracing!! > > So, after global disabling kprobes, ALL kprobes event should be disabled, but > not. > > OK, I think your first patch is better than the second one, but not enough. > What we should do is use kprobes_all_disarmed for force option like below. > > unoptimize_kprobe(p, kprobes_all_disarmed); /* Try to unoptimize */ > > We also would better to check the flag in unregistering path for skipping > unneeded > disarming process when kprobes globally disarmed. > > Thank you, >
Thanks to your quick reply. I'll post an improved v1 patch tomorrow. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
