Hi Naveen, On Thu, 29 Jun 2017 19:38:37 +0530 "Naveen N. Rao" <[email protected]> wrote:
> Currently, the kprobe blacklist file in debugfs (usually > /sys/kernel/debug/kprobes/blacklist) does not capture all address > ranges that are blacklisted. For instance, functions annotated with > __kprobes (between __kprobes_text_start and __kprobes_text_end) are > blacklisted on all architectures. Certain architectures have additional > address ranges that are blacklisted. Ah, I see. The problem is that the blacklist is not populated for arch-dependent areas. > > These address ranges are currently prevented from being kprobe'd by > arch_within_kprobe_blacklist(). Instead, add the relevant address ranges > to the main kprobe blacklist allowing such address ranges to be > discovered by userspace. > > With this patch, on powerpc64le, we see 2 additional entries at the end > of the blacklist: > root@ubuntu:/sys/kernel/debug# tail kprobes/blacklist > 0xc00000000022d500-0xc00000000022d580 print_type_s64 > 0xc00000000022d480-0xc00000000022d500 print_type_s32 > 0xc00000000022d400-0xc00000000022d480 print_type_s16 > 0xc00000000022d380-0xc00000000022d400 print_type_s8 > 0xc00000000022d300-0xc00000000022d380 print_type_u64 > 0xc00000000022d280-0xc00000000022d300 print_type_u32 > 0xc00000000022d200-0xc00000000022d280 print_type_u16 > 0xc00000000022d180-0xc00000000022d200 print_type_u8 > 0xc0000000009ed778-0xc0000000009ed778 __irqentry_text_end > 0xc000000000000000-0xc000000000008000 start_first_256B > > __irqentry_text_end() happens to be at __kprobes_text_start and hence > figures there. Ditto with start_first_256B(). A subsequent patch helps > provide meaningful names for such address ranges. > OK, looks good to me :) Acked-by: Masami Hiramatsu <[email protected]> Thank you! > Signed-off-by: Naveen N. Rao <[email protected]> > --- > arch/arm64/kernel/probes/kprobes.c | 31 +++++++++--------- > arch/powerpc/kernel/kprobes.c | 10 +++--- > arch/x86/kernel/kprobes/core.c | 10 +++--- > include/linux/kprobes.h | 2 ++ > kernel/kprobes.c | 64 > ++++++++++++++++++++++---------------- > 5 files changed, 66 insertions(+), 51 deletions(-) > > diff --git a/arch/arm64/kernel/probes/kprobes.c > b/arch/arm64/kernel/probes/kprobes.c > index c5c45942fb6e..7f453a71b5a8 100644 > --- a/arch/arm64/kernel/probes/kprobes.c > +++ b/arch/arm64/kernel/probes/kprobes.c > @@ -535,24 +535,27 @@ int __kprobes longjmp_break_handler(struct kprobe *p, > struct pt_regs *regs) > > bool arch_within_kprobe_blacklist(unsigned long addr) > { > - if ((addr >= (unsigned long)__kprobes_text_start && > - addr < (unsigned long)__kprobes_text_end) || > - (addr >= (unsigned long)__entry_text_start && > - addr < (unsigned long)__entry_text_end) || > - (addr >= (unsigned long)__idmap_text_start && > - addr < (unsigned long)__idmap_text_end) || > - !!search_exception_tables(addr)) > + if (!!search_exception_tables(addr)) > return true; > > + return false; > +} > + > +void __init arch_populate_kprobe_blacklist(void) > +{ > + insert_kprobe_blacklist((unsigned long)__kprobes_text_start, > + (unsigned long)__kprobes_text_end); > + insert_kprobe_blacklist((unsigned long)__entry_text_start, > + (unsigned long)__entry_text_end); > + insert_kprobe_blacklist((unsigned long)__idmap_text_start, > + (unsigned long)__idmap_text_end); > + > if (!is_kernel_in_hyp_mode()) { > - if ((addr >= (unsigned long)__hyp_text_start && > - addr < (unsigned long)__hyp_text_end) || > - (addr >= (unsigned long)__hyp_idmap_text_start && > - addr < (unsigned long)__hyp_idmap_text_end)) > - return true; > + insert_kprobe_blacklist((unsigned long)__hyp_text_start, > + (unsigned long)__hyp_text_end); > + insert_kprobe_blacklist((unsigned long)__hyp_idmap_text_start, > + (unsigned long)__hyp_idmap_text_end); > } > - > - return false; > } > > void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs) > diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c > index 01addfb0ed0a..f707e42b3482 100644 > --- a/arch/powerpc/kernel/kprobes.c > +++ b/arch/powerpc/kernel/kprobes.c > @@ -49,12 +49,12 @@ int is_current_kprobe_addr(unsigned long addr) > return (p && (unsigned long)p->addr == addr) ? 1 : 0; > } > > -bool arch_within_kprobe_blacklist(unsigned long addr) > +void __init arch_populate_kprobe_blacklist(void) > { > - return (addr >= (unsigned long)__kprobes_text_start && > - addr < (unsigned long)__kprobes_text_end) || > - (addr >= (unsigned long)_stext && > - addr < (unsigned long)__head_end); > + insert_kprobe_blacklist((unsigned long)__kprobes_text_start, > + (unsigned long)__kprobes_text_end); > + insert_kprobe_blacklist((unsigned long)_stext, > + (unsigned long)__head_end); > } > > kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset) > diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c > index 6b877807598b..4a5fc04b3e92 100644 > --- a/arch/x86/kernel/kprobes/core.c > +++ b/arch/x86/kernel/kprobes/core.c > @@ -1141,12 +1141,12 @@ int longjmp_break_handler(struct kprobe *p, struct > pt_regs *regs) > } > NOKPROBE_SYMBOL(longjmp_break_handler); > > -bool arch_within_kprobe_blacklist(unsigned long addr) > +void __init arch_populate_kprobe_blacklist(void) > { > - return (addr >= (unsigned long)__kprobes_text_start && > - addr < (unsigned long)__kprobes_text_end) || > - (addr >= (unsigned long)__entry_text_start && > - addr < (unsigned long)__entry_text_end); > + insert_kprobe_blacklist((unsigned long)__kprobes_text_start, > + (unsigned long)__kprobes_text_end); > + insert_kprobe_blacklist((unsigned long)__entry_text_start, > + (unsigned long)__entry_text_end); > } > > int __init arch_init_kprobes(void) > diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h > index 541df0b5b815..6d931ea0604e 100644 > --- a/include/linux/kprobes.h > +++ b/include/linux/kprobes.h > @@ -271,6 +271,8 @@ extern bool arch_function_offset_within_entry(unsigned > long offset); > extern bool function_offset_within_entry(kprobe_opcode_t *addr, const char > *sym, unsigned long offset); > > extern bool within_kprobe_blacklist(unsigned long addr); > +extern void insert_kprobe_blacklist(unsigned long start, unsigned long end); > +extern void arch_populate_kprobe_blacklist(void); > > struct kprobe_insn_cache { > struct mutex mutex; > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index adfe3b4cfe05..8ddbee01ce01 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -1364,12 +1364,7 @@ static int register_aggr_kprobe(struct kprobe *orig_p, > struct kprobe *p) > return ret; > } > > -bool __weak arch_within_kprobe_blacklist(unsigned long addr) > -{ > - /* The __kprobes marked functions and entry code must not be probed */ > - return addr >= (unsigned long)__kprobes_text_start && > - addr < (unsigned long)__kprobes_text_end; > -} > +bool __weak arch_within_kprobe_blacklist(unsigned long addr) { return false; > } > > bool within_kprobe_blacklist(unsigned long addr) > { > @@ -2125,12 +2120,40 @@ NOKPROBE_SYMBOL(dump_kprobe); > * since a kprobe need not necessarily be at the beginning > * of a function. > */ > -static int __init populate_kprobe_blacklist(unsigned long *start, > - unsigned long *end) > + > +/* Markers of _kprobe_blacklist section */ > +extern unsigned long __start_kprobe_blacklist[]; > +extern unsigned long __stop_kprobe_blacklist[]; > + > +void __init insert_kprobe_blacklist(unsigned long start, unsigned long end) > { > - unsigned long *iter; > struct kprobe_blacklist_entry *ent; > + > + ent = kmalloc(sizeof(*ent), GFP_KERNEL); > + if (!ent) { > + pr_err_once("kprobes: failed to populate blacklist: %d\n", > -ENOMEM); > + pr_err_once("Please take care of using kprobes.\n"); > + } > + > + ent->start_addr = start; > + ent->end_addr = end; > + INIT_LIST_HEAD(&ent->list); > + list_add_tail(&ent->list, &kprobe_blacklist); > +} > + > +void __weak __init arch_populate_kprobe_blacklist(void) > +{ > + /* The __kprobes marked functions and entry code must not be probed */ > + insert_kprobe_blacklist((unsigned long)__kprobes_text_start, > + (unsigned long)__kprobes_text_end); > +} > + > +static void __init populate_kprobe_blacklist(void) > +{ > + unsigned long *iter; > unsigned long entry, offset = 0, size = 0; > + unsigned long *start = __start_kprobe_blacklist; > + unsigned long *end = __stop_kprobe_blacklist; > > for (iter = start; iter < end; iter++) { > entry = arch_deref_entry_point((void *)*iter); > @@ -2142,15 +2165,11 @@ static int __init populate_kprobe_blacklist(unsigned > long *start, > 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); > + insert_kprobe_blacklist(entry, entry + size); > } > - return 0; > + > + /* Let architectures add their own entries as well */ > + arch_populate_kprobe_blacklist(); > } > > /* Module notifier call back, checking kprobes on the module */ > @@ -2202,10 +2221,6 @@ static struct notifier_block kprobe_module_nb = { > .priority = 0 > }; > > -/* Markers of _kprobe_blacklist section */ > -extern unsigned long __start_kprobe_blacklist[]; > -extern unsigned long __stop_kprobe_blacklist[]; > - > static int __init init_kprobes(void) > { > int i, err = 0; > @@ -2218,12 +2233,7 @@ static int __init init_kprobes(void) > raw_spin_lock_init(&(kretprobe_table_locks[i].lock)); > } > > - err = populate_kprobe_blacklist(__start_kprobe_blacklist, > - __stop_kprobe_blacklist); > - if (err) { > - pr_err("kprobes: failed to populate blacklist: %d\n", err); > - pr_err("Please take care of using kprobes.\n"); > - } > + populate_kprobe_blacklist(); > > if (kretprobe_blacklist_size) { > /* lookup the function address from its name */ > -- > 2.13.1 > -- Masami Hiramatsu <[email protected]>
