Re: Re: Re: Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
* Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: (2013/11/27 22:30), Ingo Molnar wrote: * Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: (2013/11/22 11:35), Masami Hiramatsu wrote: (2013/11/21 16:29), Ingo Molnar wrote: * Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: (2013/11/21 2:36), Frank Ch. Eigler wrote: [ ... ] one needs to resort to something like: # cat /proc/kallsyms | grep ' [tT] ' | while read addr type symbol; do perf probe $symbol done then wait for a few hours for that to finish. Then, or while the loop is still running, run # perf record -e 'probe:*' -aR sleep 1 to take a kernel down. Um, indeed, current blacklist is not perfect. [...] Then it needs to be fixed ASAP! OK, I see. At least the two patches included this series should be fixed. :) And more, I need to test all symbols and drills down. OK, what I've found was; - The functions which can be ftraced look good. (see tracing/available_filter_functions) - following functions should not be able to be probed. - memcpy, memset - native_load_sp0 and some other native functions (need to be clear) - restore - trace_graph_return - trace_hardirqs_off_thunk, trace_hardirqs_on_thunk - This list still be not perfect. I just enabled/disabled kprobes one by one. There might be combined bugs (combination of several kprobes). - Some of them are hard to specify by NOKPROBE_SYMBOL because they are defined in assembly file. Anyway, to fix all of them, I think we need file-based blacklist especially for assembler symbols. assembler symbols shouldn't be particular hard either, just put them into the noprobes section. Would you mean .kprobes.text? Hmm, I hope not to use it anymore, but yeah, bugfix is more important. Agreed. No, why not put the symbol address into the 'blacklist' section, within the asm file? We fill out exception table entries in .S files as well, see the _ASM_EXTABLE() macro, it's possible to do all that. It needs not a CPP macro but an assembly macro. Thanks, Ingo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: Re: Re: Re: Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
(2013/11/30 22:46), Ingo Molnar wrote: Anyway, to fix all of them, I think we need file-based blacklist especially for assembler symbols. assembler symbols shouldn't be particular hard either, just put them into the noprobes section. Would you mean .kprobes.text? Hmm, I hope not to use it anymore, but yeah, bugfix is more important. Agreed. No, why not put the symbol address into the 'blacklist' section, within the asm file? We fill out exception table entries in .S files as well, see the _ASM_EXTABLE() macro, it's possible to do all that. Oh! I got it. Thank you for the pointer! :) It needs not a CPP macro but an assembly macro. OK, I'll try that. Thanks again, -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: Re: Re: Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
(2013/11/27 22:30), Ingo Molnar wrote: * Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: (2013/11/22 11:35), Masami Hiramatsu wrote: (2013/11/21 16:29), Ingo Molnar wrote: * Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: (2013/11/21 2:36), Frank Ch. Eigler wrote: [ ... ] one needs to resort to something like: # cat /proc/kallsyms | grep ' [tT] ' | while read addr type symbol; do perf probe $symbol done then wait for a few hours for that to finish. Then, or while the loop is still running, run # perf record -e 'probe:*' -aR sleep 1 to take a kernel down. Um, indeed, current blacklist is not perfect. [...] Then it needs to be fixed ASAP! OK, I see. At least the two patches included this series should be fixed. :) And more, I need to test all symbols and drills down. OK, what I've found was; - The functions which can be ftraced look good. (see tracing/available_filter_functions) - following functions should not be able to be probed. - memcpy, memset - native_load_sp0 and some other native functions (need to be clear) - restore - trace_graph_return - trace_hardirqs_off_thunk, trace_hardirqs_on_thunk - This list still be not perfect. I just enabled/disabled kprobes one by one. There might be combined bugs (combination of several kprobes). - Some of them are hard to specify by NOKPROBE_SYMBOL because they are defined in assembly file. Anyway, to fix all of them, I think we need file-based blacklist especially for assembler symbols. assembler symbols shouldn't be particular hard either, just put them into the noprobes section. Would you mean .kprobes.text? Hmm, I hope not to use it anymore, but yeah, bugfix is more important. Agreed. For example, we can get all text symbols by below command; nm some-file.o | grep -i t | cut -f3 -d so that we can make a blacklisted-symbol list for the file. I need to look the Kbuild for how I can do that in Makefile. I think it's generally better to add explicit annotations to the code. OK, thus I'll add annotations first. Thank you! -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: Re: Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
* Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: (2013/11/22 11:35), Masami Hiramatsu wrote: (2013/11/21 16:29), Ingo Molnar wrote: * Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: (2013/11/21 2:36), Frank Ch. Eigler wrote: [ ... ] one needs to resort to something like: # cat /proc/kallsyms | grep ' [tT] ' | while read addr type symbol; do perf probe $symbol done then wait for a few hours for that to finish. Then, or while the loop is still running, run # perf record -e 'probe:*' -aR sleep 1 to take a kernel down. Um, indeed, current blacklist is not perfect. [...] Then it needs to be fixed ASAP! OK, I see. At least the two patches included this series should be fixed. :) And more, I need to test all symbols and drills down. OK, what I've found was; - The functions which can be ftraced look good. (see tracing/available_filter_functions) - following functions should not be able to be probed. - memcpy, memset - native_load_sp0 and some other native functions (need to be clear) - restore - trace_graph_return - trace_hardirqs_off_thunk, trace_hardirqs_on_thunk - This list still be not perfect. I just enabled/disabled kprobes one by one. There might be combined bugs (combination of several kprobes). - Some of them are hard to specify by NOKPROBE_SYMBOL because they are defined in assembly file. Anyway, to fix all of them, I think we need file-based blacklist especially for assembler symbols. assembler symbols shouldn't be particular hard either, just put them into the noprobes section. For example, we can get all text symbols by below command; nm some-file.o | grep -i t | cut -f3 -d so that we can make a blacklisted-symbol list for the file. I need to look the Kbuild for how I can do that in Makefile. I think it's generally better to add explicit annotations to the code. Thanks, Ingo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: Re: Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
(2013/11/22 11:35), Masami Hiramatsu wrote: (2013/11/21 16:29), Ingo Molnar wrote: * Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: (2013/11/21 2:36), Frank Ch. Eigler wrote: [ ... ] one needs to resort to something like: # cat /proc/kallsyms | grep ' [tT] ' | while read addr type symbol; do perf probe $symbol done then wait for a few hours for that to finish. Then, or while the loop is still running, run # perf record -e 'probe:*' -aR sleep 1 to take a kernel down. Um, indeed, current blacklist is not perfect. [...] Then it needs to be fixed ASAP! OK, I see. At least the two patches included this series should be fixed. :) And more, I need to test all symbols and drills down. OK, what I've found was; - The functions which can be ftraced look good. (see tracing/available_filter_functions) - following functions should not be able to be probed. - memcpy, memset - native_load_sp0 and some other native functions (need to be clear) - restore - trace_graph_return - trace_hardirqs_off_thunk, trace_hardirqs_on_thunk - This list still be not perfect. I just enabled/disabled kprobes one by one. There might be combined bugs (combination of several kprobes). - Some of them are hard to specify by NOKPROBE_SYMBOL because they are defined in assembly file. Anyway, to fix all of them, I think we need file-based blacklist especially for assembler symbols. For example, we can get all text symbols by below command; nm some-file.o | grep -i t | cut -f3 -d so that we can make a blacklisted-symbol list for the file. I need to look the Kbuild for how I can do that in Makefile. Thank you, -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
(2013/11/21 16:29), Ingo Molnar wrote: * Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: (2013/11/21 2:36), Frank Ch. Eigler wrote: [ ... ] one needs to resort to something like: # cat /proc/kallsyms | grep ' [tT] ' | while read addr type symbol; do perf probe $symbol done then wait for a few hours for that to finish. Then, or while the loop is still running, run # perf record -e 'probe:*' -aR sleep 1 to take a kernel down. Um, indeed, current blacklist is not perfect. [...] Then it needs to be fixed ASAP! OK, I see. At least the two patches included this series should be fixed. :) And more, I need to test all symbols and drills down. [...] As I reported in this series, I've found 2 more patterns. I guess there still have some others. But anyway, I don't think it is good to fix all such bugs in this series. Fixing these bugs is far more important than any cleanup work. I see. This cleanup started with the bugfixes :) We can apply the fixes together with your cleanup series to make it all simpler, but the bug fixing absolutely needs to happen right now. OK, I'll test it first and include the bugfixes in this series. Or should I push the fixes separated? Thank you, -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
* Frank Ch. Eigler f...@redhat.com wrote: masami.hiramatsu.pt wrote: [...] This series also includes a change which prohibits probing on the address in .entry.text because the code is used for very low-level sensitive interrupt/syscall entries. Probing such code may cause unexpected result (actually most of that area is already in the kprobe blacklist). So I've decide to prohibit probing all of them. [...] Does this new blacklist cover enough that the kernel now survives a broadly wildcarded perf-probe, e.g. over e.g. all of its kallsyms? That's generally the purpose of the annotations - if it doesn't then that's a bug. Thanks, Ingo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
On Wed, 20 Nov 2013 12:36:00 -0500 Frank Ch. Eigler f...@redhat.com wrote: Hi - Does this new blacklist cover enough that the kernel now survives a broadly wildcarded perf-probe, e.g. over e.g. all of its kallsyms? That's generally the purpose of the annotations - if it doesn't then that's a bug. AFAIK, no kernel since kprobes was introduced has ever stood up to that test. perf probe lacks the wildcarding powers of systemtap, so one needs to resort to something like: # cat /proc/kallsyms | grep ' [tT] ' | while read addr type symbol; do perf probe $symbol done I'm curious to why one would do that. IIUC, perf now has function tracing support. -- Steve then wait for a few hours for that to finish. Then, or while the loop is still running, run # perf record -e 'probe:*' -aR sleep 1 to take a kernel down. - FChE ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
masami.hiramatsu.pt wrote: [...] This series also includes a change which prohibits probing on the address in .entry.text because the code is used for very low-level sensitive interrupt/syscall entries. Probing such code may cause unexpected result (actually most of that area is already in the kprobe blacklist). So I've decide to prohibit probing all of them. [...] Does this new blacklist cover enough that the kernel now survives a broadly wildcarded perf-probe, e.g. over e.g. all of its kallsyms? - FChE ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
Hi - Does this new blacklist cover enough that the kernel now survives a broadly wildcarded perf-probe, e.g. over e.g. all of its kallsyms? That's generally the purpose of the annotations - if it doesn't then that's a bug. AFAIK, no kernel since kprobes was introduced has ever stood up to that test. perf probe lacks the wildcarding powers of systemtap, so one needs to resort to something like: # cat /proc/kallsyms | grep ' [tT] ' | while read addr type symbol; do perf probe $symbol done then wait for a few hours for that to finish. Then, or while the loop is still running, run # perf record -e 'probe:*' -aR sleep 1 to take a kernel down. - FChE ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
On 11/20/2013 09:56 AM, Steven Rostedt wrote: On Wed, 20 Nov 2013 12:36:00 -0500 Frank Ch. Eigler f...@redhat.com wrote: Hi - Does this new blacklist cover enough that the kernel now survives a broadly wildcarded perf-probe, e.g. over e.g. all of its kallsyms? That's generally the purpose of the annotations - if it doesn't then that's a bug. AFAIK, no kernel since kprobes was introduced has ever stood up to that test. perf probe lacks the wildcarding powers of systemtap, so one needs to resort to something like: # cat /proc/kallsyms | grep ' [tT] ' | while read addr type symbol; do perf probe $symbol done I'm curious to why one would do that. IIUC, perf now has function tracing support. Then consider something like probing all inline call sites, which will be sprinkled in the middle where ftrace doesn't apply. The point is not whether there's an alternative - kprobes really ought to be wholly safe regardless. Slow, if you did such broad probing, sure, but still safe. And a real use-case probably wouldn't probe *all* functions/inlines, but it illustrates that there are at least a few in the full set that don't behave well. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
(2013/11/21 2:36), Frank Ch. Eigler wrote: Hi - Does this new blacklist cover enough that the kernel now survives a broadly wildcarded perf-probe, e.g. over e.g. all of its kallsyms? That's generally the purpose of the annotations - if it doesn't then that's a bug. AFAIK, no kernel since kprobes was introduced has ever stood up to that test. perf probe lacks the wildcarding powers of systemtap, so one needs to resort to something like: # cat /proc/kallsyms | grep ' [tT] ' | while read addr type symbol; do perf probe $symbol done then wait for a few hours for that to finish. Then, or while the loop is still running, run # perf record -e 'probe:*' -aR sleep 1 to take a kernel down. Um, indeed, current blacklist is not perfect. As I reported in this series, I've found 2 more patterns. I guess there still have some others. But anyway, I don't think it is good to fix all such bugs in this series. This is just the first step to do it. :) Thank you, -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
* Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: (2013/11/21 2:36), Frank Ch. Eigler wrote: [ ... ] one needs to resort to something like: # cat /proc/kallsyms | grep ' [tT] ' | while read addr type symbol; do perf probe $symbol done then wait for a few hours for that to finish. Then, or while the loop is still running, run # perf record -e 'probe:*' -aR sleep 1 to take a kernel down. Um, indeed, current blacklist is not perfect. [...] Then it needs to be fixed ASAP! [...] As I reported in this series, I've found 2 more patterns. I guess there still have some others. But anyway, I don't think it is good to fix all such bugs in this series. Fixing these bugs is far more important than any cleanup work. We can apply the fixes together with your cleanup series to make it all simpler, but the bug fixing absolutely needs to happen right now. Thanks, Ingo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH -tip v3 00/23] kprobes: introduce NOKPROBE_SYMBOL() and general cleaning of kprobe blacklist
Hi, Here is the version 3 of NOKPORBE_SYMBOL series. Currently the blacklist is maintained by hand in kprobes.c which is separated from the function definition and is hard to catch up the kernel update. To solve this issue, I've introduced NOKPROBE_SYMBOL() macro for making kprobe blacklist at build time. Since the NOKPROBE_SYMBOL() macros can be placed right after the function is defined (as like as EXPORT_SYMBOL), it is easy to maintain. This series replaces __kprobes with NOKPROBE_SYMBOL() macro or apply __always_inline annotation for some cases, because NOKPROBE_SYMBOL() will inhibit inlining by referring the symbol address. :( At this point, I replaced all __kprobes under kernel/ and arch/x86. For future work, I'd like to replace all the __kprobes annotation for all archs too. Also, I decided to classify current __kprobes annotation users who misuse it. Most of the preparation, registration, optimization functions related to kprobes are not involved in the breakpoint or other exception handling. This means that those never cause problems such as infinite recursion if we put kprobes on it. This also reduces blacklist a lot. For easy to check the blacklist, you can see what address region/symbols are not allowed to probe via /sys/kernel/debug/kprobes/blacklist. Since the new blacklist can be populated/shrinked dynamically, the blacklist now also support modules. :) kprobes users can make a custom blacklisted functions which will be called from kprobes handlers. Example codes are also updated, so you can see how it works. This series also includes a change which prohibits probing on the address in .entry.text because the code is used for very low-level sensitive interrupt/syscall entries. Probing such code may cause unexpected result (actually most of that area is already in the kprobe blacklist). So I've decide to prohibit probing all of them. Finally, I got an empty .kprobes.text on x86 :) $ grep kprobes_text System.map 81604980 T __kprobes_text_end 81604980 T __kprobes_text_start Thank you, Changes from v2 to v3: - Introduce arch_within_kprobe_blacklist() which checks the address is within the .kprobes.text (generic,x86) or .entry.text (x86), for fixing build issue on !x86. - Rename in_nokprobes_functions to within_kprobe_blacklist and it returns a bool value istead of an error. - Fix the type of kprobe_blacklist_seq_stop(). - Use blacklist entry to check the blacklisted address ranges (.entry.text/.kprobes.text). This also eliminates arch_within_kprobe_blacklist(). :) Changes from v1 to v2: - Replace __kprobes with NOKPROBE_SYMBOL() and remove unneeded __kprobes on the files compiled on x86. - Add blacklist on modules support. - Add debugfs interface for blacklist. - Fix indent of the NOKPROBE_SYMBOL() by using tabs. - Fix NOKPROBE_SYMBOL() for expanding nested macro. - Update Documentations/kprobes.txt about blacklist. --- Masami Hiramatsu (23): kprobes: Prohibit probing on .entry.text code kprobes: Introduce NOKPROBE_SYMBOL() macro for blacklist kprobes: Show blacklist entries via debugfs kprobes: Support blacklist functions in module kprobes: Use NOKPROBE_SYMBOL() in sample modules kprobes/x86: Allow probe on some kprobe preparation functions kprobes/x86: Use NOKPROBE_SYMBOL instead of __kprobes kprobes: Allow probe on some kprobe functions kprobes: Use NOKPROBE_SYMBOL macro instead of __kprobes ftrace/kprobes: Allow probing on some preparation functions ftrace/kprobes: Use NOKPROBE_SYMBOL macro in ftrace x86/hw_breakpoint: Use NOKPROBE_SYMBOL macro in hw_breakpoint x86/trap: Use NOKPROBE_SYMBOL macro in trap.c x86/fault: Use NOKPROBE_SYMBOL macro in fault.c x86/alternative: Use NOKPROBE_SYMBOL macro in alternative.c x86/nmi: Use NOKPROBE_SYMBOL macro for nmi handlers x86/kvm: Use NOKPROBE_SYMBOL macro in kvm.c x86/dumpstack: Use NOKPROBE_SYMBOL macro in dumpstack.c [BUGFIX] kprobes/x86: Prohibit probing on debug_stack_* [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text notifier: Use NOKPROBE_SYMBOL macro in notifier sched: Use NOKPROBE_SYMBOL macro in sched kprobes/x86: Use kprobe_blacklist for .kprobes.text and .entry.text Documentation/kprobes.txt| 24 ++ arch/x86/include/asm/traps.h |2 arch/x86/kernel/alternative.c|3 arch/x86/kernel/apic/hw_nmi.c|3 arch/x86/kernel/cpu/common.c |4 arch/x86/kernel/cpu/perf_event.c |3 arch/x86/kernel/cpu/perf_event_amd_ibs.c |3 arch/x86/kernel/dumpstack.c |9 - arch/x86/kernel/entry_32.S | 33 -- arch/x86/kernel/entry_64.S | 20 - arch/x86/kernel/hw_breakpoint.c |6 arch/x86/kernel/kprobes/core.c | 105 +-- arch/x86/kernel/kprobes/ftrace.c | 17 +
