* [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions @ 2018-12-06 9:56 Andrea Righi 2018-12-06 16:39 ` [tip:perf/urgent] kprobes/x86: Blacklist " tip-bot for Andrea Righi 2018-12-07 14:47 ` [PATCH] kprobes: x86_64: blacklist " Masami Hiramatsu 0 siblings, 2 replies; 11+ messages in thread From: Andrea Righi @ 2018-12-06 9:56 UTC (permalink / raw) To: Naveen N. Rao, Anil S Keshavamurthy, David S. Miller, Masami Hiramatsu Cc: Yonghong Song, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-kernel These interrupt functions are already non-attachable by kprobes. Blacklist them explicitly so that they can show up in /sys/kernel/debug/kprobes/blacklist and tools like BCC can use this additional information. Signed-off-by: Andrea Righi <righi.andrea@gmail.com> --- arch/x86/entry/entry_64.S | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index ce25d84023c0..1f0efdb7b629 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -566,6 +566,7 @@ ENTRY(interrupt_entry) ret END(interrupt_entry) +_ASM_NOKPROBE(interrupt_entry) /* Interrupt entry/exit. */ @@ -766,6 +767,7 @@ native_irq_return_ldt: jmp native_irq_return_iret #endif END(common_interrupt) +_ASM_NOKPROBE(common_interrupt) /* * APIC interrupts. @@ -780,6 +782,7 @@ ENTRY(\sym) call \do_sym /* rdi points to pt_regs */ jmp ret_from_intr END(\sym) +_ASM_NOKPROBE(\sym) .endm /* Make sure APIC interrupt handlers end up in the irqentry section: */ @@ -960,6 +963,7 @@ ENTRY(\sym) jmp error_exit .endif +_ASM_NOKPROBE(\sym) END(\sym) .endm -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [tip:perf/urgent] kprobes/x86: Blacklist non-attachable interrupt functions 2018-12-06 9:56 [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions Andrea Righi @ 2018-12-06 16:39 ` tip-bot for Andrea Righi 2018-12-07 14:47 ` [PATCH] kprobes: x86_64: blacklist " Masami Hiramatsu 1 sibling, 0 replies; 11+ messages in thread From: tip-bot for Andrea Righi @ 2018-12-06 16:39 UTC (permalink / raw) To: linux-tip-commits Cc: yhs, tglx, linux-kernel, mhiramat, luto, hpa, righi.andrea, torvalds, bp, peterz, naveen.n.rao, mingo, davem, anil.s.keshavamurthy Commit-ID: a50480cb6d61d5c5fc13308479407b628b6bc1c5 Gitweb: https://git.kernel.org/tip/a50480cb6d61d5c5fc13308479407b628b6bc1c5 Author: Andrea Righi <righi.andrea@gmail.com> AuthorDate: Thu, 6 Dec 2018 10:56:48 +0100 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Thu, 6 Dec 2018 16:52:03 +0100 kprobes/x86: Blacklist non-attachable interrupt functions These interrupt functions are already non-attachable by kprobes. Blacklist them explicitly so that they can show up in /sys/kernel/debug/kprobes/blacklist and tools like BCC can use this additional information. Signed-off-by: Andrea Righi <righi.andrea@gmail.com> Cc: Andy Lutomirski <luto@kernel.org> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com> Cc: Borislav Petkov <bp@alien8.de> Cc: David S. Miller <davem@davemloft.net> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Masami Hiramatsu <mhiramat@kernel.org> Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Yonghong Song <yhs@fb.com> Link: http://lkml.kernel.org/r/20181206095648.GA8249@Dell Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/entry/entry_64.S | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index ce25d84023c0..1f0efdb7b629 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -566,6 +566,7 @@ ENTRY(interrupt_entry) ret END(interrupt_entry) +_ASM_NOKPROBE(interrupt_entry) /* Interrupt entry/exit. */ @@ -766,6 +767,7 @@ native_irq_return_ldt: jmp native_irq_return_iret #endif END(common_interrupt) +_ASM_NOKPROBE(common_interrupt) /* * APIC interrupts. @@ -780,6 +782,7 @@ ENTRY(\sym) call \do_sym /* rdi points to pt_regs */ jmp ret_from_intr END(\sym) +_ASM_NOKPROBE(\sym) .endm /* Make sure APIC interrupt handlers end up in the irqentry section: */ @@ -960,6 +963,7 @@ ENTRY(\sym) jmp error_exit .endif +_ASM_NOKPROBE(\sym) END(\sym) .endm ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions 2018-12-06 9:56 [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions Andrea Righi 2018-12-06 16:39 ` [tip:perf/urgent] kprobes/x86: Blacklist " tip-bot for Andrea Righi @ 2018-12-07 14:47 ` Masami Hiramatsu 2018-12-07 16:01 ` Masami Hiramatsu 1 sibling, 1 reply; 11+ messages in thread From: Masami Hiramatsu @ 2018-12-07 14:47 UTC (permalink / raw) To: Andrea Righi Cc: Naveen N. Rao, Anil S Keshavamurthy, David S. Miller, Yonghong Song, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-kernel On Thu, 6 Dec 2018 10:56:48 +0100 Andrea Righi <righi.andrea@gmail.com> wrote: > These interrupt functions are already non-attachable by kprobes. > Blacklist them explicitly so that they can show up in > /sys/kernel/debug/kprobes/blacklist and tools like BCC can use this > additional information. > > Signed-off-by: Andrea Righi <righi.andrea@gmail.com> For the short term, this is OK. But this is not the best way to cover other functions which are also prohibited by arch_within_kprobe_blacklist(). I'll fix this issue later. Acked-by: Masami Hiramatsu <mhiramat@kernel.org> Thank you, > --- > arch/x86/entry/entry_64.S | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S > index ce25d84023c0..1f0efdb7b629 100644 > --- a/arch/x86/entry/entry_64.S > +++ b/arch/x86/entry/entry_64.S > @@ -566,6 +566,7 @@ ENTRY(interrupt_entry) > > ret > END(interrupt_entry) > +_ASM_NOKPROBE(interrupt_entry) > > > /* Interrupt entry/exit. */ > @@ -766,6 +767,7 @@ native_irq_return_ldt: > jmp native_irq_return_iret > #endif > END(common_interrupt) > +_ASM_NOKPROBE(common_interrupt) > > /* > * APIC interrupts. > @@ -780,6 +782,7 @@ ENTRY(\sym) > call \do_sym /* rdi points to pt_regs */ > jmp ret_from_intr > END(\sym) > +_ASM_NOKPROBE(\sym) > .endm > > /* Make sure APIC interrupt handlers end up in the irqentry section: */ > @@ -960,6 +963,7 @@ ENTRY(\sym) > > jmp error_exit > .endif > +_ASM_NOKPROBE(\sym) > END(\sym) > .endm > > -- > 2.17.1 -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions 2018-12-07 14:47 ` [PATCH] kprobes: x86_64: blacklist " Masami Hiramatsu @ 2018-12-07 16:01 ` Masami Hiramatsu 2018-12-07 17:00 ` Andrea Righi 2018-12-07 17:58 ` Andrea Righi 0 siblings, 2 replies; 11+ messages in thread From: Masami Hiramatsu @ 2018-12-07 16:01 UTC (permalink / raw) To: Masami Hiramatsu Cc: Andrea Righi, Naveen N. Rao, Anil S Keshavamurthy, David S. Miller, Yonghong Song, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-kernel Hi Andrea and Ingo, Here is the patch what I meant. I just ran it on qemu-x86, and seemed working. After introducing this patch, I will start adding arch_populate_kprobe_blacklist() to some arches. Thank you, [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area From: Masami Hiramatsu <mhiramat@kernel.org> Blacklist symbols in arch-defined probe-prohibited areas. With this change, user can see all symbols which are prohibited to probe in debugfs. All archtectures which have custom prohibit areas should define its own arch_populate_kprobe_blacklist() function, but unless that, all symbols marked __kprobes are blacklisted. Reported-by: Andrea Righi <righi.andrea@gmail.com> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- arch/x86/kernel/kprobes/core.c | 27 ++++++++++++++++++ include/linux/kprobes.h | 2 + kernel/kprobes.c | 60 +++++++++++++++++++++++++++++++--------- 3 files changed, 75 insertions(+), 14 deletions(-) diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c index c33b06f..52771e3 100644 --- a/arch/x86/kernel/kprobes/core.c +++ b/arch/x86/kernel/kprobes/core.c @@ -1034,6 +1034,33 @@ bool arch_within_kprobe_blacklist(unsigned long addr) addr < (unsigned long)__entry_text_end); } +int __init arch_populate_kprobe_blacklist(void) +{ + unsigned long entry; + int ret = 0; + + for (entry = (unsigned long)__kprobes_text_start; + entry < (unsigned long)__kprobes_text_end; + entry += ret) { + ret = kprobe_add_ksym_blacklist(entry); + if (ret < 0) + return ret; + if (ret == 0) /* In case of alias symbol */ + ret = 1; + } + + for (entry = (unsigned long)__entry_text_start; + entry < (unsigned long)__entry_text_end; + entry += ret) { + ret = kprobe_add_ksym_blacklist(entry); + if (ret < 0) + return ret; + if (ret == 0) /* In case of alias symbol */ + ret = 1; + } + return 0; +} + int __init arch_init_kprobes(void) { return 0; diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h index e909413..a68cbbe 100644 --- a/include/linux/kprobes.h +++ b/include/linux/kprobes.h @@ -242,10 +242,12 @@ extern int arch_init_kprobes(void); extern void show_registers(struct pt_regs *regs); extern void kprobes_inc_nmissed_count(struct kprobe *p); extern bool arch_within_kprobe_blacklist(unsigned long addr); +extern int arch_populate_kprobe_blacklist(void); extern bool arch_kprobe_on_func_entry(unsigned long offset); extern bool kprobe_on_func_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset); extern bool within_kprobe_blacklist(unsigned long addr); +extern int kprobe_add_ksym_blacklist(unsigned long entry); struct kprobe_insn_cache { struct mutex mutex; diff --git a/kernel/kprobes.c b/kernel/kprobes.c index 90e98e2..8b2eb62 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -2093,6 +2093,44 @@ void dump_kprobe(struct kprobe *kp) } NOKPROBE_SYMBOL(dump_kprobe); +int kprobe_add_ksym_blacklist(unsigned long entry) +{ + struct kprobe_blacklist_entry *ent; + unsigned long offset = 0, size = 0; + + if (!kernel_text_address(entry) || + !kallsyms_lookup_size_offset(entry, &size, &offset)) + return -EINVAL; + + ent = kmalloc(sizeof(*ent), GFP_KERNEL); + if (!ent) + return -ENOMEM; + ent->start_addr = entry - offset; + ent->end_addr = entry - offset + size; + INIT_LIST_HEAD(&ent->list); + list_add_tail(&ent->list, &kprobe_blacklist); + + return (int)size; +} + +/* Add functions in arch defined probe-prohibited area */ +int __weak arch_populate_kprobe_blacklist(void) +{ + unsigned long entry; + int ret = 0; + + for (entry = (unsigned long)__kprobes_text_start; + entry < (unsigned long)__kprobes_text_end; + entry += ret) { + ret = kprobe_add_ksym_blacklist(entry); + if (ret < 0) + return ret; + if (ret == 0) /* In case of alias symbol */ + ret = 1; + } + return 0; +} + /* * Lookup and populate the kprobe_blacklist. * @@ -2104,26 +2142,20 @@ NOKPROBE_SYMBOL(dump_kprobe); static int __init populate_kprobe_blacklist(unsigned long *start, unsigned long *end) { + unsigned long entry; unsigned long *iter; - struct kprobe_blacklist_entry *ent; - unsigned long entry, offset = 0, size = 0; + int ret; for (iter = start; iter < end; iter++) { entry = arch_deref_entry_point((void *)*iter); - - if (!kernel_text_address(entry) || - !kallsyms_lookup_size_offset(entry, &size, &offset)) + ret = kprobe_add_ksym_blacklist(entry); + if (ret == -EINVAL) continue; - - ent = kmalloc(sizeof(*ent), GFP_KERNEL); - if (!ent) - return -ENOMEM; - ent->start_addr = entry; - ent->end_addr = entry + size; - INIT_LIST_HEAD(&ent->list); - list_add_tail(&ent->list, &kprobe_blacklist); + if (ret < 0) + return ret; } - return 0; + + return arch_populate_kprobe_blacklist(); } /* Module notifier call back, checking kprobes on the module */ ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions 2018-12-07 16:01 ` Masami Hiramatsu @ 2018-12-07 17:00 ` Andrea Righi 2018-12-08 3:42 ` Masami Hiramatsu 2018-12-07 17:58 ` Andrea Righi 1 sibling, 1 reply; 11+ messages in thread From: Andrea Righi @ 2018-12-07 17:00 UTC (permalink / raw) To: Masami Hiramatsu Cc: Naveen N. Rao, Anil S Keshavamurthy, David S. Miller, Yonghong Song, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-kernel On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote: > Hi Andrea and Ingo, > > Here is the patch what I meant. I just ran it on qemu-x86, and seemed working. > After introducing this patch, I will start adding arch_populate_kprobe_blacklist() > to some arches. > > Thank you, > > [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area > > From: Masami Hiramatsu <mhiramat@kernel.org> > > Blacklist symbols in arch-defined probe-prohibited areas. > With this change, user can see all symbols which are prohibited > to probe in debugfs. > > All archtectures which have custom prohibit areas should define > its own arch_populate_kprobe_blacklist() function, but unless that, > all symbols marked __kprobes are blacklisted. > > Reported-by: Andrea Righi <righi.andrea@gmail.com> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- [snip] > +int kprobe_add_ksym_blacklist(unsigned long entry) > +{ > + struct kprobe_blacklist_entry *ent; > + unsigned long offset = 0, size = 0; > + > + if (!kernel_text_address(entry) || > + !kallsyms_lookup_size_offset(entry, &size, &offset)) > + return -EINVAL; > + > + ent = kmalloc(sizeof(*ent), GFP_KERNEL); > + if (!ent) > + return -ENOMEM; > + ent->start_addr = entry - offset; > + ent->end_addr = entry - offset + size; Do we need to take offset into account? The code before wasn't using it. > + INIT_LIST_HEAD(&ent->list); > + list_add_tail(&ent->list, &kprobe_blacklist); > + > + return (int)size; > +} > + > +/* Add functions in arch defined probe-prohibited area */ > +int __weak arch_populate_kprobe_blacklist(void) > +{ > + unsigned long entry; > + int ret = 0; > + > + for (entry = (unsigned long)__kprobes_text_start; > + entry < (unsigned long)__kprobes_text_end; > + entry += ret) { > + ret = kprobe_add_ksym_blacklist(entry); > + if (ret < 0) > + return ret; > + if (ret == 0) /* In case of alias symbol */ > + ret = 1; > + } > + return 0; > +} > + > /* > * Lookup and populate the kprobe_blacklist. > * > @@ -2104,26 +2142,20 @@ NOKPROBE_SYMBOL(dump_kprobe); > static int __init populate_kprobe_blacklist(unsigned long *start, > unsigned long *end) > { > + unsigned long entry; > unsigned long *iter; > - struct kprobe_blacklist_entry *ent; > - unsigned long entry, offset = 0, size = 0; > + int ret; > > for (iter = start; iter < end; iter++) { > entry = arch_deref_entry_point((void *)*iter); > - > - if (!kernel_text_address(entry) || > - !kallsyms_lookup_size_offset(entry, &size, &offset)) > + ret = kprobe_add_ksym_blacklist(entry); > + if (ret == -EINVAL) > continue; > - > - ent = kmalloc(sizeof(*ent), GFP_KERNEL); > - if (!ent) > - return -ENOMEM; > - ent->start_addr = entry; > - ent->end_addr = entry + size; > - INIT_LIST_HEAD(&ent->list); > - list_add_tail(&ent->list, &kprobe_blacklist); > + if (ret < 0) > + return ret; > } > - return 0; > + > + return arch_populate_kprobe_blacklist(); > } > > /* Module notifier call back, checking kprobes on the module */ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions 2018-12-07 17:00 ` Andrea Righi @ 2018-12-08 3:42 ` Masami Hiramatsu 2018-12-08 7:07 ` Andrea Righi 0 siblings, 1 reply; 11+ messages in thread From: Masami Hiramatsu @ 2018-12-08 3:42 UTC (permalink / raw) To: Andrea Righi Cc: Naveen N. Rao, Anil S Keshavamurthy, David S. Miller, Yonghong Song, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-kernel On Fri, 7 Dec 2018 18:00:26 +0100 Andrea Righi <righi.andrea@gmail.com> wrote: > On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote: > > Hi Andrea and Ingo, > > > > Here is the patch what I meant. I just ran it on qemu-x86, and seemed working. > > After introducing this patch, I will start adding arch_populate_kprobe_blacklist() > > to some arches. > > > > Thank you, > > > > [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area > > > > From: Masami Hiramatsu <mhiramat@kernel.org> > > > > Blacklist symbols in arch-defined probe-prohibited areas. > > With this change, user can see all symbols which are prohibited > > to probe in debugfs. > > > > All archtectures which have custom prohibit areas should define > > its own arch_populate_kprobe_blacklist() function, but unless that, > > all symbols marked __kprobes are blacklisted. > > > > Reported-by: Andrea Righi <righi.andrea@gmail.com> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > --- > > [snip] > > > +int kprobe_add_ksym_blacklist(unsigned long entry) > > +{ > > + struct kprobe_blacklist_entry *ent; > > + unsigned long offset = 0, size = 0; > > + > > + if (!kernel_text_address(entry) || > > + !kallsyms_lookup_size_offset(entry, &size, &offset)) > > + return -EINVAL; > > + > > + ent = kmalloc(sizeof(*ent), GFP_KERNEL); > > + if (!ent) > > + return -ENOMEM; > > + ent->start_addr = entry - offset; > > + ent->end_addr = entry - offset + size; > > Do we need to take offset into account? The code before wasn't using it. Yes, if we hit an alias symbol (zero-size), we forcibly increment address and retry it. In that case, offset will be 1. > > > + INIT_LIST_HEAD(&ent->list); > > + list_add_tail(&ent->list, &kprobe_blacklist); > > + > > + return (int)size; > > +} > > + > > +/* Add functions in arch defined probe-prohibited area */ > > +int __weak arch_populate_kprobe_blacklist(void) > > +{ > > + unsigned long entry; > > + int ret = 0; > > + > > + for (entry = (unsigned long)__kprobes_text_start; > > + entry < (unsigned long)__kprobes_text_end; > > + entry += ret) { > > + ret = kprobe_add_ksym_blacklist(entry); > > + if (ret < 0) > > + return ret; > > + if (ret == 0) /* In case of alias symbol */ > > + ret = 1; Here, we incremented. Thank you, > > + } > > + return 0; > > +} > > + > > /* > > * Lookup and populate the kprobe_blacklist. > > * > > @@ -2104,26 +2142,20 @@ NOKPROBE_SYMBOL(dump_kprobe); > > static int __init populate_kprobe_blacklist(unsigned long *start, > > unsigned long *end) > > { > > + unsigned long entry; > > unsigned long *iter; > > - struct kprobe_blacklist_entry *ent; > > - unsigned long entry, offset = 0, size = 0; > > + int ret; > > > > for (iter = start; iter < end; iter++) { > > entry = arch_deref_entry_point((void *)*iter); > > - > > - if (!kernel_text_address(entry) || > > - !kallsyms_lookup_size_offset(entry, &size, &offset)) > > + ret = kprobe_add_ksym_blacklist(entry); > > + if (ret == -EINVAL) > > continue; > > - > > - ent = kmalloc(sizeof(*ent), GFP_KERNEL); > > - if (!ent) > > - return -ENOMEM; > > - ent->start_addr = entry; > > - ent->end_addr = entry + size; > > - INIT_LIST_HEAD(&ent->list); > > - list_add_tail(&ent->list, &kprobe_blacklist); > > + if (ret < 0) > > + return ret; > > } > > - return 0; > > + > > + return arch_populate_kprobe_blacklist(); > > } > > > > /* Module notifier call back, checking kprobes on the module */ -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions 2018-12-08 3:42 ` Masami Hiramatsu @ 2018-12-08 7:07 ` Andrea Righi 0 siblings, 0 replies; 11+ messages in thread From: Andrea Righi @ 2018-12-08 7:07 UTC (permalink / raw) To: Masami Hiramatsu Cc: Naveen N. Rao, Anil S Keshavamurthy, David S. Miller, Yonghong Song, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-kernel On Sat, Dec 08, 2018 at 12:42:10PM +0900, Masami Hiramatsu wrote: > On Fri, 7 Dec 2018 18:00:26 +0100 > Andrea Righi <righi.andrea@gmail.com> wrote: > > > On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote: > > > Hi Andrea and Ingo, > > > > > > Here is the patch what I meant. I just ran it on qemu-x86, and seemed working. > > > After introducing this patch, I will start adding arch_populate_kprobe_blacklist() > > > to some arches. > > > > > > Thank you, > > > > > > [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area > > > > > > From: Masami Hiramatsu <mhiramat@kernel.org> > > > > > > Blacklist symbols in arch-defined probe-prohibited areas. > > > With this change, user can see all symbols which are prohibited > > > to probe in debugfs. > > > > > > All archtectures which have custom prohibit areas should define > > > its own arch_populate_kprobe_blacklist() function, but unless that, > > > all symbols marked __kprobes are blacklisted. > > > > > > Reported-by: Andrea Righi <righi.andrea@gmail.com> > > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > > --- > > > > [snip] > > > > > +int kprobe_add_ksym_blacklist(unsigned long entry) > > > +{ > > > + struct kprobe_blacklist_entry *ent; > > > + unsigned long offset = 0, size = 0; > > > + > > > + if (!kernel_text_address(entry) || > > > + !kallsyms_lookup_size_offset(entry, &size, &offset)) > > > + return -EINVAL; > > > + > > > + ent = kmalloc(sizeof(*ent), GFP_KERNEL); > > > + if (!ent) > > > + return -ENOMEM; > > > + ent->start_addr = entry - offset; > > > + ent->end_addr = entry - offset + size; > > > > Do we need to take offset into account? The code before wasn't using it. > > Yes, if we hit an alias symbol (zero-size), we forcibly increment address > and retry it. In that case, offset will be 1. > > > > > > + INIT_LIST_HEAD(&ent->list); > > > + list_add_tail(&ent->list, &kprobe_blacklist); > > > + > > > + return (int)size; > > > +} > > > + > > > +/* Add functions in arch defined probe-prohibited area */ > > > +int __weak arch_populate_kprobe_blacklist(void) > > > +{ > > > + unsigned long entry; > > > + int ret = 0; > > > + > > > + for (entry = (unsigned long)__kprobes_text_start; > > > + entry < (unsigned long)__kprobes_text_end; > > > + entry += ret) { > > > + ret = kprobe_add_ksym_blacklist(entry); > > > + if (ret < 0) > > > + return ret; > > > + if (ret == 0) /* In case of alias symbol */ > > > + ret = 1; > > Here, we incremented. > > Thank you, Makes sense, thanks for the clarification. -Andrea ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions 2018-12-07 16:01 ` Masami Hiramatsu 2018-12-07 17:00 ` Andrea Righi @ 2018-12-07 17:58 ` Andrea Righi 2018-12-08 3:48 ` Masami Hiramatsu 1 sibling, 1 reply; 11+ messages in thread From: Andrea Righi @ 2018-12-07 17:58 UTC (permalink / raw) To: Masami Hiramatsu Cc: Naveen N. Rao, Anil S Keshavamurthy, David S. Miller, Yonghong Song, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-kernel On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote: > Hi Andrea and Ingo, > > Here is the patch what I meant. I just ran it on qemu-x86, and seemed working. > After introducing this patch, I will start adding arch_populate_kprobe_blacklist() > to some arches. > > Thank you, > > [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area > > From: Masami Hiramatsu <mhiramat@kernel.org> > > Blacklist symbols in arch-defined probe-prohibited areas. > With this change, user can see all symbols which are prohibited > to probe in debugfs. > > All archtectures which have custom prohibit areas should define > its own arch_populate_kprobe_blacklist() function, but unless that, > all symbols marked __kprobes are blacklisted. What about iterating all symbols and use arch_within_kprobe_blacklist() to check if we need to blacklist them or not. In this way we don't have to introduce an arch_populate_kprobe_blacklist() for each architecture. Something like the following maybe. Thanks. [RFC] kprobes: blacklist all symbols in arch-defined prohibited area From: Andrea Righi <righi.andrea@gmail.com> Blacklist symbols in arch-defined probe-prohibited areas. With this change, user can see all symbols which are prohibited to probe in debugfs. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> Signed-off-by: Andrea Righi <righi.andrea@gmail.com> --- kernel/kprobes.c | 55 +++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 14 deletions(-) diff --git a/kernel/kprobes.c b/kernel/kprobes.c index 90e98e233647..e67598dd7468 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -2093,6 +2093,35 @@ void dump_kprobe(struct kprobe *kp) } NOKPROBE_SYMBOL(dump_kprobe); +static int kprobe_blacklist_add(unsigned long entry) +{ + struct kprobe_blacklist_entry *ent; + unsigned long offset = 0, size = 0; + + if (!kernel_text_address(entry) || + !kallsyms_lookup_size_offset(entry, &size, &offset)) + return -EINVAL; + + ent = kmalloc(sizeof(*ent), GFP_KERNEL); + if (!ent) + return -ENOMEM; + ent->start_addr = entry; + ent->end_addr = entry + size; + INIT_LIST_HEAD(&ent->list); + list_add_tail(&ent->list, &kprobe_blacklist); + + return 0; +} + +static int arch_populate_kprobe_blacklist(void *data, const char *name, + struct module *mod, + unsigned long entry) +{ + if (arch_within_kprobe_blacklist(entry)) + kprobe_blacklist_add(entry); + return 0; +} + /* * Lookup and populate the kprobe_blacklist. * @@ -2104,24 +2133,22 @@ NOKPROBE_SYMBOL(dump_kprobe); static int __init populate_kprobe_blacklist(unsigned long *start, unsigned long *end) { - unsigned long *iter; - struct kprobe_blacklist_entry *ent; - unsigned long entry, offset = 0, size = 0; + unsigned long entry, *iter; + int ret; + /* Blacklist all arch_within_kprobe_blacklist() symbols */ + mutex_lock(&module_mutex); + kallsyms_on_each_symbol(arch_populate_kprobe_blacklist, NULL); + mutex_unlock(&module_mutex); + + /* Add explicitly blacklisted symbols */ for (iter = start; iter < end; iter++) { entry = arch_deref_entry_point((void *)*iter); - - if (!kernel_text_address(entry) || - !kallsyms_lookup_size_offset(entry, &size, &offset)) + ret = kprobe_blacklist_add(entry); + if (ret == -EINVAL) continue; - - ent = kmalloc(sizeof(*ent), GFP_KERNEL); - if (!ent) - return -ENOMEM; - ent->start_addr = entry; - ent->end_addr = entry + size; - INIT_LIST_HEAD(&ent->list); - list_add_tail(&ent->list, &kprobe_blacklist); + if (ret < 0) + return ret; } return 0; } ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions 2018-12-07 17:58 ` Andrea Righi @ 2018-12-08 3:48 ` Masami Hiramatsu 2018-12-08 7:09 ` Andrea Righi 0 siblings, 1 reply; 11+ messages in thread From: Masami Hiramatsu @ 2018-12-08 3:48 UTC (permalink / raw) To: Andrea Righi Cc: Naveen N. Rao, Anil S Keshavamurthy, David S. Miller, Yonghong Song, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-kernel On Fri, 7 Dec 2018 18:58:05 +0100 Andrea Righi <righi.andrea@gmail.com> wrote: > On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote: > > Hi Andrea and Ingo, > > > > Here is the patch what I meant. I just ran it on qemu-x86, and seemed working. > > After introducing this patch, I will start adding arch_populate_kprobe_blacklist() > > to some arches. > > > > Thank you, > > > > [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area > > > > From: Masami Hiramatsu <mhiramat@kernel.org> > > > > Blacklist symbols in arch-defined probe-prohibited areas. > > With this change, user can see all symbols which are prohibited > > to probe in debugfs. > > > > All archtectures which have custom prohibit areas should define > > its own arch_populate_kprobe_blacklist() function, but unless that, > > all symbols marked __kprobes are blacklisted. > > What about iterating all symbols and use arch_within_kprobe_blacklist() > to check if we need to blacklist them or not. Sorry, I don't want to iterate all ksyms since it may take a long time (especially embedded small devices.) > > In this way we don't have to introduce an > arch_populate_kprobe_blacklist() for each architecture. Hmm, I had a same idea, but there are some arch which prohibit probing extable entries (e.g. arm64.) For correctness of the blacklist, I think it should be listed (not entire the function body). I also rather like to remove arch_within_kprobe_blacklist() instead. Thank you, > > Something like the following maybe. > > Thanks. > > [RFC] kprobes: blacklist all symbols in arch-defined prohibited area > > From: Andrea Righi <righi.andrea@gmail.com> > > Blacklist symbols in arch-defined probe-prohibited areas. > With this change, user can see all symbols which are prohibited > to probe in debugfs. > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > Signed-off-by: Andrea Righi <righi.andrea@gmail.com> > --- > kernel/kprobes.c | 55 +++++++++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 41 insertions(+), 14 deletions(-) > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index 90e98e233647..e67598dd7468 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -2093,6 +2093,35 @@ void dump_kprobe(struct kprobe *kp) > } > NOKPROBE_SYMBOL(dump_kprobe); > > +static int kprobe_blacklist_add(unsigned long entry) > +{ > + struct kprobe_blacklist_entry *ent; > + unsigned long offset = 0, size = 0; > + > + if (!kernel_text_address(entry) || > + !kallsyms_lookup_size_offset(entry, &size, &offset)) > + return -EINVAL; > + > + ent = kmalloc(sizeof(*ent), GFP_KERNEL); > + if (!ent) > + return -ENOMEM; > + ent->start_addr = entry; > + ent->end_addr = entry + size; > + INIT_LIST_HEAD(&ent->list); > + list_add_tail(&ent->list, &kprobe_blacklist); > + > + return 0; > +} > + > +static int arch_populate_kprobe_blacklist(void *data, const char *name, > + struct module *mod, > + unsigned long entry) > +{ > + if (arch_within_kprobe_blacklist(entry)) > + kprobe_blacklist_add(entry); > + return 0; > +} > + > /* > * Lookup and populate the kprobe_blacklist. > * > @@ -2104,24 +2133,22 @@ NOKPROBE_SYMBOL(dump_kprobe); > static int __init populate_kprobe_blacklist(unsigned long *start, > unsigned long *end) > { > - unsigned long *iter; > - struct kprobe_blacklist_entry *ent; > - unsigned long entry, offset = 0, size = 0; > + unsigned long entry, *iter; > + int ret; > > + /* Blacklist all arch_within_kprobe_blacklist() symbols */ > + mutex_lock(&module_mutex); > + kallsyms_on_each_symbol(arch_populate_kprobe_blacklist, NULL); > + mutex_unlock(&module_mutex); > + > + /* Add explicitly blacklisted symbols */ > for (iter = start; iter < end; iter++) { > entry = arch_deref_entry_point((void *)*iter); > - > - if (!kernel_text_address(entry) || > - !kallsyms_lookup_size_offset(entry, &size, &offset)) > + ret = kprobe_blacklist_add(entry); > + if (ret == -EINVAL) > continue; > - > - ent = kmalloc(sizeof(*ent), GFP_KERNEL); > - if (!ent) > - return -ENOMEM; > - ent->start_addr = entry; > - ent->end_addr = entry + size; > - INIT_LIST_HEAD(&ent->list); > - list_add_tail(&ent->list, &kprobe_blacklist); > + if (ret < 0) > + return ret; > } > return 0; > } -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions 2018-12-08 3:48 ` Masami Hiramatsu @ 2018-12-08 7:09 ` Andrea Righi 2018-12-16 16:07 ` Masami Hiramatsu 0 siblings, 1 reply; 11+ messages in thread From: Andrea Righi @ 2018-12-08 7:09 UTC (permalink / raw) To: Masami Hiramatsu Cc: Naveen N. Rao, Anil S Keshavamurthy, David S. Miller, Yonghong Song, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-kernel On Sat, Dec 08, 2018 at 12:48:59PM +0900, Masami Hiramatsu wrote: > On Fri, 7 Dec 2018 18:58:05 +0100 > Andrea Righi <righi.andrea@gmail.com> wrote: > > > On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote: > > > Hi Andrea and Ingo, > > > > > > Here is the patch what I meant. I just ran it on qemu-x86, and seemed working. > > > After introducing this patch, I will start adding arch_populate_kprobe_blacklist() > > > to some arches. > > > > > > Thank you, > > > > > > [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area > > > > > > From: Masami Hiramatsu <mhiramat@kernel.org> > > > > > > Blacklist symbols in arch-defined probe-prohibited areas. > > > With this change, user can see all symbols which are prohibited > > > to probe in debugfs. > > > > > > All archtectures which have custom prohibit areas should define > > > its own arch_populate_kprobe_blacklist() function, but unless that, > > > all symbols marked __kprobes are blacklisted. > > > > What about iterating all symbols and use arch_within_kprobe_blacklist() > > to check if we need to blacklist them or not. > > Sorry, I don't want to iterate all ksyms since it may take a long time > (especially embedded small devices.) > > > > > In this way we don't have to introduce an > > arch_populate_kprobe_blacklist() for each architecture. > > Hmm, I had a same idea, but there are some arch which prohibit probing > extable entries (e.g. arm64.) For correctness of the blacklist, I think > it should be listed (not entire the function body). > I also rather like to remove arch_within_kprobe_blacklist() instead. OK. Thanks. -Andrea ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions 2018-12-08 7:09 ` Andrea Righi @ 2018-12-16 16:07 ` Masami Hiramatsu 0 siblings, 0 replies; 11+ messages in thread From: Masami Hiramatsu @ 2018-12-16 16:07 UTC (permalink / raw) To: Andrea Righi Cc: Naveen N. Rao, Anil S Keshavamurthy, David S. Miller, Yonghong Song, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-kernel On Sat, 8 Dec 2018 08:09:22 +0100 Andrea Righi <righi.andrea@gmail.com> wrote: > On Sat, Dec 08, 2018 at 12:48:59PM +0900, Masami Hiramatsu wrote: > > On Fri, 7 Dec 2018 18:58:05 +0100 > > Andrea Righi <righi.andrea@gmail.com> wrote: > > > > > On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote: > > > > Hi Andrea and Ingo, > > > > > > > > Here is the patch what I meant. I just ran it on qemu-x86, and seemed working. > > > > After introducing this patch, I will start adding arch_populate_kprobe_blacklist() > > > > to some arches. > > > > > > > > Thank you, > > > > > > > > [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area > > > > > > > > From: Masami Hiramatsu <mhiramat@kernel.org> > > > > > > > > Blacklist symbols in arch-defined probe-prohibited areas. > > > > With this change, user can see all symbols which are prohibited > > > > to probe in debugfs. > > > > > > > > All archtectures which have custom prohibit areas should define > > > > its own arch_populate_kprobe_blacklist() function, but unless that, > > > > all symbols marked __kprobes are blacklisted. > > > > > > What about iterating all symbols and use arch_within_kprobe_blacklist() > > > to check if we need to blacklist them or not. > > > > Sorry, I don't want to iterate all ksyms since it may take a long time > > (especially embedded small devices.) > > > > > > > > In this way we don't have to introduce an > > > arch_populate_kprobe_blacklist() for each architecture. > > > > Hmm, I had a same idea, but there are some arch which prohibit probing > > extable entries (e.g. arm64.) For correctness of the blacklist, I think > > it should be listed (not entire the function body). Aah, my bad memory. I remember that the extable should not be in blacklist, since it is a kind of "unsupported instruction to be probed" on some arch. I'll move that in arch_prepare_kprobe() on arm64. > > I also rather like to remove arch_within_kprobe_blacklist() instead. Anyway, this is true. If I could make a complete list, I will remove this function. Thank you, > > OK. Thanks. > > -Andrea -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-12-16 16:07 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-06 9:56 [PATCH] kprobes: x86_64: blacklist non-attachable interrupt functions Andrea Righi 2018-12-06 16:39 ` [tip:perf/urgent] kprobes/x86: Blacklist " tip-bot for Andrea Righi 2018-12-07 14:47 ` [PATCH] kprobes: x86_64: blacklist " Masami Hiramatsu 2018-12-07 16:01 ` Masami Hiramatsu 2018-12-07 17:00 ` Andrea Righi 2018-12-08 3:42 ` Masami Hiramatsu 2018-12-08 7:07 ` Andrea Righi 2018-12-07 17:58 ` Andrea Righi 2018-12-08 3:48 ` Masami Hiramatsu 2018-12-08 7:09 ` Andrea Righi 2018-12-16 16:07 ` Masami Hiramatsu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).