Messages in this thread | | | Date | Fri, 25 Apr 2014 10:19:51 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH -tip v9 20/26] kprobes: Support blacklist functions in module |
| |
* Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> (2014/04/24 17:56), Ingo Molnar wrote: > >> diff --git a/include/linux/module.h b/include/linux/module.h > >> index f520a76..2fdb673 100644 > >> --- a/include/linux/module.h > >> +++ b/include/linux/module.h > >> @@ -16,6 +16,7 @@ > >> #include <linux/kobject.h> > >> #include <linux/moduleparam.h> > >> #include <linux/jump_label.h> > >> +#include <linux/kprobes.h> > > > > This include breaks the x86 build: > > > > CC arch/x86/kernel/jump_label.o > > In file included from arch/x86/kernel/jump_label.c:14:0: > > /fast/mingo/tip/arch/x86/include/asm/kprobes.h:35:12: error: conflicting types for ‘kprobe_opcode_t' typedef u8 kprobe_opcode_t; > > [...] > > Hmm, this error seems very odd... and I don't see
Needs 'make allnoconfig' or some similar .config combination.
> > But the #include kprobes.h is unnecessary to begin with, as no kprobe > > specific types are used. > > OK, anyway I'll remove that. > > > > >> #include <linux/export.h> > >> > >> #include <linux/percpu.h> > >> @@ -357,6 +358,10 @@ struct module { > >> unsigned int num_ftrace_callsites; > >> unsigned long *ftrace_callsites; > >> #endif > >> +#ifdef CONFIG_KPROBES > >> + unsigned int num_kprobe_blacklist; > >> + unsigned long *kprobe_blacklist; > >> +#endif > > > > There's a small coding style problem here. > > > > More importantly, I think more should be done to make sure that module > > symbols are marked properly: since the module is going to register the > > kprobes handler, that would be a perfect place to emit a warning, > > right? > > > > In fact, why don't kprobe handlers get added to the exclusion list > > explicitly, when the handler gets registered? With such an approach > > handlers are automatically nokprobe and don't need any annotation - > > which is a far more robust usage model. > > Ah, I see. That is because there are some local functions called > only from the kprobe handlers. It is easy to blacklist the kprobe > handlers itself, but not for the functions which are only called > from them. :( > > So, I can add a patch which automatically add handler functions to > blacklist. But that is another story. I think this patch is also > required.
Fair enough! I'd even argue to not do the auto-blacklisting I suggested, to make it really apparent to module authors that annotations are needed.
Thanks,
Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |