On ia64 and ppc64, the function pointer does not point the
entry address of the function, but the address of function
discriptor (which contains the entry address and misc
data.) Since the kprobes passes the function pointer stored
by NOKPROBE_SYMBOL() to kallsyms_lookup_size_offset() for
initalizing its blacklist, it fails and reports many errors
as below.
Failed to find blacklist 0001013168300000
Failed to find blacklist 0001013000f0a000
Failed to find blacklist 000101315f70a000
Failed to find blacklist 000101324c80a000
Failed to find blacklist 0001013063f0a000
Failed to find blacklist 000101327800a000
Failed to find blacklist 0001013277f0a000
Failed to find blacklist 000101315a70a000
Failed to find blacklist 0001013277e0a000
Failed to find blacklist 000101305a20a000
Failed to find blacklist 0001013277d0a000
Failed to find blacklist 00010130bdc0a000
Failed to find blacklist 00010130dc20a000
Failed to find blacklist 000101309a00a000
Failed to find blacklist 0001013277c0a000
Failed to find blacklist 0001013277b0a000
Failed to find blacklist 0001013277a0a000
Failed to find blacklist 000101327790a000
Failed to find blacklist 000101303140a000
Failed to find blacklist 0001013a3280a000
To fix this bug, this introduces function_entry() macro to
retrieve the entry address from the given function pointer,
and uses for kallsyms_lookup_size_offset() while initializing
blacklist.
Changes in v4:
- Add kernel_text_address() check for verifying the address.
- Moved on the latest linus tree.
Changes in v3:
- Fix a bug to get blacklist address based on function entry
instead of function descriptor. (Suzuki's work, Thanks!)
Changes in V2:
- Use function_entry() macro when lookin up symbols instead
of storing it.
- Update for the latest -next.
Signed-off-by: Masami Hiramatsu <[email protected]>
Reported-by: Tony Luck <[email protected]>
Tested-by: Tony Luck <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Suzuki K. Poulose <[email protected]>
Cc: Tony Luck <[email protected]>
Cc: Fenghua Yu <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Kevin Hao <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/ia64/include/asm/types.h | 2 ++
arch/powerpc/include/asm/types.h | 11 +++++++++++
include/linux/types.h | 4 ++++
kernel/kprobes.c | 15 ++++++++++-----
4 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/arch/ia64/include/asm/types.h b/arch/ia64/include/asm/types.h
index 4c351b1..95279dd 100644
--- a/arch/ia64/include/asm/types.h
+++ b/arch/ia64/include/asm/types.h
@@ -27,5 +27,7 @@ struct fnptr {
unsigned long gp;
};
+#define function_entry(fn) (((struct fnptr *)(fn))->ip)
+
#endif /* !__ASSEMBLY__ */
#endif /* _ASM_IA64_TYPES_H */
diff --git a/arch/powerpc/include/asm/types.h b/arch/powerpc/include/asm/types.h
index bfb6ded..8b89d65 100644
--- a/arch/powerpc/include/asm/types.h
+++ b/arch/powerpc/include/asm/types.h
@@ -25,6 +25,17 @@ typedef struct {
unsigned long env;
} func_descr_t;
+#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF == 1)
+/*
+ * On PPC64 ABIv1 the function pointer actually points to the
+ * function's descriptor. The first entry in the descriptor is the
+ * address of the function text.
+ */
+#define function_entry(fn) (((func_descr_t *)(fn))->entry)
+#else
+#define function_entry(fn) ((unsigned long)(fn))
+#endif
+
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_TYPES_H */
diff --git a/include/linux/types.h b/include/linux/types.h
index a0bb704..3b95369 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -213,5 +213,9 @@ struct callback_head {
};
#define rcu_head callback_head
+#ifndef function_entry
+#define function_entry(fn) ((unsigned long)(fn))
+#endif
+
#endif /* __ASSEMBLY__ */
#endif /* _LINUX_TYPES_H */
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 3214289..7412535 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -32,6 +32,7 @@
* <[email protected]> added function-return probes.
*/
#include <linux/kprobes.h>
+#include <linux/types.h>
#include <linux/hash.h>
#include <linux/init.h>
#include <linux/slab.h>
@@ -2037,19 +2038,23 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
{
unsigned long *iter;
struct kprobe_blacklist_entry *ent;
- unsigned long offset = 0, size = 0;
+ unsigned long entry, offset = 0, size = 0;
for (iter = start; iter < end; iter++) {
- if (!kallsyms_lookup_size_offset(*iter, &size, &offset)) {
- pr_err("Failed to find blacklist %p\n", (void *)*iter);
+ entry = function_entry(*iter);
+
+ if (!kernel_text_address(entry) ||
+ !kallsyms_lookup_size_offset(entry, &size, &offset)) {
+ pr_err("Failed to find blacklist at %p\n",
+ (void *)entry);
continue;
}
ent = kmalloc(sizeof(*ent), GFP_KERNEL);
if (!ent)
return -ENOMEM;
- ent->start_addr = *iter;
- ent->end_addr = *iter + size;
+ ent->start_addr = entry;
+ ent->end_addr = entry + size;
INIT_LIST_HEAD(&ent->list);
list_add_tail(&ent->list, &kprobe_blacklist);
}
Ping? :)
(2014/06/20 11:23), Masami Hiramatsu wrote:
> On ia64 and ppc64, the function pointer does not point the
> entry address of the function, but the address of function
> discriptor (which contains the entry address and misc
> data.) Since the kprobes passes the function pointer stored
> by NOKPROBE_SYMBOL() to kallsyms_lookup_size_offset() for
> initalizing its blacklist, it fails and reports many errors
> as below.
>
> Failed to find blacklist 0001013168300000
> Failed to find blacklist 0001013000f0a000
> Failed to find blacklist 000101315f70a000
> Failed to find blacklist 000101324c80a000
> Failed to find blacklist 0001013063f0a000
> Failed to find blacklist 000101327800a000
> Failed to find blacklist 0001013277f0a000
> Failed to find blacklist 000101315a70a000
> Failed to find blacklist 0001013277e0a000
> Failed to find blacklist 000101305a20a000
> Failed to find blacklist 0001013277d0a000
> Failed to find blacklist 00010130bdc0a000
> Failed to find blacklist 00010130dc20a000
> Failed to find blacklist 000101309a00a000
> Failed to find blacklist 0001013277c0a000
> Failed to find blacklist 0001013277b0a000
> Failed to find blacklist 0001013277a0a000
> Failed to find blacklist 000101327790a000
> Failed to find blacklist 000101303140a000
> Failed to find blacklist 0001013a3280a000
>
> To fix this bug, this introduces function_entry() macro to
> retrieve the entry address from the given function pointer,
> and uses for kallsyms_lookup_size_offset() while initializing
> blacklist.
>
> Changes in v4:
> - Add kernel_text_address() check for verifying the address.
> - Moved on the latest linus tree.
>
> Changes in v3:
> - Fix a bug to get blacklist address based on function entry
> instead of function descriptor. (Suzuki's work, Thanks!)
>
> Changes in V2:
> - Use function_entry() macro when lookin up symbols instead
> of storing it.
> - Update for the latest -next.
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Reported-by: Tony Luck <[email protected]>
> Tested-by: Tony Luck <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Suzuki K. Poulose <[email protected]>
> Cc: Tony Luck <[email protected]>
> Cc: Fenghua Yu <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Ananth N Mavinakayanahalli <[email protected]>
> Cc: Kevin Hao <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/ia64/include/asm/types.h | 2 ++
> arch/powerpc/include/asm/types.h | 11 +++++++++++
> include/linux/types.h | 4 ++++
> kernel/kprobes.c | 15 ++++++++++-----
> 4 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/arch/ia64/include/asm/types.h b/arch/ia64/include/asm/types.h
> index 4c351b1..95279dd 100644
> --- a/arch/ia64/include/asm/types.h
> +++ b/arch/ia64/include/asm/types.h
> @@ -27,5 +27,7 @@ struct fnptr {
> unsigned long gp;
> };
>
> +#define function_entry(fn) (((struct fnptr *)(fn))->ip)
> +
> #endif /* !__ASSEMBLY__ */
> #endif /* _ASM_IA64_TYPES_H */
> diff --git a/arch/powerpc/include/asm/types.h b/arch/powerpc/include/asm/types.h
> index bfb6ded..8b89d65 100644
> --- a/arch/powerpc/include/asm/types.h
> +++ b/arch/powerpc/include/asm/types.h
> @@ -25,6 +25,17 @@ typedef struct {
> unsigned long env;
> } func_descr_t;
>
> +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF == 1)
> +/*
> + * On PPC64 ABIv1 the function pointer actually points to the
> + * function's descriptor. The first entry in the descriptor is the
> + * address of the function text.
> + */
> +#define function_entry(fn) (((func_descr_t *)(fn))->entry)
> +#else
> +#define function_entry(fn) ((unsigned long)(fn))
> +#endif
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* _ASM_POWERPC_TYPES_H */
> diff --git a/include/linux/types.h b/include/linux/types.h
> index a0bb704..3b95369 100644
> --- a/include/linux/types.h
> +++ b/include/linux/types.h
> @@ -213,5 +213,9 @@ struct callback_head {
> };
> #define rcu_head callback_head
>
> +#ifndef function_entry
> +#define function_entry(fn) ((unsigned long)(fn))
> +#endif
> +
> #endif /* __ASSEMBLY__ */
> #endif /* _LINUX_TYPES_H */
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 3214289..7412535 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -32,6 +32,7 @@
> * <[email protected]> added function-return probes.
> */
> #include <linux/kprobes.h>
> +#include <linux/types.h>
> #include <linux/hash.h>
> #include <linux/init.h>
> #include <linux/slab.h>
> @@ -2037,19 +2038,23 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
> {
> unsigned long *iter;
> struct kprobe_blacklist_entry *ent;
> - unsigned long offset = 0, size = 0;
> + unsigned long entry, offset = 0, size = 0;
>
> for (iter = start; iter < end; iter++) {
> - if (!kallsyms_lookup_size_offset(*iter, &size, &offset)) {
> - pr_err("Failed to find blacklist %p\n", (void *)*iter);
> + entry = function_entry(*iter);
> +
> + if (!kernel_text_address(entry) ||
> + !kallsyms_lookup_size_offset(entry, &size, &offset)) {
> + pr_err("Failed to find blacklist at %p\n",
> + (void *)entry);
> continue;
> }
>
> ent = kmalloc(sizeof(*ent), GFP_KERNEL);
> if (!ent)
> return -ENOMEM;
> - ent->start_addr = *iter;
> - ent->end_addr = *iter + size;
> + ent->start_addr = entry;
> + ent->end_addr = entry + size;
> INIT_LIST_HEAD(&ent->list);
> list_add_tail(&ent->list, &kprobe_blacklist);
> }
>
>
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
On Mon, 2014-06-30 at 12:14 +0900, Masami Hiramatsu wrote:
> Ping? :)
Yeah sorry. I started looking at this and got dragged into another mess.
You seem to have duplicated the functionality of arch_deref_entry_point(),
which was also added for kprobes, and for the same reason - ie. because some
arches have strange function pointers. Is there some reason you can't use it?
cheers
(2014/06/30 20:36), Michael Ellerman wrote:
> On Mon, 2014-06-30 at 12:14 +0900, Masami Hiramatsu wrote:
>> Ping? :)
>
> Yeah sorry. I started looking at this and got dragged into another mess.
>
> You seem to have duplicated the functionality of arch_deref_entry_point(),
> which was also added for kprobes, and for the same reason - ie. because some
> arches have strange function pointers. Is there some reason you can't use it?
Ah, right! Hmm, it seems some more work to update it. but basically, we can do.
BTW, is there any other users who need to access the actual function entry (for
kallsyms case)?
If so, I guess it'd better to merge this version and replace kprobe's local
arch_deref_entry_point() with generic function_entry() macro.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
On Tue, 2014-07-01 at 11:21 +0900, Masami Hiramatsu wrote:
> (2014/06/30 20:36), Michael Ellerman wrote:
> > On Mon, 2014-06-30 at 12:14 +0900, Masami Hiramatsu wrote:
> >> Ping? :)
> >
> > Yeah sorry. I started looking at this and got dragged into another mess.
> >
> > You seem to have duplicated the functionality of arch_deref_entry_point(),
> > which was also added for kprobes, and for the same reason - ie. because some
> > arches have strange function pointers. Is there some reason you can't use it?
>
> Ah, right! Hmm, it seems some more work to update it. but basically, we can do.
> BTW, is there any other users who need to access the actual function entry (for
> kallsyms case)?
Not that I'm aware of. We have had function descriptors on 64-bit powerpc for
ever, so in theory by now we should have already found any cases where we need
that sort of wrapper.
cheers
(2014/07/02 13:41), Michael Ellerman wrote:
> On Tue, 2014-07-01 at 11:21 +0900, Masami Hiramatsu wrote:
>> (2014/06/30 20:36), Michael Ellerman wrote:
>>> On Mon, 2014-06-30 at 12:14 +0900, Masami Hiramatsu wrote:
>>>> Ping? :)
>>>
>>> Yeah sorry. I started looking at this and got dragged into another mess.
>>>
>>> You seem to have duplicated the functionality of arch_deref_entry_point(),
>>> which was also added for kprobes, and for the same reason - ie. because some
>>> arches have strange function pointers. Is there some reason you can't use it?
>>
>> Ah, right! Hmm, it seems some more work to update it. but basically, we can do.
>> BTW, is there any other users who need to access the actual function entry (for
>> kallsyms case)?
>
> Not that I'm aware of. We have had function descriptors on 64-bit powerpc for
> ever, so in theory by now we should have already found any cases where we need
> that sort of wrapper.
OK, then I'll update this patch to use arch_deref_entry_point(), and add additional
patch which update to support PPC64 ABIv2.
Thank you!
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
On Wed, 2014-07-02 at 15:39 +0900, Masami Hiramatsu wrote:
> (2014/07/02 13:41), Michael Ellerman wrote:
> > On Tue, 2014-07-01 at 11:21 +0900, Masami Hiramatsu wrote:
> >> (2014/06/30 20:36), Michael Ellerman wrote:
> >>> On Mon, 2014-06-30 at 12:14 +0900, Masami Hiramatsu wrote:
> >>>> Ping? :)
> >>>
> >>> Yeah sorry. I started looking at this and got dragged into another mess.
> >>>
> >>> You seem to have duplicated the functionality of arch_deref_entry_point(),
> >>> which was also added for kprobes, and for the same reason - ie. because some
> >>> arches have strange function pointers. Is there some reason you can't use it?
> >>
> >> Ah, right! Hmm, it seems some more work to update it. but basically, we can do.
> >> BTW, is there any other users who need to access the actual function entry (for
> >> kallsyms case)?
> >
> > Not that I'm aware of. We have had function descriptors on 64-bit powerpc for
> > ever, so in theory by now we should have already found any cases where we need
> > that sort of wrapper.
>
> OK, then I'll update this patch to use arch_deref_entry_point(), and add additional
> patch which update to support PPC64 ABIv2.
I've already done the latter:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=2f0143c91d30823f6f6e7d94d7fa818f7ab18a18
cheers
Since PowerPC64 ABIv2 doesn't have function descriptor
any more, arch_deref_entry_point(), which returns function
entry point from function descriptor, should be updated.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Anoop Thomas Mathew <[email protected]>
Cc: Jiri Kosina <[email protected]>
Cc: [email protected]
---
arch/powerpc/kernel/kprobes.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 90fab64..72a1034 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -491,7 +491,12 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
return ret;
}
-#ifdef CONFIG_PPC64
+#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF == 1)
+/*
+ * On PPC64 ABIv1 the function pointer actually points to the
+ * function's descriptor. The first entry in the descriptor is the
+ * address of the function text.
+ */
unsigned long arch_deref_entry_point(void *entry)
{
return ((func_descr_t *)entry)->entry;
On ia64 and ppc64, the function pointer does not point the
entry address of the function, but the address of function
discriptor (which contains the entry address and misc
data.) Since the kprobes passes the function pointer stored
by NOKPROBE_SYMBOL() to kallsyms_lookup_size_offset() for
initalizing its blacklist, it fails and reports many errors
as below.
Failed to find blacklist 0001013168300000
Failed to find blacklist 0001013000f0a000
Failed to find blacklist 000101315f70a000
Failed to find blacklist 000101324c80a000
Failed to find blacklist 0001013063f0a000
Failed to find blacklist 000101327800a000
Failed to find blacklist 0001013277f0a000
Failed to find blacklist 000101315a70a000
Failed to find blacklist 0001013277e0a000
Failed to find blacklist 000101305a20a000
Failed to find blacklist 0001013277d0a000
Failed to find blacklist 00010130bdc0a000
Failed to find blacklist 00010130dc20a000
Failed to find blacklist 000101309a00a000
Failed to find blacklist 0001013277c0a000
Failed to find blacklist 0001013277b0a000
Failed to find blacklist 0001013277a0a000
Failed to find blacklist 000101327790a000
Failed to find blacklist 000101303140a000
Failed to find blacklist 0001013a3280a000
To fix this bug, this introduces function_entry() macro to
retrieve the entry address from the given function pointer,
and uses for kallsyms_lookup_size_offset() while initializing
blacklist.
Changes in v5:
- Use arch_deref_entry_point() instead of function_entry().
Changes in v4:
- Add kernel_text_address() check for verifying the address.
- Moved on the latest linus tree.
Changes in v3:
- Fix a bug to get blacklist address based on function entry
instead of function descriptor. (Suzuki's work, Thanks!)
Changes in V2:
- Use function_entry() macro when lookin up symbols instead
of storing it.
- Update for the latest -next.
Signed-off-by: Masami Hiramatsu <[email protected]>
Reported-by: Tony Luck <[email protected]>
Tested-by: Tony Luck <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Suzuki K. Poulose <[email protected]>
Cc: Tony Luck <[email protected]>
Cc: Fenghua Yu <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Kevin Hao <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
kernel/kprobes.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 3214289..ec370cc 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -32,6 +32,7 @@
* <[email protected]> added function-return probes.
*/
#include <linux/kprobes.h>
+#include <linux/types.h>
#include <linux/hash.h>
#include <linux/init.h>
#include <linux/slab.h>
@@ -2037,19 +2038,23 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
{
unsigned long *iter;
struct kprobe_blacklist_entry *ent;
- unsigned long offset = 0, size = 0;
+ unsigned long entry, offset = 0, size = 0;
for (iter = start; iter < end; iter++) {
- if (!kallsyms_lookup_size_offset(*iter, &size, &offset)) {
- pr_err("Failed to find blacklist %p\n", (void *)*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 blacklist at %p\n",
+ (void *)entry);
continue;
}
ent = kmalloc(sizeof(*ent), GFP_KERNEL);
if (!ent)
return -ENOMEM;
- ent->start_addr = *iter;
- ent->end_addr = *iter + size;
+ ent->start_addr = entry;
+ ent->end_addr = entry + size;
INIT_LIST_HEAD(&ent->list);
list_add_tail(&ent->list, &kprobe_blacklist);
}
(2014/07/02 15:56), Michael Ellerman wrote:
> On Wed, 2014-07-02 at 15:39 +0900, Masami Hiramatsu wrote:
>> (2014/07/02 13:41), Michael Ellerman wrote:
>>> On Tue, 2014-07-01 at 11:21 +0900, Masami Hiramatsu wrote:
>>>> (2014/06/30 20:36), Michael Ellerman wrote:
>>>>> On Mon, 2014-06-30 at 12:14 +0900, Masami Hiramatsu wrote:
>>>>>> Ping? :)
>>>>>
>>>>> Yeah sorry. I started looking at this and got dragged into another mess.
>>>>>
>>>>> You seem to have duplicated the functionality of arch_deref_entry_point(),
>>>>> which was also added for kprobes, and for the same reason - ie. because some
>>>>> arches have strange function pointers. Is there some reason you can't use it?
>>>>
>>>> Ah, right! Hmm, it seems some more work to update it. but basically, we can do.
>>>> BTW, is there any other users who need to access the actual function entry (for
>>>> kallsyms case)?
>>>
>>> Not that I'm aware of. We have had function descriptors on 64-bit powerpc for
>>> ever, so in theory by now we should have already found any cases where we need
>>> that sort of wrapper.
>>
>> OK, then I'll update this patch to use arch_deref_entry_point(), and add additional
>> patch which update to support PPC64 ABIv2.
>
> I've already done the latter:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=2f0143c91d30823f6f6e7d94d7fa818f7ab18a18
Oh, thanks!
In that case, my 1/2 patch should be dropped.
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
(2014/07/02 16:00), Masami Hiramatsu wrote:
> Since PowerPC64 ABIv2 doesn't have function descriptor
> any more, arch_deref_entry_point(), which returns function
> entry point from function descriptor, should be updated.
Please ignore this patch, since it is already fixed by Michael
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=2f0143c91d30823f6f6e7d94d7fa818f7ab18a18
We only need 2/2.
Thank you,
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Anoop Thomas Mathew <[email protected]>
> Cc: Jiri Kosina <[email protected]>
> Cc: [email protected]
> ---
> arch/powerpc/kernel/kprobes.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 90fab64..72a1034 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -491,7 +491,12 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
> return ret;
> }
>
> -#ifdef CONFIG_PPC64
> +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF == 1)
> +/*
> + * On PPC64 ABIv1 the function pointer actually points to the
> + * function's descriptor. The first entry in the descriptor is the
> + * address of the function text.
> + */
> unsigned long arch_deref_entry_point(void *entry)
> {
> return ((func_descr_t *)entry)->entry;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
Ping?
This patch can be applied without 1/2, and will fix ia64/ppc64 problem.
Thank you,
(2014/07/02 16:00), Masami Hiramatsu wrote:
> On ia64 and ppc64, the function pointer does not point the
> entry address of the function, but the address of function
> discriptor (which contains the entry address and misc
> data.) Since the kprobes passes the function pointer stored
> by NOKPROBE_SYMBOL() to kallsyms_lookup_size_offset() for
> initalizing its blacklist, it fails and reports many errors
> as below.
>
> Failed to find blacklist 0001013168300000
> Failed to find blacklist 0001013000f0a000
> Failed to find blacklist 000101315f70a000
> Failed to find blacklist 000101324c80a000
> Failed to find blacklist 0001013063f0a000
> Failed to find blacklist 000101327800a000
> Failed to find blacklist 0001013277f0a000
> Failed to find blacklist 000101315a70a000
> Failed to find blacklist 0001013277e0a000
> Failed to find blacklist 000101305a20a000
> Failed to find blacklist 0001013277d0a000
> Failed to find blacklist 00010130bdc0a000
> Failed to find blacklist 00010130dc20a000
> Failed to find blacklist 000101309a00a000
> Failed to find blacklist 0001013277c0a000
> Failed to find blacklist 0001013277b0a000
> Failed to find blacklist 0001013277a0a000
> Failed to find blacklist 000101327790a000
> Failed to find blacklist 000101303140a000
> Failed to find blacklist 0001013a3280a000
>
> To fix this bug, this introduces function_entry() macro to
> retrieve the entry address from the given function pointer,
> and uses for kallsyms_lookup_size_offset() while initializing
> blacklist.
>
> Changes in v5:
> - Use arch_deref_entry_point() instead of function_entry().
>
> Changes in v4:
> - Add kernel_text_address() check for verifying the address.
> - Moved on the latest linus tree.
>
> Changes in v3:
> - Fix a bug to get blacklist address based on function entry
> instead of function descriptor. (Suzuki's work, Thanks!)
>
> Changes in V2:
> - Use function_entry() macro when lookin up symbols instead
> of storing it.
> - Update for the latest -next.
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Reported-by: Tony Luck <[email protected]>
> Tested-by: Tony Luck <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Suzuki K. Poulose <[email protected]>
> Cc: Tony Luck <[email protected]>
> Cc: Fenghua Yu <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Ananth N Mavinakayanahalli <[email protected]>
> Cc: Kevin Hao <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> kernel/kprobes.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 3214289..ec370cc 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -32,6 +32,7 @@
> * <[email protected]> added function-return probes.
> */
> #include <linux/kprobes.h>
> +#include <linux/types.h>
> #include <linux/hash.h>
> #include <linux/init.h>
> #include <linux/slab.h>
> @@ -2037,19 +2038,23 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
> {
> unsigned long *iter;
> struct kprobe_blacklist_entry *ent;
> - unsigned long offset = 0, size = 0;
> + unsigned long entry, offset = 0, size = 0;
>
> for (iter = start; iter < end; iter++) {
> - if (!kallsyms_lookup_size_offset(*iter, &size, &offset)) {
> - pr_err("Failed to find blacklist %p\n", (void *)*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 blacklist at %p\n",
> + (void *)entry);
> continue;
> }
>
> ent = kmalloc(sizeof(*ent), GFP_KERNEL);
> if (!ent)
> return -ENOMEM;
> - ent->start_addr = *iter;
> - ent->end_addr = *iter + size;
> + ent->start_addr = entry;
> + ent->end_addr = entry + size;
> INIT_LIST_HEAD(&ent->list);
> list_add_tail(&ent->list, &kprobe_blacklist);
> }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
On Tue, Jul 8, 2014 at 5:07 AM, Masami Hiramatsu
<[email protected]> wrote:
> Ping?
>
> This patch can be applied without 1/2, and will fix ia64/ppc64 problem.
Is somebody going to push this upstream? Another week has gone by,
we are at -rc5, and I'm still seeing the
Failed to find blacklist a00000010133b150
messages on ia64.
-Tony
On Mon, 2014-07-14 at 10:17 -0700, Tony Luck wrote:
> On Tue, Jul 8, 2014 at 5:07 AM, Masami Hiramatsu
> <[email protected]> wrote:
> > Ping?
> >
> > This patch can be applied without 1/2, and will fix ia64/ppc64 problem.
>
> Is somebody going to push this upstream? Another week has gone by,
> we are at -rc5, and I'm still seeing the
>
> Failed to find blacklist a00000010133b150
>
> messages on ia64.
I don't see those messages on ppc64, I don't know where the original report
that it was broken on ppc64 came from. So I'm a bit lukewarm on the patch.
cheers
(2014/07/15 11:11), Michael Ellerman wrote:
> On Mon, 2014-07-14 at 10:17 -0700, Tony Luck wrote:
>> On Tue, Jul 8, 2014 at 5:07 AM, Masami Hiramatsu
>> <[email protected]> wrote:
>>> Ping?
>>>
>>> This patch can be applied without 1/2, and will fix ia64/ppc64 problem.
>>
>> Is somebody going to push this upstream? Another week has gone by,
>> we are at -rc5, and I'm still seeing the
>>
>> Failed to find blacklist a00000010133b150
>>
>> messages on ia64.
>
> I don't see those messages on ppc64, I don't know where the original report
> that it was broken on ppc64 came from. So I'm a bit lukewarm on the patch.
Right, on ppc64(ABIv1) it may be silently failed. Because each function
descriptor has another entry on kallsyms, original code can't detect
that. On the other hand, ia64's function descriptors have no entries,
so it can detect the failure on boot.
This patch adds new detection code using kernel_text_address() and
translates function descriptor to function entry.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
On Tue, 2014-07-15 at 11:24 +0900, Masami Hiramatsu wrote:
> (2014/07/15 11:11), Michael Ellerman wrote:
> > On Mon, 2014-07-14 at 10:17 -0700, Tony Luck wrote:
> >> On Tue, Jul 8, 2014 at 5:07 AM, Masami Hiramatsu
> >> <[email protected]> wrote:
> >>> Ping?
> >>>
> >>> This patch can be applied without 1/2, and will fix ia64/ppc64 problem.
> >>
> >> Is somebody going to push this upstream? Another week has gone by,
> >> we are at -rc5, and I'm still seeing the
> >>
> >> Failed to find blacklist a00000010133b150
> >>
> >> messages on ia64.
> >
> > I don't see those messages on ppc64, I don't know where the original report
> > that it was broken on ppc64 came from. So I'm a bit lukewarm on the patch.
>
> Right, on ppc64(ABIv1) it may be silently failed. Because each function
> descriptor has another entry on kallsyms, original code can't detect
> that.
OK, that would have been good to know :)
It's actually much worse than you describe. On ppc64 (ABIv1) we are
successfully blacklisting the function descriptors. But that doesn't prevent
you from probing the text address. So basically NOKPROBE_SYMBOL() does nothing
useful for us.
$ head -2 ../kprobes/blacklist
0xc000000000d4cff8-0xc000000000d4d010 notify_die
0xc000000000d4cf80-0xc000000000d4cf98 atomic_notifier_call_chain
$ echo "p:atomic_notifier_call_chain .atomic_notifier_call_chain" > kprobe_events
$ echo 1 > events/kprobes/enable
$ ls
available_events instances saved_cmdlines trace_options
available_tracers kprobe_events saved_cmdlines_size trace_pipe
buffer_size_kb kprobe_profile set_event tracing_cpumask
buffer_total_size_kb options snapshot tracing_max_latency
current_tracer per_cpu trace tracing_on
events printk_formats trace_clock tracing_thresh
free_buffer README trace_marker
$ Dumping ftrace buffer:
cpu 0x2: Vector: 400 (Instruction Access) at [c0000001defaf830]
pc: 0000000000000000
lr: 0000000000000001
sp: c0000001defafab0
msr: 8000000140009032
current = 0xc0000001def57e40
paca = 0xc00000000fe00800 softe: 0 irq_happened: 0x01
pid = 1, comm = swapper/2
cpu 0x3: Vector: 400 (Instruction Access) at [c0000001ddbcc640]
pc: 0000000000000000
lr: 0000000000000000
sp: c0000001ddbcc8c0
msr: 8000000040001032
current = 0xc0000001def5a100
paca = 0xc00000000fe00c00 softe: 0 irq_happened: 0x01
pid = -554326528, comm =
Dead machine.
With your patch:
$ head -2 kprobes/blacklist
0xc0000000000bf860-0xc0000000000bf8b0 .notify_die
0xc0000000000bf750-0xc0000000000bf780 .atomic_notifier_call_chain
$ echo "p:notify_die .notify_die" > tracing/kprobe_events
-bash: echo: write error: Invalid argument
So that is much better.
cheers
On Wed, 2014-07-02 at 07:00 +0000, Masami Hiramatsu wrote:
> On ia64 and ppc64, the function pointer does not point the
> entry address of the function, but the address of function
> discriptor (which contains the entry address and misc
> data.) Since the kprobes passes the function pointer stored
> by NOKPROBE_SYMBOL() to kallsyms_lookup_size_offset() for
> initalizing its blacklist, it fails and reports many errors
> as below.
>
> Failed to find blacklist 0001013168300000
> Failed to find blacklist 0001013000f0a000
> Failed to find blacklist 000101315f70a000
> Failed to find blacklist 000101324c80a000
> Failed to find blacklist 0001013063f0a000
> Failed to find blacklist 000101327800a000
> Failed to find blacklist 0001013277f0a000
> Failed to find blacklist 000101315a70a000
> Failed to find blacklist 0001013277e0a000
> Failed to find blacklist 000101305a20a000
> Failed to find blacklist 0001013277d0a000
> Failed to find blacklist 00010130bdc0a000
> Failed to find blacklist 00010130dc20a000
> Failed to find blacklist 000101309a00a000
> Failed to find blacklist 0001013277c0a000
> Failed to find blacklist 0001013277b0a000
> Failed to find blacklist 0001013277a0a000
> Failed to find blacklist 000101327790a000
> Failed to find blacklist 000101303140a000
> Failed to find blacklist 0001013a3280a000
>
> To fix this bug, this introduces function_entry() macro to
> retrieve the entry address from the given function pointer,
> and uses for kallsyms_lookup_size_offset() while initializing
> blacklist.
>
> Changes in v5:
> - Use arch_deref_entry_point() instead of function_entry().
>
> Changes in v4:
> - Add kernel_text_address() check for verifying the address.
> - Moved on the latest linus tree.
>
> Changes in v3:
> - Fix a bug to get blacklist address based on function entry
> instead of function descriptor. (Suzuki's work, Thanks!)
>
> Changes in V2:
> - Use function_entry() macro when lookin up symbols instead
> of storing it.
> - Update for the latest -next.
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Reported-by: Tony Luck <[email protected]>
> Tested-by: Tony Luck <[email protected]>
> Cc: Michael Ellerman <[email protected]>
Tested-by: Michael Ellerman <[email protected]>
Acked-by: Michael Ellerman <[email protected]> (for powerpc)
Ben, can you take this in your tree?
cheers
On Tue, 2014-07-15 at 13:19 +1000, Michael Ellerman wrote:
> > Signed-off-by: Masami Hiramatsu <[email protected]>
> > Reported-by: Tony Luck <[email protected]>
> > Tested-by: Tony Luck <[email protected]>
> > Cc: Michael Ellerman <[email protected]>
>
> Tested-by: Michael Ellerman <[email protected]>
> Acked-by: Michael Ellerman <[email protected]> (for powerpc)
>
> Ben, can you take this in your tree?
Acked-by: Benjamin Herrenschmidt <[email protected]>
That looks more like generic material. Do we have a kprobes maintainer ?
Andrew, do you want to take this ?
I'm happy to put it in powerpc and send it to Linus tomorrow if nobody
cares :-)
Cheers,
Ben.
(2014/07/15 16:16), Benjamin Herrenschmidt wrote:
> On Tue, 2014-07-15 at 13:19 +1000, Michael Ellerman wrote:
>
>>> Signed-off-by: Masami Hiramatsu <[email protected]>
>>> Reported-by: Tony Luck <[email protected]>
>>> Tested-by: Tony Luck <[email protected]>
>>> Cc: Michael Ellerman <[email protected]>
>>
>> Tested-by: Michael Ellerman <[email protected]>
>> Acked-by: Michael Ellerman <[email protected]> (for powerpc)
>>
>> Ben, can you take this in your tree?
>
> Acked-by: Benjamin Herrenschmidt <[email protected]>
>
> That looks more like generic material. Do we have a kprobes maintainer ?
> Andrew, do you want to take this ?
Yeah, I usually use Ingo's tip tree for kprobes maintenance.
Ingo, could you pull this as urgent-for-linus patch?
> I'm happy to put it in powerpc and send it to Linus tomorrow if nobody
> cares :-)
Thank you!
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
* Masami Hiramatsu <[email protected]> wrote:
> (2014/07/15 16:16), Benjamin Herrenschmidt wrote:
> > On Tue, 2014-07-15 at 13:19 +1000, Michael Ellerman wrote:
> >
> >>> Signed-off-by: Masami Hiramatsu <[email protected]>
> >>> Reported-by: Tony Luck <[email protected]>
> >>> Tested-by: Tony Luck <[email protected]>
> >>> Cc: Michael Ellerman <[email protected]>
> >>
> >> Tested-by: Michael Ellerman <[email protected]>
> >> Acked-by: Michael Ellerman <[email protected]> (for powerpc)
> >>
> >> Ben, can you take this in your tree?
> >
> > Acked-by: Benjamin Herrenschmidt <[email protected]>
> >
> > That looks more like generic material. Do we have a kprobes maintainer ?
> > Andrew, do you want to take this ?
>
> Yeah, I usually use Ingo's tip tree for kprobes maintenance.
> Ingo, could you pull this as urgent-for-linus patch?
Mind resending it in a separate thread with all acks added, etc?
Thanks,
Ingo
(2014/07/16 22:28), Ingo Molnar wrote:
>
> * Masami Hiramatsu <[email protected]> wrote:
>
>> (2014/07/15 16:16), Benjamin Herrenschmidt wrote:
>>> On Tue, 2014-07-15 at 13:19 +1000, Michael Ellerman wrote:
>>>
>>>>> Signed-off-by: Masami Hiramatsu <[email protected]>
>>>>> Reported-by: Tony Luck <[email protected]>
>>>>> Tested-by: Tony Luck <[email protected]>
>>>>> Cc: Michael Ellerman <[email protected]>
>>>>
>>>> Tested-by: Michael Ellerman <[email protected]>
>>>> Acked-by: Michael Ellerman <[email protected]> (for powerpc)
>>>>
>>>> Ben, can you take this in your tree?
>>>
>>> Acked-by: Benjamin Herrenschmidt <[email protected]>
>>>
>>> That looks more like generic material. Do we have a kprobes maintainer ?
>>> Andrew, do you want to take this ?
>>
>> Yeah, I usually use Ingo's tip tree for kprobes maintenance.
>> Ingo, could you pull this as urgent-for-linus patch?
>
> Mind resending it in a separate thread with all acks added, etc?
OK, I'll resend that soon.
BTW, I missed Suzuki's Signed-off-by in this version,
I'd like to recover that since he made a big bugfix on this.
Thanks,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
* Masami Hiramatsu <[email protected]> wrote:
> (2014/07/16 22:28), Ingo Molnar wrote:
> >
> > * Masami Hiramatsu <[email protected]> wrote:
> >
> >> (2014/07/15 16:16), Benjamin Herrenschmidt wrote:
> >>> On Tue, 2014-07-15 at 13:19 +1000, Michael Ellerman wrote:
> >>>
> >>>>> Signed-off-by: Masami Hiramatsu <[email protected]>
> >>>>> Reported-by: Tony Luck <[email protected]>
> >>>>> Tested-by: Tony Luck <[email protected]>
> >>>>> Cc: Michael Ellerman <[email protected]>
> >>>>
> >>>> Tested-by: Michael Ellerman <[email protected]>
> >>>> Acked-by: Michael Ellerman <[email protected]> (for powerpc)
> >>>>
> >>>> Ben, can you take this in your tree?
> >>>
> >>> Acked-by: Benjamin Herrenschmidt <[email protected]>
> >>>
> >>> That looks more like generic material. Do we have a kprobes maintainer ?
> >>> Andrew, do you want to take this ?
> >>
> >> Yeah, I usually use Ingo's tip tree for kprobes maintenance.
> >> Ingo, could you pull this as urgent-for-linus patch?
> >
> > Mind resending it in a separate thread with all acks added, etc?
>
> OK, I'll resend that soon.
> BTW, I missed Suzuki's Signed-off-by in this version,
> I'd like to recover that since he made a big bugfix on this.
That kind of credit can be added as:
Fixed-by: Suzuki K. Poulose <[email protected]>
and by also adding a sentence to the log message (outside of the
version changelogs which get stripped ususally).
Thanks,
Ingo