Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
On Wed, 6 Nov 2013 07:07:37 +0100 Ingo Molnar wrote: > So until compilers get smarter (or there's some compiler trick I haven't > noticed) lets stay with the separate section - it's not the end of the > world, the (effective) 'noinline' aspect of noprobes changes code > generation anyway. > It's fine now. I just wanted everyone to be ware if this annotation starts to get used a bit more, that it can cause unwanted side effects. -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
* Masami Hiramatsu wrote: > (2013/11/06 15:07), Ingo Molnar wrote: > > > > * Masami Hiramatsu wrote: > > > [...] I hope to build the list when the kernel build time if > possible... Would you have any idea to classify some annotated(but no > side-effect) functions? > >>> > >>> The macro magic I can think of would need to change the syntax of the > >>> function definition - for example that is how the SYSCALL_DEFINE*() > >>> macros work. > >> > >> Would you mean something like the below macro? :) > >> > >> NOKPROBE_SYMBOL(int, func_ptr_is_kernel_text)(void *ptr) > > > > I think this is rather ugly and harder to maintain. The whole _point_ of > > such annotations is to make them 'easy on the eyes', to make it easy to > > skip a 'noinline', 'noprobe' or 'notrace' tag. > > > > Using something like NOKPROBE_SYMBOL() makes the whole construct ugly and > > attention seeking. > > Hmm, by the way how about Steven's idea? A macro like EXPORT_SYMBOL? At > least for kprobes_blacklist, which is defined/maintained in kprobes.c > for some symbols(*), that is useful for updating it because we can put > it near the function definition. Dunno, it feels a bit fragile to me. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
(2013/11/06 15:07), Ingo Molnar wrote: > > * Masami Hiramatsu wrote: > [...] I hope to build the list when the kernel build time if possible... Would you have any idea to classify some annotated(but no side-effect) functions? >>> >>> The macro magic I can think of would need to change the syntax of the >>> function definition - for example that is how the SYSCALL_DEFINE*() >>> macros work. >> >> Would you mean something like the below macro? :) >> >> NOKPROBE_SYMBOL(int, func_ptr_is_kernel_text)(void *ptr) > > I think this is rather ugly and harder to maintain. The whole _point_ of > such annotations is to make them 'easy on the eyes', to make it easy to > skip a 'noinline', 'noprobe' or 'notrace' tag. > > Using something like NOKPROBE_SYMBOL() makes the whole construct ugly and > attention seeking. Hmm, by the way how about Steven's idea? A macro like EXPORT_SYMBOL? At least for kprobes_blacklist, which is defined/maintained in kprobes.c for some symbols(*), that is useful for updating it because we can put it near the function definition. * These symbols can not moves to other section because it already in a different section. Of course, still this is not a big problem since there are a few symbols in the kprobe_blacklist. > So until compilers get smarter (or there's some compiler trick I haven't > noticed) lets stay with the separate section - it's not the end of the > world, the (effective) 'noinline' aspect of noprobes changes code > generation anyway. I see. :) So, would you pull this series ? Or I need any update? Thank you, -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
(2013/11/06 15:07), Ingo Molnar wrote: * Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: [...] I hope to build the list when the kernel build time if possible... Would you have any idea to classify some annotated(but no side-effect) functions? The macro magic I can think of would need to change the syntax of the function definition - for example that is how the SYSCALL_DEFINE*() macros work. Would you mean something like the below macro? :) NOKPROBE_SYMBOL(int, func_ptr_is_kernel_text)(void *ptr) I think this is rather ugly and harder to maintain. The whole _point_ of such annotations is to make them 'easy on the eyes', to make it easy to skip a 'noinline', 'noprobe' or 'notrace' tag. Using something like NOKPROBE_SYMBOL() makes the whole construct ugly and attention seeking. Hmm, by the way how about Steven's idea? A macro like EXPORT_SYMBOL? At least for kprobes_blacklist, which is defined/maintained in kprobes.c for some symbols(*), that is useful for updating it because we can put it near the function definition. * These symbols can not moves to other section because it already in a different section. Of course, still this is not a big problem since there are a few symbols in the kprobe_blacklist. So until compilers get smarter (or there's some compiler trick I haven't noticed) lets stay with the separate section - it's not the end of the world, the (effective) 'noinline' aspect of noprobes changes code generation anyway. I see. :) So, would you pull this series ? Or I need any update? Thank you, -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
* Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: (2013/11/06 15:07), Ingo Molnar wrote: * Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: [...] I hope to build the list when the kernel build time if possible... Would you have any idea to classify some annotated(but no side-effect) functions? The macro magic I can think of would need to change the syntax of the function definition - for example that is how the SYSCALL_DEFINE*() macros work. Would you mean something like the below macro? :) NOKPROBE_SYMBOL(int, func_ptr_is_kernel_text)(void *ptr) I think this is rather ugly and harder to maintain. The whole _point_ of such annotations is to make them 'easy on the eyes', to make it easy to skip a 'noinline', 'noprobe' or 'notrace' tag. Using something like NOKPROBE_SYMBOL() makes the whole construct ugly and attention seeking. Hmm, by the way how about Steven's idea? A macro like EXPORT_SYMBOL? At least for kprobes_blacklist, which is defined/maintained in kprobes.c for some symbols(*), that is useful for updating it because we can put it near the function definition. Dunno, it feels a bit fragile to me. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
On Wed, 6 Nov 2013 07:07:37 +0100 Ingo Molnar mi...@kernel.org wrote: So until compilers get smarter (or there's some compiler trick I haven't noticed) lets stay with the separate section - it's not the end of the world, the (effective) 'noinline' aspect of noprobes changes code generation anyway. It's fine now. I just wanted everyone to be ware if this annotation starts to get used a bit more, that it can cause unwanted side effects. -- Steve -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
* Masami Hiramatsu wrote: > >> [...] I hope to build the list when the kernel build time if > >> possible... Would you have any idea to classify some annotated(but no > >> side-effect) functions? > > > > The macro magic I can think of would need to change the syntax of the > > function definition - for example that is how the SYSCALL_DEFINE*() > > macros work. > > Would you mean something like the below macro? :) > > NOKPROBE_SYMBOL(int, func_ptr_is_kernel_text)(void *ptr) I think this is rather ugly and harder to maintain. The whole _point_ of such annotations is to make them 'easy on the eyes', to make it easy to skip a 'noinline', 'noprobe' or 'notrace' tag. Using something like NOKPROBE_SYMBOL() makes the whole construct ugly and attention seeking. So until compilers get smarter (or there's some compiler trick I haven't noticed) lets stay with the separate section - it's not the end of the world, the (effective) 'noinline' aspect of noprobes changes code generation anyway. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
On Tue, 5 Nov 2013 08:05:37 +0100
Ingo Molnar wrote:
> The macro magic I can think of would need to change the syntax of the
> function definition - for example that is how the SYSCALL_DEFINE*() macros
> work.
Or something like the EXPORT_SYMBOL(), but that wouldn't include the
size of the function. But using the name we could use kallsyms to see
if a probe is placed in a function that is blacklisted. Not very pretty
to do though.
>
> It would be nice if there was a GCC extension that marked a function
> noinline and allowed the emitting of the function's address (and size)
> into a special section - but I'm not aware of any such compiler feature
> today.
Yeah, I was wishing the same thing. Maybe I'll try to talk with the gcc
folks about adding such a feature. Something like
void __attribute__((save_loc_and_size(".section"))) function(void)
{
}
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
(2013/11/05 20:38), Masami Hiramatsu wrote:
> (2013/11/05 16:05), Ingo Molnar wrote:
>>
>> * Masami Hiramatsu wrote:
>>
>>> (2013/11/05 15:09), Ingo Molnar wrote:
* Steven Rostedt wrote:
> On Fri, 01 Nov 2013 11:25:37 +
> Masami Hiramatsu wrote:
>
>> Prohibit probing on func_ptr_is_kernel_text().
>> Since the func_ptr_is_kernel_text() is called from
>> notifier_call_chain() which is called from int3 handler,
>> probing it may cause double int3 fault and kernel will
>> reboot.
>>
>> This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
>>
>> Signed-off-by: Masami Hiramatsu
>> Cc: Andrew Morton
>> Cc: "Uwe Kleine-König"
>> Cc: Borislav Petkov
>> Cc: Ingo Molnar
>> ---
>> kernel/extable.c |2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/extable.c b/kernel/extable.c
>> index 832cb28..022fb25 100644
>> --- a/kernel/extable.c
>> +++ b/kernel/extable.c
>> @@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
>> * pointer is part of the kernel text, we need to do some
>> * special dereferencing first.
>> */
>> -int func_ptr_is_kernel_text(void *ptr)
>> +int nokprobe func_ptr_is_kernel_text(void *ptr)
>> {
>> unsigned long addr;
>> addr = (unsigned long) dereference_function_descriptor(ptr);
>>
>
> One thing I worry about the "nokprobe" annotation, is that it moves the
> location of the function out of local. This function no exists in the
> section with its users. Same with the debug functions in the other
> patch.
Well, it's a bit like noinline, that changes the position of the function
as well. So it's not true that 'noxyz' attributes don't affect function
placement - they often don't, but some do.
The more important aspect is that 'noprobe' makes it really, really
apparent what the tag is about, at first sight.
_How_ the 'non probing' is achived is an implementational detail when
kprobes are enabled: right now it puts a function into a separate section,
but we could just a much build a list of function names and check against
it at probe insertion time.
>>>
>>> Actually, kprobes already has it -- kprobes_blacklist. Currently the
>>> list is manually maintained in kprobes.c separated from the function
>>> definition. [...]
>>
>> Yes, I meant a list that is built automatically from the 'noprobe'
>> annotations.
>
> Agreed. That makes maintenance work simple, and we can remove
> ".kprobes.text" section.
>
>>> [...] I hope to build the list when the kernel build time if possible...
>>> Would you have any idea to classify some annotated(but no side-effect)
>>> functions?
>>
>> The macro magic I can think of would need to change the syntax of the
>> function definition - for example that is how the SYSCALL_DEFINE*() macros
>> work.
>
> Would you mean something like the below macro? :)
>
> NOKPROBE_SYMBOL(int, func_ptr_is_kernel_text)(void *ptr)
>
> which is expanded as;
>
> static struct nokprobe_entry __used
> __nokprobe_entry_func_ptr_is_kernel_text = {
> .name = "func_ptr_is_kernel_text"
> };
> static struct kprobe_blacklist_entry __used
> __attribute__((section("_nokprobe_list")))
> __p_nokprobe_entry_func_ptr_is_kernel_text =
> &__nokprobe_entry_func_ptr_is_kernel_text;
> int func_ptr_is_kernel_text(void *ptr)
>
By the way, this method can be done in C file, but not in asm file.
And there are many sensitive entries in the entry_*.S. I think we have
two options.
(A) keep the kprobes.text(or nokprobe.text) section for such assembler parts.
(B) Put a starter symbol and end symbol in such region and make a list
of symbols between them in build time by using nm.
Anyway, until removing all __kprobes from kernel, we can not remove the
kprobes.text section. So, at the first step I just try to introduce
above macro and apply it to only the symbols in kprobes_blacklist.
After that, I'll try to classify the real unsafe entries and apply
the new macro. Eventually, I think I can remove all __kprobes.
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
(2013/11/05 16:05), Ingo Molnar wrote:
>
> * Masami Hiramatsu wrote:
>
>> (2013/11/05 15:09), Ingo Molnar wrote:
>>>
>>> * Steven Rostedt wrote:
>>>
On Fri, 01 Nov 2013 11:25:37 +
Masami Hiramatsu wrote:
> Prohibit probing on func_ptr_is_kernel_text().
> Since the func_ptr_is_kernel_text() is called from
> notifier_call_chain() which is called from int3 handler,
> probing it may cause double int3 fault and kernel will
> reboot.
>
> This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
>
> Signed-off-by: Masami Hiramatsu
> Cc: Andrew Morton
> Cc: "Uwe Kleine-König"
> Cc: Borislav Petkov
> Cc: Ingo Molnar
> ---
> kernel/extable.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/extable.c b/kernel/extable.c
> index 832cb28..022fb25 100644
> --- a/kernel/extable.c
> +++ b/kernel/extable.c
> @@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
> * pointer is part of the kernel text, we need to do some
> * special dereferencing first.
> */
> -int func_ptr_is_kernel_text(void *ptr)
> +int nokprobe func_ptr_is_kernel_text(void *ptr)
> {
> unsigned long addr;
> addr = (unsigned long) dereference_function_descriptor(ptr);
>
One thing I worry about the "nokprobe" annotation, is that it moves the
location of the function out of local. This function no exists in the
section with its users. Same with the debug functions in the other
patch.
>>>
>>> Well, it's a bit like noinline, that changes the position of the function
>>> as well. So it's not true that 'noxyz' attributes don't affect function
>>> placement - they often don't, but some do.
>>>
>>> The more important aspect is that 'noprobe' makes it really, really
>>> apparent what the tag is about, at first sight.
>>>
>>> _How_ the 'non probing' is achived is an implementational detail when
>>> kprobes are enabled: right now it puts a function into a separate section,
>>> but we could just a much build a list of function names and check against
>>> it at probe insertion time.
>>
>> Actually, kprobes already has it -- kprobes_blacklist. Currently the
>> list is manually maintained in kprobes.c separated from the function
>> definition. [...]
>
> Yes, I meant a list that is built automatically from the 'noprobe'
> annotations.
Agreed. That makes maintenance work simple, and we can remove
".kprobes.text" section.
>> [...] I hope to build the list when the kernel build time if possible...
>> Would you have any idea to classify some annotated(but no side-effect)
>> functions?
>
> The macro magic I can think of would need to change the syntax of the
> function definition - for example that is how the SYSCALL_DEFINE*() macros
> work.
Would you mean something like the below macro? :)
NOKPROBE_SYMBOL(int, func_ptr_is_kernel_text)(void *ptr)
which is expanded as;
static struct nokprobe_entry __used
__nokprobe_entry_func_ptr_is_kernel_text = {
.name = "func_ptr_is_kernel_text"
};
static struct kprobe_blacklist_entry __used
__attribute__((section("_nokprobe_list")))
__p_nokprobe_entry_func_ptr_is_kernel_text =
&__nokprobe_entry_func_ptr_is_kernel_text;
int func_ptr_is_kernel_text(void *ptr)
Hmm, this looks worth to try.
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
(2013/11/05 16:05), Ingo Molnar wrote:
* Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
(2013/11/05 15:09), Ingo Molnar wrote:
* Steven Rostedt rost...@goodmis.org wrote:
On Fri, 01 Nov 2013 11:25:37 +
Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
Prohibit probing on func_ptr_is_kernel_text().
Since the func_ptr_is_kernel_text() is called from
notifier_call_chain() which is called from int3 handler,
probing it may cause double int3 fault and kernel will
reboot.
This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Andrew Morton a...@linux-foundation.org
Cc: Uwe Kleine-König u.kleine-koe...@pengutronix.de
Cc: Borislav Petkov b...@suse.de
Cc: Ingo Molnar mi...@kernel.org
---
kernel/extable.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/extable.c b/kernel/extable.c
index 832cb28..022fb25 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
* pointer is part of the kernel text, we need to do some
* special dereferencing first.
*/
-int func_ptr_is_kernel_text(void *ptr)
+int nokprobe func_ptr_is_kernel_text(void *ptr)
{
unsigned long addr;
addr = (unsigned long) dereference_function_descriptor(ptr);
One thing I worry about the nokprobe annotation, is that it moves the
location of the function out of local. This function no exists in the
section with its users. Same with the debug functions in the other
patch.
Well, it's a bit like noinline, that changes the position of the function
as well. So it's not true that 'noxyz' attributes don't affect function
placement - they often don't, but some do.
The more important aspect is that 'noprobe' makes it really, really
apparent what the tag is about, at first sight.
_How_ the 'non probing' is achived is an implementational detail when
kprobes are enabled: right now it puts a function into a separate section,
but we could just a much build a list of function names and check against
it at probe insertion time.
Actually, kprobes already has it -- kprobes_blacklist. Currently the
list is manually maintained in kprobes.c separated from the function
definition. [...]
Yes, I meant a list that is built automatically from the 'noprobe'
annotations.
Agreed. That makes maintenance work simple, and we can remove
.kprobes.text section.
[...] I hope to build the list when the kernel build time if possible...
Would you have any idea to classify some annotated(but no side-effect)
functions?
The macro magic I can think of would need to change the syntax of the
function definition - for example that is how the SYSCALL_DEFINE*() macros
work.
Would you mean something like the below macro? :)
NOKPROBE_SYMBOL(int, func_ptr_is_kernel_text)(void *ptr)
which is expanded as;
static struct nokprobe_entry __used
__nokprobe_entry_func_ptr_is_kernel_text = {
.name = func_ptr_is_kernel_text
};
static struct kprobe_blacklist_entry __used
__attribute__((section(_nokprobe_list)))
__p_nokprobe_entry_func_ptr_is_kernel_text =
__nokprobe_entry_func_ptr_is_kernel_text;
int func_ptr_is_kernel_text(void *ptr)
Hmm, this looks worth to try.
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
(2013/11/05 20:38), Masami Hiramatsu wrote:
(2013/11/05 16:05), Ingo Molnar wrote:
* Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
(2013/11/05 15:09), Ingo Molnar wrote:
* Steven Rostedt rost...@goodmis.org wrote:
On Fri, 01 Nov 2013 11:25:37 +
Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
Prohibit probing on func_ptr_is_kernel_text().
Since the func_ptr_is_kernel_text() is called from
notifier_call_chain() which is called from int3 handler,
probing it may cause double int3 fault and kernel will
reboot.
This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Andrew Morton a...@linux-foundation.org
Cc: Uwe Kleine-König u.kleine-koe...@pengutronix.de
Cc: Borislav Petkov b...@suse.de
Cc: Ingo Molnar mi...@kernel.org
---
kernel/extable.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/extable.c b/kernel/extable.c
index 832cb28..022fb25 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
* pointer is part of the kernel text, we need to do some
* special dereferencing first.
*/
-int func_ptr_is_kernel_text(void *ptr)
+int nokprobe func_ptr_is_kernel_text(void *ptr)
{
unsigned long addr;
addr = (unsigned long) dereference_function_descriptor(ptr);
One thing I worry about the nokprobe annotation, is that it moves the
location of the function out of local. This function no exists in the
section with its users. Same with the debug functions in the other
patch.
Well, it's a bit like noinline, that changes the position of the function
as well. So it's not true that 'noxyz' attributes don't affect function
placement - they often don't, but some do.
The more important aspect is that 'noprobe' makes it really, really
apparent what the tag is about, at first sight.
_How_ the 'non probing' is achived is an implementational detail when
kprobes are enabled: right now it puts a function into a separate section,
but we could just a much build a list of function names and check against
it at probe insertion time.
Actually, kprobes already has it -- kprobes_blacklist. Currently the
list is manually maintained in kprobes.c separated from the function
definition. [...]
Yes, I meant a list that is built automatically from the 'noprobe'
annotations.
Agreed. That makes maintenance work simple, and we can remove
.kprobes.text section.
[...] I hope to build the list when the kernel build time if possible...
Would you have any idea to classify some annotated(but no side-effect)
functions?
The macro magic I can think of would need to change the syntax of the
function definition - for example that is how the SYSCALL_DEFINE*() macros
work.
Would you mean something like the below macro? :)
NOKPROBE_SYMBOL(int, func_ptr_is_kernel_text)(void *ptr)
which is expanded as;
static struct nokprobe_entry __used
__nokprobe_entry_func_ptr_is_kernel_text = {
.name = func_ptr_is_kernel_text
};
static struct kprobe_blacklist_entry __used
__attribute__((section(_nokprobe_list)))
__p_nokprobe_entry_func_ptr_is_kernel_text =
__nokprobe_entry_func_ptr_is_kernel_text;
int func_ptr_is_kernel_text(void *ptr)
By the way, this method can be done in C file, but not in asm file.
And there are many sensitive entries in the entry_*.S. I think we have
two options.
(A) keep the kprobes.text(or nokprobe.text) section for such assembler parts.
(B) Put a starter symbol and end symbol in such region and make a list
of symbols between them in build time by using nm.
Anyway, until removing all __kprobes from kernel, we can not remove the
kprobes.text section. So, at the first step I just try to introduce
above macro and apply it to only the symbols in kprobes_blacklist.
After that, I'll try to classify the real unsafe entries and apply
the new macro. Eventually, I think I can remove all __kprobes.
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
On Tue, 5 Nov 2013 08:05:37 +0100
Ingo Molnar mi...@kernel.org wrote:
The macro magic I can think of would need to change the syntax of the
function definition - for example that is how the SYSCALL_DEFINE*() macros
work.
Or something like the EXPORT_SYMBOL(), but that wouldn't include the
size of the function. But using the name we could use kallsyms to see
if a probe is placed in a function that is blacklisted. Not very pretty
to do though.
It would be nice if there was a GCC extension that marked a function
noinline and allowed the emitting of the function's address (and size)
into a special section - but I'm not aware of any such compiler feature
today.
Yeah, I was wishing the same thing. Maybe I'll try to talk with the gcc
folks about adding such a feature. Something like
void __attribute__((save_loc_and_size(.section))) function(void)
{
}
-- Steve
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
* Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote: [...] I hope to build the list when the kernel build time if possible... Would you have any idea to classify some annotated(but no side-effect) functions? The macro magic I can think of would need to change the syntax of the function definition - for example that is how the SYSCALL_DEFINE*() macros work. Would you mean something like the below macro? :) NOKPROBE_SYMBOL(int, func_ptr_is_kernel_text)(void *ptr) I think this is rather ugly and harder to maintain. The whole _point_ of such annotations is to make them 'easy on the eyes', to make it easy to skip a 'noinline', 'noprobe' or 'notrace' tag. Using something like NOKPROBE_SYMBOL() makes the whole construct ugly and attention seeking. So until compilers get smarter (or there's some compiler trick I haven't noticed) lets stay with the separate section - it's not the end of the world, the (effective) 'noinline' aspect of noprobes changes code generation anyway. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
* Masami Hiramatsu wrote:
> (2013/11/05 15:09), Ingo Molnar wrote:
> >
> > * Steven Rostedt wrote:
> >
> >> On Fri, 01 Nov 2013 11:25:37 +
> >> Masami Hiramatsu wrote:
> >>
> >>> Prohibit probing on func_ptr_is_kernel_text().
> >>> Since the func_ptr_is_kernel_text() is called from
> >>> notifier_call_chain() which is called from int3 handler,
> >>> probing it may cause double int3 fault and kernel will
> >>> reboot.
> >>>
> >>> This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
> >>>
> >>> Signed-off-by: Masami Hiramatsu
> >>> Cc: Andrew Morton
> >>> Cc: "Uwe Kleine-König"
> >>> Cc: Borislav Petkov
> >>> Cc: Ingo Molnar
> >>> ---
> >>> kernel/extable.c |2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/kernel/extable.c b/kernel/extable.c
> >>> index 832cb28..022fb25 100644
> >>> --- a/kernel/extable.c
> >>> +++ b/kernel/extable.c
> >>> @@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
> >>> * pointer is part of the kernel text, we need to do some
> >>> * special dereferencing first.
> >>> */
> >>> -int func_ptr_is_kernel_text(void *ptr)
> >>> +int nokprobe func_ptr_is_kernel_text(void *ptr)
> >>> {
> >>> unsigned long addr;
> >>> addr = (unsigned long) dereference_function_descriptor(ptr);
> >>>
> >>
> >> One thing I worry about the "nokprobe" annotation, is that it moves the
> >> location of the function out of local. This function no exists in the
> >> section with its users. Same with the debug functions in the other
> >> patch.
> >
> > Well, it's a bit like noinline, that changes the position of the function
> > as well. So it's not true that 'noxyz' attributes don't affect function
> > placement - they often don't, but some do.
> >
> > The more important aspect is that 'noprobe' makes it really, really
> > apparent what the tag is about, at first sight.
> >
> > _How_ the 'non probing' is achived is an implementational detail when
> > kprobes are enabled: right now it puts a function into a separate section,
> > but we could just a much build a list of function names and check against
> > it at probe insertion time.
>
> Actually, kprobes already has it -- kprobes_blacklist. Currently the
> list is manually maintained in kprobes.c separated from the function
> definition. [...]
Yes, I meant a list that is built automatically from the 'noprobe'
annotations.
> [...] I hope to build the list when the kernel build time if possible...
> Would you have any idea to classify some annotated(but no side-effect)
> functions?
The macro magic I can think of would need to change the syntax of the
function definition - for example that is how the SYSCALL_DEFINE*() macros
work.
It would be nice if there was a GCC extension that marked a function
noinline and allowed the emitting of the function's address (and size)
into a special section - but I'm not aware of any such compiler feature
today.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
(2013/11/05 15:09), Ingo Molnar wrote:
>
> * Steven Rostedt wrote:
>
>> On Fri, 01 Nov 2013 11:25:37 +
>> Masami Hiramatsu wrote:
>>
>>> Prohibit probing on func_ptr_is_kernel_text().
>>> Since the func_ptr_is_kernel_text() is called from
>>> notifier_call_chain() which is called from int3 handler,
>>> probing it may cause double int3 fault and kernel will
>>> reboot.
>>>
>>> This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
>>>
>>> Signed-off-by: Masami Hiramatsu
>>> Cc: Andrew Morton
>>> Cc: "Uwe Kleine-König"
>>> Cc: Borislav Petkov
>>> Cc: Ingo Molnar
>>> ---
>>> kernel/extable.c |2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/kernel/extable.c b/kernel/extable.c
>>> index 832cb28..022fb25 100644
>>> --- a/kernel/extable.c
>>> +++ b/kernel/extable.c
>>> @@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
>>> * pointer is part of the kernel text, we need to do some
>>> * special dereferencing first.
>>> */
>>> -int func_ptr_is_kernel_text(void *ptr)
>>> +int nokprobe func_ptr_is_kernel_text(void *ptr)
>>> {
>>> unsigned long addr;
>>> addr = (unsigned long) dereference_function_descriptor(ptr);
>>>
>>
>> One thing I worry about the "nokprobe" annotation, is that it moves the
>> location of the function out of local. This function no exists in the
>> section with its users. Same with the debug functions in the other
>> patch.
>
> Well, it's a bit like noinline, that changes the position of the function
> as well. So it's not true that 'noxyz' attributes don't affect function
> placement - they often don't, but some do.
>
> The more important aspect is that 'noprobe' makes it really, really
> apparent what the tag is about, at first sight.
>
> _How_ the 'non probing' is achived is an implementational detail when
> kprobes are enabled: right now it puts a function into a separate section,
> but we could just a much build a list of function names and check against
> it at probe insertion time.
Actually, kprobes already has it -- kprobes_blacklist. Currently
the list is manually maintained in kprobes.c separated from the
function definition. I hope to build the list when the kernel
build time if possible... Would you have any idea to classify
some annotated(but no side-effect) functions?
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
* Steven Rostedt wrote:
> On Fri, 01 Nov 2013 11:25:37 +
> Masami Hiramatsu wrote:
>
> > Prohibit probing on func_ptr_is_kernel_text().
> > Since the func_ptr_is_kernel_text() is called from
> > notifier_call_chain() which is called from int3 handler,
> > probing it may cause double int3 fault and kernel will
> > reboot.
> >
> > This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
> >
> > Signed-off-by: Masami Hiramatsu
> > Cc: Andrew Morton
> > Cc: "Uwe Kleine-König"
> > Cc: Borislav Petkov
> > Cc: Ingo Molnar
> > ---
> > kernel/extable.c |2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/extable.c b/kernel/extable.c
> > index 832cb28..022fb25 100644
> > --- a/kernel/extable.c
> > +++ b/kernel/extable.c
> > @@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
> > * pointer is part of the kernel text, we need to do some
> > * special dereferencing first.
> > */
> > -int func_ptr_is_kernel_text(void *ptr)
> > +int nokprobe func_ptr_is_kernel_text(void *ptr)
> > {
> > unsigned long addr;
> > addr = (unsigned long) dereference_function_descriptor(ptr);
> >
>
> One thing I worry about the "nokprobe" annotation, is that it moves the
> location of the function out of local. This function no exists in the
> section with its users. Same with the debug functions in the other
> patch.
Well, it's a bit like noinline, that changes the position of the function
as well. So it's not true that 'noxyz' attributes don't affect function
placement - they often don't, but some do.
The more important aspect is that 'noprobe' makes it really, really
apparent what the tag is about, at first sight.
_How_ the 'non probing' is achived is an implementational detail when
kprobes are enabled: right now it puts a function into a separate section,
but we could just a much build a list of function names and check against
it at probe insertion time.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
(2013/11/05 11:00), Steven Rostedt wrote:
> On Fri, 01 Nov 2013 11:25:37 +
> Masami Hiramatsu wrote:
>
>> Prohibit probing on func_ptr_is_kernel_text().
>> Since the func_ptr_is_kernel_text() is called from
>> notifier_call_chain() which is called from int3 handler,
>> probing it may cause double int3 fault and kernel will
>> reboot.
>>
>> This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
>>
>> Signed-off-by: Masami Hiramatsu
>> Cc: Andrew Morton
>> Cc: "Uwe Kleine-König"
>> Cc: Borislav Petkov
>> Cc: Ingo Molnar
>> ---
>> kernel/extable.c |2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/extable.c b/kernel/extable.c
>> index 832cb28..022fb25 100644
>> --- a/kernel/extable.c
>> +++ b/kernel/extable.c
>> @@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
>> * pointer is part of the kernel text, we need to do some
>> * special dereferencing first.
>> */
>> -int func_ptr_is_kernel_text(void *ptr)
>> +int nokprobe func_ptr_is_kernel_text(void *ptr)
>> {
>> unsigned long addr;
>> addr = (unsigned long) dereference_function_descriptor(ptr);
>>
>
> One thing I worry about the "nokprobe" annotation, is that it moves the
> location of the function out of local. This function no exists in
> the section with its users. Same with the debug functions in the
> other patch.
>
> Now these may be a slow path where we really don't care, but if the
> nokprobe expands this can cause issues.
>
> The "nokprobe" works differently than "notrace" as "notrace" is just an
> attribute that tells gcc not to add mcount to it. The "nokprobe"
> actually moves the function into a different section.
Well, in that case, I can put it in the opt-out type
blacklist(kprobe_blacklist). :)
Hmm, I think if I can list nokprobe functions up at build time, we can almost
remove the .kprobes.text (Note that some of entry functions in asm still
require it.)
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
On Fri, 01 Nov 2013 11:25:37 +
Masami Hiramatsu wrote:
> Prohibit probing on func_ptr_is_kernel_text().
> Since the func_ptr_is_kernel_text() is called from
> notifier_call_chain() which is called from int3 handler,
> probing it may cause double int3 fault and kernel will
> reboot.
>
> This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
>
> Signed-off-by: Masami Hiramatsu
> Cc: Andrew Morton
> Cc: "Uwe Kleine-König"
> Cc: Borislav Petkov
> Cc: Ingo Molnar
> ---
> kernel/extable.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/extable.c b/kernel/extable.c
> index 832cb28..022fb25 100644
> --- a/kernel/extable.c
> +++ b/kernel/extable.c
> @@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
> * pointer is part of the kernel text, we need to do some
> * special dereferencing first.
> */
> -int func_ptr_is_kernel_text(void *ptr)
> +int nokprobe func_ptr_is_kernel_text(void *ptr)
> {
> unsigned long addr;
> addr = (unsigned long) dereference_function_descriptor(ptr);
>
One thing I worry about the "nokprobe" annotation, is that it moves the
location of the function out of local. This function no exists in
the section with its users. Same with the debug functions in the
other patch.
Now these may be a slow path where we really don't care, but if the
nokprobe expands this can cause issues.
The "nokprobe" works differently than "notrace" as "notrace" is just an
attribute that tells gcc not to add mcount to it. The "nokprobe"
actually moves the function into a different section.
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
On Fri, 01 Nov 2013 11:25:37 +
Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
Prohibit probing on func_ptr_is_kernel_text().
Since the func_ptr_is_kernel_text() is called from
notifier_call_chain() which is called from int3 handler,
probing it may cause double int3 fault and kernel will
reboot.
This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Andrew Morton a...@linux-foundation.org
Cc: Uwe Kleine-König u.kleine-koe...@pengutronix.de
Cc: Borislav Petkov b...@suse.de
Cc: Ingo Molnar mi...@kernel.org
---
kernel/extable.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/extable.c b/kernel/extable.c
index 832cb28..022fb25 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
* pointer is part of the kernel text, we need to do some
* special dereferencing first.
*/
-int func_ptr_is_kernel_text(void *ptr)
+int nokprobe func_ptr_is_kernel_text(void *ptr)
{
unsigned long addr;
addr = (unsigned long) dereference_function_descriptor(ptr);
One thing I worry about the nokprobe annotation, is that it moves the
location of the function out of local. This function no exists in
the section with its users. Same with the debug functions in the
other patch.
Now these may be a slow path where we really don't care, but if the
nokprobe expands this can cause issues.
The nokprobe works differently than notrace as notrace is just an
attribute that tells gcc not to add mcount to it. The nokprobe
actually moves the function into a different section.
-- Steve
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
(2013/11/05 11:00), Steven Rostedt wrote:
On Fri, 01 Nov 2013 11:25:37 +
Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
Prohibit probing on func_ptr_is_kernel_text().
Since the func_ptr_is_kernel_text() is called from
notifier_call_chain() which is called from int3 handler,
probing it may cause double int3 fault and kernel will
reboot.
This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Andrew Morton a...@linux-foundation.org
Cc: Uwe Kleine-König u.kleine-koe...@pengutronix.de
Cc: Borislav Petkov b...@suse.de
Cc: Ingo Molnar mi...@kernel.org
---
kernel/extable.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/extable.c b/kernel/extable.c
index 832cb28..022fb25 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
* pointer is part of the kernel text, we need to do some
* special dereferencing first.
*/
-int func_ptr_is_kernel_text(void *ptr)
+int nokprobe func_ptr_is_kernel_text(void *ptr)
{
unsigned long addr;
addr = (unsigned long) dereference_function_descriptor(ptr);
One thing I worry about the nokprobe annotation, is that it moves the
location of the function out of local. This function no exists in
the section with its users. Same with the debug functions in the
other patch.
Now these may be a slow path where we really don't care, but if the
nokprobe expands this can cause issues.
The nokprobe works differently than notrace as notrace is just an
attribute that tells gcc not to add mcount to it. The nokprobe
actually moves the function into a different section.
Well, in that case, I can put it in the opt-out type
blacklist(kprobe_blacklist). :)
Hmm, I think if I can list nokprobe functions up at build time, we can almost
remove the .kprobes.text (Note that some of entry functions in asm still
require it.)
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
* Steven Rostedt rost...@goodmis.org wrote:
On Fri, 01 Nov 2013 11:25:37 +
Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
Prohibit probing on func_ptr_is_kernel_text().
Since the func_ptr_is_kernel_text() is called from
notifier_call_chain() which is called from int3 handler,
probing it may cause double int3 fault and kernel will
reboot.
This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Andrew Morton a...@linux-foundation.org
Cc: Uwe Kleine-König u.kleine-koe...@pengutronix.de
Cc: Borislav Petkov b...@suse.de
Cc: Ingo Molnar mi...@kernel.org
---
kernel/extable.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/extable.c b/kernel/extable.c
index 832cb28..022fb25 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
* pointer is part of the kernel text, we need to do some
* special dereferencing first.
*/
-int func_ptr_is_kernel_text(void *ptr)
+int nokprobe func_ptr_is_kernel_text(void *ptr)
{
unsigned long addr;
addr = (unsigned long) dereference_function_descriptor(ptr);
One thing I worry about the nokprobe annotation, is that it moves the
location of the function out of local. This function no exists in the
section with its users. Same with the debug functions in the other
patch.
Well, it's a bit like noinline, that changes the position of the function
as well. So it's not true that 'noxyz' attributes don't affect function
placement - they often don't, but some do.
The more important aspect is that 'noprobe' makes it really, really
apparent what the tag is about, at first sight.
_How_ the 'non probing' is achived is an implementational detail when
kprobes are enabled: right now it puts a function into a separate section,
but we could just a much build a list of function names and check against
it at probe insertion time.
Thanks,
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
(2013/11/05 15:09), Ingo Molnar wrote:
* Steven Rostedt rost...@goodmis.org wrote:
On Fri, 01 Nov 2013 11:25:37 +
Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
Prohibit probing on func_ptr_is_kernel_text().
Since the func_ptr_is_kernel_text() is called from
notifier_call_chain() which is called from int3 handler,
probing it may cause double int3 fault and kernel will
reboot.
This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Andrew Morton a...@linux-foundation.org
Cc: Uwe Kleine-König u.kleine-koe...@pengutronix.de
Cc: Borislav Petkov b...@suse.de
Cc: Ingo Molnar mi...@kernel.org
---
kernel/extable.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/extable.c b/kernel/extable.c
index 832cb28..022fb25 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
* pointer is part of the kernel text, we need to do some
* special dereferencing first.
*/
-int func_ptr_is_kernel_text(void *ptr)
+int nokprobe func_ptr_is_kernel_text(void *ptr)
{
unsigned long addr;
addr = (unsigned long) dereference_function_descriptor(ptr);
One thing I worry about the nokprobe annotation, is that it moves the
location of the function out of local. This function no exists in the
section with its users. Same with the debug functions in the other
patch.
Well, it's a bit like noinline, that changes the position of the function
as well. So it's not true that 'noxyz' attributes don't affect function
placement - they often don't, but some do.
The more important aspect is that 'noprobe' makes it really, really
apparent what the tag is about, at first sight.
_How_ the 'non probing' is achived is an implementational detail when
kprobes are enabled: right now it puts a function into a separate section,
but we could just a much build a list of function names and check against
it at probe insertion time.
Actually, kprobes already has it -- kprobes_blacklist. Currently
the list is manually maintained in kprobes.c separated from the
function definition. I hope to build the list when the kernel
build time if possible... Would you have any idea to classify
some annotated(but no side-effect) functions?
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
* Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
(2013/11/05 15:09), Ingo Molnar wrote:
* Steven Rostedt rost...@goodmis.org wrote:
On Fri, 01 Nov 2013 11:25:37 +
Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
Prohibit probing on func_ptr_is_kernel_text().
Since the func_ptr_is_kernel_text() is called from
notifier_call_chain() which is called from int3 handler,
probing it may cause double int3 fault and kernel will
reboot.
This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Andrew Morton a...@linux-foundation.org
Cc: Uwe Kleine-König u.kleine-koe...@pengutronix.de
Cc: Borislav Petkov b...@suse.de
Cc: Ingo Molnar mi...@kernel.org
---
kernel/extable.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/extable.c b/kernel/extable.c
index 832cb28..022fb25 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
* pointer is part of the kernel text, we need to do some
* special dereferencing first.
*/
-int func_ptr_is_kernel_text(void *ptr)
+int nokprobe func_ptr_is_kernel_text(void *ptr)
{
unsigned long addr;
addr = (unsigned long) dereference_function_descriptor(ptr);
One thing I worry about the nokprobe annotation, is that it moves the
location of the function out of local. This function no exists in the
section with its users. Same with the debug functions in the other
patch.
Well, it's a bit like noinline, that changes the position of the function
as well. So it's not true that 'noxyz' attributes don't affect function
placement - they often don't, but some do.
The more important aspect is that 'noprobe' makes it really, really
apparent what the tag is about, at first sight.
_How_ the 'non probing' is achived is an implementational detail when
kprobes are enabled: right now it puts a function into a separate section,
but we could just a much build a list of function names and check against
it at probe insertion time.
Actually, kprobes already has it -- kprobes_blacklist. Currently the
list is manually maintained in kprobes.c separated from the function
definition. [...]
Yes, I meant a list that is built automatically from the 'noprobe'
annotations.
[...] I hope to build the list when the kernel build time if possible...
Would you have any idea to classify some annotated(but no side-effect)
functions?
The macro magic I can think of would need to change the syntax of the
function definition - for example that is how the SYSCALL_DEFINE*() macros
work.
It would be nice if there was a GCC extension that marked a function
noinline and allowed the emitting of the function's address (and size)
into a special section - but I'm not aware of any such compiler feature
today.
Thanks,
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
On Fri, 01 Nov 2013 11:25:37 +
Masami Hiramatsu wrote:
> Prohibit probing on func_ptr_is_kernel_text().
> Since the func_ptr_is_kernel_text() is called from
> notifier_call_chain() which is called from int3 handler,
> probing it may cause double int3 fault and kernel will
> reboot.
>
> This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
Reviewed-by: Steven Rostedt
-- Steve
>
> Signed-off-by: Masami Hiramatsu
> Cc: Andrew Morton
> Cc: "Uwe Kleine-König"
> Cc: Borislav Petkov
> Cc: Ingo Molnar
> ---
> kernel/extable.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/extable.c b/kernel/extable.c
> index 832cb28..022fb25 100644
> --- a/kernel/extable.c
> +++ b/kernel/extable.c
> @@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
> * pointer is part of the kernel text, we need to do some
> * special dereferencing first.
> */
> -int func_ptr_is_kernel_text(void *ptr)
> +int nokprobe func_ptr_is_kernel_text(void *ptr)
> {
> unsigned long addr;
> addr = (unsigned long) dereference_function_descriptor(ptr);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
Prohibit probing on func_ptr_is_kernel_text().
Since the func_ptr_is_kernel_text() is called from
notifier_call_chain() which is called from int3 handler,
probing it may cause double int3 fault and kernel will
reboot.
This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
Signed-off-by: Masami Hiramatsu
Cc: Andrew Morton
Cc: "Uwe Kleine-König"
Cc: Borislav Petkov
Cc: Ingo Molnar
---
kernel/extable.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/extable.c b/kernel/extable.c
index 832cb28..022fb25 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
* pointer is part of the kernel text, we need to do some
* special dereferencing first.
*/
-int func_ptr_is_kernel_text(void *ptr)
+int nokprobe func_ptr_is_kernel_text(void *ptr)
{
unsigned long addr;
addr = (unsigned long) dereference_function_descriptor(ptr);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
Prohibit probing on func_ptr_is_kernel_text().
Since the func_ptr_is_kernel_text() is called from
notifier_call_chain() which is called from int3 handler,
probing it may cause double int3 fault and kernel will
reboot.
This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Andrew Morton a...@linux-foundation.org
Cc: Uwe Kleine-König u.kleine-koe...@pengutronix.de
Cc: Borislav Petkov b...@suse.de
Cc: Ingo Molnar mi...@kernel.org
---
kernel/extable.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/extable.c b/kernel/extable.c
index 832cb28..022fb25 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
* pointer is part of the kernel text, we need to do some
* special dereferencing first.
*/
-int func_ptr_is_kernel_text(void *ptr)
+int nokprobe func_ptr_is_kernel_text(void *ptr)
{
unsigned long addr;
addr = (unsigned long) dereference_function_descriptor(ptr);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -tip v2 3/3] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text
On Fri, 01 Nov 2013 11:25:37 +
Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
Prohibit probing on func_ptr_is_kernel_text().
Since the func_ptr_is_kernel_text() is called from
notifier_call_chain() which is called from int3 handler,
probing it may cause double int3 fault and kernel will
reboot.
This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.
Reviewed-by: Steven Rostedt rost...@goodmis.org
-- Steve
Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Andrew Morton a...@linux-foundation.org
Cc: Uwe Kleine-König u.kleine-koe...@pengutronix.de
Cc: Borislav Petkov b...@suse.de
Cc: Ingo Molnar mi...@kernel.org
---
kernel/extable.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/extable.c b/kernel/extable.c
index 832cb28..022fb25 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -129,7 +129,7 @@ int kernel_text_address(unsigned long addr)
* pointer is part of the kernel text, we need to do some
* special dereferencing first.
*/
-int func_ptr_is_kernel_text(void *ptr)
+int nokprobe func_ptr_is_kernel_text(void *ptr)
{
unsigned long addr;
addr = (unsigned long) dereference_function_descriptor(ptr);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
