| Message ID | 1511385814-20863-2-git-send-email-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 22 Nov 2017 16:23:30 -0500 Josef Bacik <josef@toxicpanda.com> wrote: > From: Josef Bacik <jbacik@fb.com> > > Using BPF we can override kprob'ed functions and return arbitrary > values. Obviously this can be a bit unsafe, so make this feature opt-in > for functions. Simply tag a function with KPROBE_ERROR_INJECT_SYMBOL in > order to give BPF access to that function for error injection purposes. > > Signed-off-by: Josef Bacik <jbacik@fb.com> > Acked-by: Ingo Molnar <mingo@kernel.org> > --- > arch/x86/include/asm/asm.h | 6 ++ > include/asm-generic/vmlinux.lds.h | 10 +++ > include/linux/bpf.h | 11 +++ > include/linux/kprobes.h | 1 + > include/linux/module.h | 5 ++ > kernel/kprobes.c | 163 ++++++++++++++++++++++++++++++++++++++ > kernel/module.c | 6 +- > 7 files changed, 201 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h > index b0dc91f4bedc..340f4cc43255 100644 > --- a/arch/x86/include/asm/asm.h > +++ b/arch/x86/include/asm/asm.h > @@ -85,6 +85,12 @@ > _ASM_PTR (entry); \ > .popsection > > +# define _ASM_KPROBE_ERROR_INJECT(entry) \ > + .pushsection "_kprobe_error_inject_list","aw" ; \ > + _ASM_ALIGN ; \ > + _ASM_PTR (entry); \ > + .popseciton So this stuff is not my area of greatest expertise, but I do have to wonder how ".popseciton" can work ... ? Thanks, jon -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 28, 2017 at 11:58:41AM -0700, Jonathan Corbet wrote: > On Wed, 22 Nov 2017 16:23:30 -0500 > Josef Bacik <josef@toxicpanda.com> wrote: > > > From: Josef Bacik <jbacik@fb.com> > > > > Using BPF we can override kprob'ed functions and return arbitrary > > values. Obviously this can be a bit unsafe, so make this feature opt-in > > for functions. Simply tag a function with KPROBE_ERROR_INJECT_SYMBOL in > > order to give BPF access to that function for error injection purposes. > > > > Signed-off-by: Josef Bacik <jbacik@fb.com> > > Acked-by: Ingo Molnar <mingo@kernel.org> > > --- > > arch/x86/include/asm/asm.h | 6 ++ > > include/asm-generic/vmlinux.lds.h | 10 +++ > > include/linux/bpf.h | 11 +++ > > include/linux/kprobes.h | 1 + > > include/linux/module.h | 5 ++ > > kernel/kprobes.c | 163 ++++++++++++++++++++++++++++++++++++++ > > kernel/module.c | 6 +- > > 7 files changed, 201 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h > > index b0dc91f4bedc..340f4cc43255 100644 > > --- a/arch/x86/include/asm/asm.h > > +++ b/arch/x86/include/asm/asm.h > > @@ -85,6 +85,12 @@ > > _ASM_PTR (entry); \ > > .popsection > > > > +# define _ASM_KPROBE_ERROR_INJECT(entry) \ > > + .pushsection "_kprobe_error_inject_list","aw" ; \ > > + _ASM_ALIGN ; \ > > + _ASM_PTR (entry); \ > > + .popseciton > > So this stuff is not my area of greatest expertise, but I do have to wonder > how ".popseciton" can work ... ? > Well fuck, do you want me to send a increment Daniel/Alexei or resend this patch fixed? Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/28/2017 09:02 PM, Josef Bacik wrote: > On Tue, Nov 28, 2017 at 11:58:41AM -0700, Jonathan Corbet wrote: >> On Wed, 22 Nov 2017 16:23:30 -0500 >> Josef Bacik <josef@toxicpanda.com> wrote: >>> From: Josef Bacik <jbacik@fb.com> >>> >>> Using BPF we can override kprob'ed functions and return arbitrary >>> values. Obviously this can be a bit unsafe, so make this feature opt-in >>> for functions. Simply tag a function with KPROBE_ERROR_INJECT_SYMBOL in >>> order to give BPF access to that function for error injection purposes. >>> >>> Signed-off-by: Josef Bacik <jbacik@fb.com> >>> Acked-by: Ingo Molnar <mingo@kernel.org> >>> --- >>> arch/x86/include/asm/asm.h | 6 ++ >>> include/asm-generic/vmlinux.lds.h | 10 +++ >>> include/linux/bpf.h | 11 +++ >>> include/linux/kprobes.h | 1 + >>> include/linux/module.h | 5 ++ >>> kernel/kprobes.c | 163 ++++++++++++++++++++++++++++++++++++++ >>> kernel/module.c | 6 +- >>> 7 files changed, 201 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h >>> index b0dc91f4bedc..340f4cc43255 100644 >>> --- a/arch/x86/include/asm/asm.h >>> +++ b/arch/x86/include/asm/asm.h >>> @@ -85,6 +85,12 @@ >>> _ASM_PTR (entry); \ >>> .popsection >>> >>> +# define _ASM_KPROBE_ERROR_INJECT(entry) \ >>> + .pushsection "_kprobe_error_inject_list","aw" ; \ >>> + _ASM_ALIGN ; \ >>> + _ASM_PTR (entry); \ >>> + .popseciton >> >> So this stuff is not my area of greatest expertise, but I do have to wonder >> how ".popseciton" can work ... ? > > Well fuck, do you want me to send a increment Daniel/Alexei or resend this patch > fixed? Thanks, Sorry for late reply, please rebase + respin the whole series with this fixed. There were also few typos in the cover letter / commit messages that would be good to get fixed along the way. Also, could you debug why this wasn't caught at compile/runtime during testing? Thanks a lot, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 29, 2017 at 05:59:39PM +0100, Daniel Borkmann wrote: > On 11/28/2017 09:02 PM, Josef Bacik wrote: > > On Tue, Nov 28, 2017 at 11:58:41AM -0700, Jonathan Corbet wrote: > >> On Wed, 22 Nov 2017 16:23:30 -0500 > >> Josef Bacik <josef@toxicpanda.com> wrote: > >>> From: Josef Bacik <jbacik@fb.com> > >>> > >>> Using BPF we can override kprob'ed functions and return arbitrary > >>> values. Obviously this can be a bit unsafe, so make this feature opt-in > >>> for functions. Simply tag a function with KPROBE_ERROR_INJECT_SYMBOL in > >>> order to give BPF access to that function for error injection purposes. > >>> > >>> Signed-off-by: Josef Bacik <jbacik@fb.com> > >>> Acked-by: Ingo Molnar <mingo@kernel.org> > >>> --- > >>> arch/x86/include/asm/asm.h | 6 ++ > >>> include/asm-generic/vmlinux.lds.h | 10 +++ > >>> include/linux/bpf.h | 11 +++ > >>> include/linux/kprobes.h | 1 + > >>> include/linux/module.h | 5 ++ > >>> kernel/kprobes.c | 163 ++++++++++++++++++++++++++++++++++++++ > >>> kernel/module.c | 6 +- > >>> 7 files changed, 201 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h > >>> index b0dc91f4bedc..340f4cc43255 100644 > >>> --- a/arch/x86/include/asm/asm.h > >>> +++ b/arch/x86/include/asm/asm.h > >>> @@ -85,6 +85,12 @@ > >>> _ASM_PTR (entry); \ > >>> .popsection > >>> > >>> +# define _ASM_KPROBE_ERROR_INJECT(entry) \ > >>> + .pushsection "_kprobe_error_inject_list","aw" ; \ > >>> + _ASM_ALIGN ; \ > >>> + _ASM_PTR (entry); \ > >>> + .popseciton > >> > >> So this stuff is not my area of greatest expertise, but I do have to wonder > >> how ".popseciton" can work ... ? > > > > Well fuck, do you want me to send a increment Daniel/Alexei or resend this patch > > fixed? Thanks, > > Sorry for late reply, please rebase + respin the whole series with > this fixed. There were also few typos in the cover letter / commit > messages that would be good to get fixed along the way. > > Also, could you debug why this wasn't caught at compile/runtime during > testing? > Sat down to figure out what was wrong here, and realized I'm just an idiot. I was copying the no kprobe stuff, and my grepping did not uncover what _ASM_NOKPROBE() was used for, so I assumed it was some auto generated magic and just copied what it did to cover my bases. Sat down to figure it out and it is actually called in some assembly files (which is why cscope didn't find it). So we don't need _ASM_KPROBE_ERROR_INJECT at all. I'll drop it and respin and send it along. Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h index b0dc91f4bedc..340f4cc43255 100644 --- a/arch/x86/include/asm/asm.h +++ b/arch/x86/include/asm/asm.h @@ -85,6 +85,12 @@ _ASM_PTR (entry); \ .popsection +# define _ASM_KPROBE_ERROR_INJECT(entry) \ + .pushsection "_kprobe_error_inject_list","aw" ; \ + _ASM_ALIGN ; \ + _ASM_PTR (entry); \ + .popseciton + .macro ALIGN_DESTINATION /* check for bad alignment of destination */ movl %edi,%ecx diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 8acfc1e099e1..85822804861e 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -136,6 +136,15 @@ #define KPROBE_BLACKLIST() #endif +#ifdef CONFIG_BPF_KPROBE_OVERRIDE +#define ERROR_INJECT_LIST() . = ALIGN(8); \ + VMLINUX_SYMBOL(__start_kprobe_error_inject_list) = .; \ + KEEP(*(_kprobe_error_inject_list)) \ + VMLINUX_SYMBOL(__stop_kprobe_error_inject_list) = .; +#else +#define ERROR_INJECT_LIST() +#endif + #ifdef CONFIG_EVENT_TRACING #define FTRACE_EVENTS() . = ALIGN(8); \ VMLINUX_SYMBOL(__start_ftrace_events) = .; \ @@ -560,6 +569,7 @@ FTRACE_EVENTS() \ TRACE_SYSCALLS() \ KPROBE_BLACKLIST() \ + ERROR_INJECT_LIST() \ MEM_DISCARD(init.rodata) \ CLK_OF_TABLES() \ RESERVEDMEM_OF_TABLES() \ diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 520aeebe0d93..552a666a338b 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -530,4 +530,15 @@ extern const struct bpf_func_proto bpf_sock_map_update_proto; void bpf_user_rnd_init_once(void); u64 bpf_user_rnd_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5); +#if defined(__KERNEL__) && !defined(__ASSEMBLY__) +#ifdef CONFIG_BPF_KPROBE_OVERRIDE +#define BPF_ALLOW_ERROR_INJECTION(fname) \ +static unsigned long __used \ + __attribute__((__section__("_kprobe_error_inject_list"))) \ + _eil_addr_##fname = (unsigned long)fname; +#else +#define BPF_ALLOW_ERROR_INJECTION(fname) +#endif +#endif + #endif /* _LINUX_BPF_H */ diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h index bd2684700b74..4f501cb73aec 100644 --- a/include/linux/kprobes.h +++ b/include/linux/kprobes.h @@ -271,6 +271,7 @@ 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 bool within_kprobe_error_injection_list(unsigned long addr); struct kprobe_insn_cache { struct mutex mutex; diff --git a/include/linux/module.h b/include/linux/module.h index fe5aa3736707..7bb1a9b9a322 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -475,6 +475,11 @@ struct module { ctor_fn_t *ctors; unsigned int num_ctors; #endif + +#ifdef CONFIG_BPF_KPROBE_OVERRIDE + unsigned int num_kprobe_ei_funcs; + unsigned long *kprobe_ei_funcs; +#endif } ____cacheline_aligned __randomize_layout; #ifndef MODULE_ARCH_INIT #define MODULE_ARCH_INIT {} diff --git a/kernel/kprobes.c b/kernel/kprobes.c index a1606a4224e1..bdd7dd724f6f 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -83,6 +83,16 @@ static raw_spinlock_t *kretprobe_table_lock_ptr(unsigned long hash) return &(kretprobe_table_locks[hash].lock); } +/* List of symbols that can be overriden for error injection. */ +static LIST_HEAD(kprobe_error_injection_list); +static DEFINE_MUTEX(kprobe_ei_mutex); +struct kprobe_ei_entry { + struct list_head list; + unsigned long start_addr; + unsigned long end_addr; + void *priv; +}; + /* Blacklist -- list of struct kprobe_blacklist_entry */ static LIST_HEAD(kprobe_blacklist); @@ -1392,6 +1402,17 @@ bool within_kprobe_blacklist(unsigned long addr) return false; } +bool within_kprobe_error_injection_list(unsigned long addr) +{ + struct kprobe_ei_entry *ent; + + list_for_each_entry(ent, &kprobe_error_injection_list, list) { + if (addr >= ent->start_addr && addr < ent->end_addr) + return true; + } + return false; +} + /* * If we have a symbol_name argument, look it up and add the offset field * to it. This way, we can specify a relative address to a symbol. @@ -2164,6 +2185,86 @@ static int __init populate_kprobe_blacklist(unsigned long *start, return 0; } +#ifdef CONFIG_BPF_KPROBE_OVERRIDE +/* Markers of the _kprobe_error_inject_list section */ +extern unsigned long __start_kprobe_error_inject_list[]; +extern unsigned long __stop_kprobe_error_inject_list[]; + +/* + * Lookup and populate the kprobe_error_injection_list. + * + * For safety reasons we only allow certain functions to be overriden with + * bpf_error_injection, so we need to populate the list of the symbols that have + * been marked as safe for overriding. + */ +static void populate_kprobe_error_injection_list(unsigned long *start, + unsigned long *end, + void *priv) +{ + unsigned long *iter; + struct kprobe_ei_entry *ent; + unsigned long entry, offset = 0, size = 0; + + mutex_lock(&kprobe_ei_mutex); + 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)) { + pr_err("Failed to find error inject entry at %p\n", + (void *)entry); + continue; + } + + ent = kmalloc(sizeof(*ent), GFP_KERNEL); + if (!ent) + break; + ent->start_addr = entry; + ent->end_addr = entry + size; + ent->priv = priv; + INIT_LIST_HEAD(&ent->list); + list_add_tail(&ent->list, &kprobe_error_injection_list); + } + mutex_unlock(&kprobe_ei_mutex); +} + +static void __init populate_kernel_kprobe_ei_list(void) +{ + populate_kprobe_error_injection_list(__start_kprobe_error_inject_list, + __stop_kprobe_error_inject_list, + NULL); +} + +static void module_load_kprobe_ei_list(struct module *mod) +{ + if (!mod->num_kprobe_ei_funcs) + return; + populate_kprobe_error_injection_list(mod->kprobe_ei_funcs, + mod->kprobe_ei_funcs + + mod->num_kprobe_ei_funcs, mod); +} + +static void module_unload_kprobe_ei_list(struct module *mod) +{ + struct kprobe_ei_entry *ent, *n; + if (!mod->num_kprobe_ei_funcs) + return; + + mutex_lock(&kprobe_ei_mutex); + list_for_each_entry_safe(ent, n, &kprobe_error_injection_list, list) { + if (ent->priv == mod) { + list_del_init(&ent->list); + kfree(ent); + } + } + mutex_unlock(&kprobe_ei_mutex); +} +#else +static inline void __init populate_kernel_kprobe_ei_list(void) {} +static inline void module_load_kprobe_ei_list(struct module *m) {} +static inline void module_unload_kprobe_ei_list(struct module *m) {} +#endif + /* Module notifier call back, checking kprobes on the module */ static int kprobes_module_callback(struct notifier_block *nb, unsigned long val, void *data) @@ -2174,6 +2275,11 @@ static int kprobes_module_callback(struct notifier_block *nb, unsigned int i; int checkcore = (val == MODULE_STATE_GOING); + if (val == MODULE_STATE_COMING) + module_load_kprobe_ei_list(mod); + else if (val == MODULE_STATE_GOING) + module_unload_kprobe_ei_list(mod); + if (val != MODULE_STATE_GOING && val != MODULE_STATE_LIVE) return NOTIFY_DONE; @@ -2236,6 +2342,8 @@ static int __init init_kprobes(void) pr_err("Please take care of using kprobes.\n"); } + populate_kernel_kprobe_ei_list(); + if (kretprobe_blacklist_size) { /* lookup the function address from its name */ for (i = 0; kretprobe_blacklist[i].name != NULL; i++) { @@ -2403,6 +2511,56 @@ static const struct file_operations debugfs_kprobe_blacklist_ops = { .release = seq_release, }; +/* + * kprobes/error_injection_list -- shows which functions can be overriden for + * error injection. + * */ +static void *kprobe_ei_seq_start(struct seq_file *m, loff_t *pos) +{ + mutex_lock(&kprobe_ei_mutex); + return seq_list_start(&kprobe_error_injection_list, *pos); +} + +static void kprobe_ei_seq_stop(struct seq_file *m, void *v) +{ + mutex_unlock(&kprobe_ei_mutex); +} + +static void *kprobe_ei_seq_next(struct seq_file *m, void *v, loff_t *pos) +{ + return seq_list_next(v, &kprobe_error_injection_list, pos); +} + +static int kprobe_ei_seq_show(struct seq_file *m, void *v) +{ + char buffer[KSYM_SYMBOL_LEN]; + struct kprobe_ei_entry *ent = + list_entry(v, struct kprobe_ei_entry, list); + + sprint_symbol(buffer, ent->start_addr); + seq_printf(m, "%s\n", buffer); + return 0; +} + +static const struct seq_operations kprobe_ei_seq_ops = { + .start = kprobe_ei_seq_start, + .next = kprobe_ei_seq_next, + .stop = kprobe_ei_seq_stop, + .show = kprobe_ei_seq_show, +}; + +static int kprobe_ei_open(struct inode *inode, struct file *filp) +{ + return seq_open(filp, &kprobe_ei_seq_ops); +} + +static const struct file_operations debugfs_kprobe_ei_ops = { + .open = kprobe_ei_open, + .read = seq_read, + .llseek = seq_lseek, + .release = seq_release, +}; + static void arm_all_kprobes(void) { struct hlist_head *head; @@ -2544,6 +2702,11 @@ static int __init debugfs_kprobe_init(void) if (!file) goto error; + file = debugfs_create_file("error_injection_list", 0444, dir, NULL, + &debugfs_kprobe_ei_ops); + if (!file) + goto error; + return 0; error: diff --git a/kernel/module.c b/kernel/module.c index de66ec825992..19d8d9455f2f 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -3110,7 +3110,11 @@ static int find_module_sections(struct module *mod, struct load_info *info) sizeof(*mod->ftrace_callsites), &mod->num_ftrace_callsites); #endif - +#ifdef CONFIG_BPF_KPROBE_OVERRIDE + mod->kprobe_ei_funcs = section_objs(info, "_kprobe_error_inject_list", + sizeof(*mod->kprobe_ei_funcs), + &mod->num_kprobe_ei_funcs); +#endif mod->extable = section_objs(info, "__ex_table", sizeof(*mod->extable), &mod->num_exentries);