This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH -tip v9 20/26] kprobes: Support blacklist functions in module
- From: Masami Hiramatsu <masami dot hiramatsu dot pt at hitachi dot com>
- To: Ingo Molnar <mingo at kernel dot org>
- Cc: linux-kernel at vger dot kernel dot org, Rusty Russell <rusty at rustcorp dot com dot au>, Andi Kleen <andi at firstfloor dot org>, Ananth N Mavinakayanahalli <ananth at in dot ibm dot com>, Sandeepa Prabhu <sandeepa dot prabhu at linaro dot org>, Frederic Weisbecker <fweisbec at gmail dot com>, x86 at kernel dot org, Steven Rostedt <rostedt at goodmis dot org>, fche at redhat dot com, mingo at redhat dot com, Rob Landley <rob at landley dot net>, "H. Peter Anvin" <hpa at zytor dot com>, Thomas Gleixner <tglx at linutronix dot de>, "David S. Miller" <davem at davemloft dot net>, systemtap at sourceware dot org
- Date: Thu, 24 Apr 2014 20:24:08 +0900
- Subject: Re: [PATCH -tip v9 20/26] kprobes: Support blacklist functions in module
- Authentication-results: sourceware.org; auth=none
- References: <20140417081636 dot 26341 dot 87858 dot stgit at ltc230 dot yrl dot intra dot hitachi dot co dot jp> <20140417081856 dot 26341 dot 53535 dot stgit at ltc230 dot yrl dot intra dot hitachi dot co dot jp> <20140424085601 dot GA7768 at gmail dot com>
(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
>
> 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.
Thank you,
>
> So I'm skipping this patch and the next one that makes use of it.
>
> Thanks,
>
> Ingo
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com