* [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 [not found] <536A16A6.5040709@hitachi.com> @ 2014-05-07 11:55 ` Masami Hiramatsu 2014-05-07 11:59 ` Masami Hiramatsu ` (2 more replies) 0 siblings, 3 replies; 26+ messages in thread From: Masami Hiramatsu @ 2014-05-07 11:55 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Tony Luck Cc: Jeremy Fitzhardinge, linux-ia64, sparse, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, Linus Torvalds, rdunlap, Linux Kernel Mailing List, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller 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 it in NOKPROBE_SYMBOL(). Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> Reported-by: Tony Luck <tony.luck@gmail.com> Cc: Tony Luck <tony.luck@intel.com> Cc: Fenghua Yu <fenghua.yu@intel.com> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Paul Mackerras <paulus@samba.org> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com> Cc: Kevin Hao <haokexin@gmail.com> Cc: linux-ia64@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org --- arch/ia64/include/asm/types.h | 2 ++ arch/powerpc/include/asm/types.h | 11 +++++++++++ include/linux/kprobes.h | 3 ++- include/linux/types.h | 4 ++++ 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/arch/ia64/include/asm/types.h b/arch/ia64/include/asm/types.h index 4c351b1..6ab7b6c 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 constant_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..fd297b8 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 constant_function_entry(fn) (((func_descr_t *)(fn))->entry) +#else +#define constant_function_entry(fn) ((unsigned long)(fn)) +#endif + #endif /* __ASSEMBLY__ */ #endif /* _ASM_POWERPC_TYPES_H */ diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h index e059507..637eafe 100644 --- a/include/linux/kprobes.h +++ b/include/linux/kprobes.h @@ -40,6 +40,7 @@ #include <linux/rcupdate.h> #include <linux/mutex.h> #include <linux/ftrace.h> +#include <linux/types.h> #ifdef CONFIG_KPROBES #include <asm/kprobes.h> @@ -485,7 +486,7 @@ static inline int enable_jprobe(struct jprobe *jp) #define __NOKPROBE_SYMBOL(fname) \ static unsigned long __used \ __attribute__((section("_kprobe_blacklist"))) \ - _kbl_addr_##fname = (unsigned long)fname; + _kbl_addr_##fname = constant_function_entry(fname); #define NOKPROBE_SYMBOL(fname) __NOKPROBE_SYMBOL(fname) #else #define NOKPROBE_SYMBOL(fname) diff --git a/include/linux/types.h b/include/linux/types.h index 4d118ba..78e2d7d 100644 --- a/include/linux/types.h +++ b/include/linux/types.h @@ -212,5 +212,9 @@ struct callback_head { }; #define rcu_head callback_head +#ifndef constant_function_entry +#define constant_function_entry(fn) ((unsigned long)(fn)) +#endif + #endif /* __ASSEMBLY__ */ #endif /* _LINUX_TYPES_H */ ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-07 11:55 ` [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 Masami Hiramatsu @ 2014-05-07 11:59 ` Masami Hiramatsu 2014-05-14 8:19 ` Masami Hiramatsu 2014-05-08 4:47 ` Ananth N Mavinakayanahalli 2014-05-26 11:25 ` Suzuki K. Poulose 2 siblings, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2014-05-07 11:59 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Tony Luck Cc: Jeremy Fitzhardinge, linux-ia64, sparse, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, Linus Torvalds, rdunlap, Linux Kernel Mailing List, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller Hi Tony, Benjamin and Paul, I've tried to fix this bug, but since I don't have either ppc64 nor ia64, this patch is not tested on those archs. Please review and test it on those machines. Thank you, (2014/05/07 20:55), 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 it in NOKPROBE_SYMBOL(). > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > Reported-by: Tony Luck <tony.luck@gmail.com> > Cc: Tony Luck <tony.luck@intel.com> > Cc: Fenghua Yu <fenghua.yu@intel.com> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com> > Cc: Kevin Hao <haokexin@gmail.com> > Cc: linux-ia64@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > --- > arch/ia64/include/asm/types.h | 2 ++ > arch/powerpc/include/asm/types.h | 11 +++++++++++ > include/linux/kprobes.h | 3 ++- > include/linux/types.h | 4 ++++ > 4 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/arch/ia64/include/asm/types.h b/arch/ia64/include/asm/types.h > index 4c351b1..6ab7b6c 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 constant_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..fd297b8 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 constant_function_entry(fn) (((func_descr_t *)(fn))->entry) > +#else > +#define constant_function_entry(fn) ((unsigned long)(fn)) > +#endif > + > #endif /* __ASSEMBLY__ */ > > #endif /* _ASM_POWERPC_TYPES_H */ > diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h > index e059507..637eafe 100644 > --- a/include/linux/kprobes.h > +++ b/include/linux/kprobes.h > @@ -40,6 +40,7 @@ > #include <linux/rcupdate.h> > #include <linux/mutex.h> > #include <linux/ftrace.h> > +#include <linux/types.h> > > #ifdef CONFIG_KPROBES > #include <asm/kprobes.h> > @@ -485,7 +486,7 @@ static inline int enable_jprobe(struct jprobe *jp) > #define __NOKPROBE_SYMBOL(fname) \ > static unsigned long __used \ > __attribute__((section("_kprobe_blacklist"))) \ > - _kbl_addr_##fname = (unsigned long)fname; > + _kbl_addr_##fname = constant_function_entry(fname); > #define NOKPROBE_SYMBOL(fname) __NOKPROBE_SYMBOL(fname) > #else > #define NOKPROBE_SYMBOL(fname) > diff --git a/include/linux/types.h b/include/linux/types.h > index 4d118ba..78e2d7d 100644 > --- a/include/linux/types.h > +++ b/include/linux/types.h > @@ -212,5 +212,9 @@ struct callback_head { > }; > #define rcu_head callback_head > > +#ifndef constant_function_entry > +#define constant_function_entry(fn) ((unsigned long)(fn)) > +#endif > + > #endif /* __ASSEMBLY__ */ > #endif /* _LINUX_TYPES_H */ > > > -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-07 11:59 ` Masami Hiramatsu @ 2014-05-14 8:19 ` Masami Hiramatsu 0 siblings, 0 replies; 26+ messages in thread From: Masami Hiramatsu @ 2014-05-14 8:19 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Tony Luck Cc: Jeremy Fitzhardinge, linux-ia64, sparse, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, Linus Torvalds, rdunlap, Linux Kernel Mailing List, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller (2014/05/07 20:59), Masami Hiramatsu wrote: > Hi Tony, Benjamin and Paul, > > I've tried to fix this bug, but since I don't have either ppc64 nor ia64, > this patch is not tested on those archs. Please review and test it on > those machines. Ping? I need your help since I don't have test environment. Thank you, > > Thank you, > > (2014/05/07 20:55), 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 it in NOKPROBE_SYMBOL(). >> >> >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> >> Reported-by: Tony Luck <tony.luck@gmail.com> >> Cc: Tony Luck <tony.luck@intel.com> >> Cc: Fenghua Yu <fenghua.yu@intel.com> >> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Cc: Paul Mackerras <paulus@samba.org> >> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com> >> Cc: Kevin Hao <haokexin@gmail.com> >> Cc: linux-ia64@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Cc: linuxppc-dev@lists.ozlabs.org >> --- >> arch/ia64/include/asm/types.h | 2 ++ >> arch/powerpc/include/asm/types.h | 11 +++++++++++ >> include/linux/kprobes.h | 3 ++- >> include/linux/types.h | 4 ++++ >> 4 files changed, 19 insertions(+), 1 deletion(-) >> >> diff --git a/arch/ia64/include/asm/types.h b/arch/ia64/include/asm/types.h >> index 4c351b1..6ab7b6c 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 constant_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..fd297b8 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 constant_function_entry(fn) (((func_descr_t *)(fn))->entry) >> +#else >> +#define constant_function_entry(fn) ((unsigned long)(fn)) >> +#endif >> + >> #endif /* __ASSEMBLY__ */ >> >> #endif /* _ASM_POWERPC_TYPES_H */ >> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h >> index e059507..637eafe 100644 >> --- a/include/linux/kprobes.h >> +++ b/include/linux/kprobes.h >> @@ -40,6 +40,7 @@ >> #include <linux/rcupdate.h> >> #include <linux/mutex.h> >> #include <linux/ftrace.h> >> +#include <linux/types.h> >> >> #ifdef CONFIG_KPROBES >> #include <asm/kprobes.h> >> @@ -485,7 +486,7 @@ static inline int enable_jprobe(struct jprobe *jp) >> #define __NOKPROBE_SYMBOL(fname) \ >> static unsigned long __used \ >> __attribute__((section("_kprobe_blacklist"))) \ >> - _kbl_addr_##fname = (unsigned long)fname; >> + _kbl_addr_##fname = constant_function_entry(fname); >> #define NOKPROBE_SYMBOL(fname) __NOKPROBE_SYMBOL(fname) >> #else >> #define NOKPROBE_SYMBOL(fname) >> diff --git a/include/linux/types.h b/include/linux/types.h >> index 4d118ba..78e2d7d 100644 >> --- a/include/linux/types.h >> +++ b/include/linux/types.h >> @@ -212,5 +212,9 @@ struct callback_head { >> }; >> #define rcu_head callback_head >> >> +#ifndef constant_function_entry >> +#define constant_function_entry(fn) ((unsigned long)(fn)) >> +#endif >> + >> #endif /* __ASSEMBLY__ */ >> #endif /* _LINUX_TYPES_H */ >> >> >> > > -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-07 11:55 ` [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 Masami Hiramatsu 2014-05-07 11:59 ` Masami Hiramatsu @ 2014-05-08 4:47 ` Ananth N Mavinakayanahalli 2014-05-08 5:40 ` Masami Hiramatsu 2014-05-26 11:25 ` Suzuki K. Poulose 2 siblings, 1 reply; 26+ messages in thread From: Ananth N Mavinakayanahalli @ 2014-05-08 4:47 UTC (permalink / raw) To: Masami Hiramatsu Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, Linus Torvalds, rdunlap, Tony Luck, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller On Wed, May 07, 2014 at 08:55:51PM +0900, Masami Hiramatsu wrote: ... > +#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 constant_function_entry(fn) (((func_descr_t *)(fn))->entry) > +#else > +#define constant_function_entry(fn) ((unsigned long)(fn)) > +#endif > + > #endif /* __ASSEMBLY__ */ Hi Masami, You could just use ppc_function_entry() instead. Ananth ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-08 4:47 ` Ananth N Mavinakayanahalli @ 2014-05-08 5:40 ` Masami Hiramatsu 2014-05-08 6:16 ` Ananth N Mavinakayanahalli 0 siblings, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2014-05-08 5:40 UTC (permalink / raw) To: ananth Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, Linus Torvalds, rdunlap, Tony Luck, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller (2014/05/08 13:47), Ananth N Mavinakayanahalli wrote: > On Wed, May 07, 2014 at 08:55:51PM +0900, Masami Hiramatsu wrote: > > ... > >> +#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 constant_function_entry(fn) (((func_descr_t *)(fn))->entry) >> +#else >> +#define constant_function_entry(fn) ((unsigned long)(fn)) >> +#endif >> + >> #endif /* __ASSEMBLY__ */ > > Hi Masami, > > You could just use ppc_function_entry() instead. No, I think ppc_function_entry() has two problems (on the latest -next kernel) At first, that is an inlined functions which is not applied in build time. Since the NOKPROBE_SYMBOL() is used outside of any functions as like as EXPORT_SYMBOL(), we can only use preprocessed macros. Next, on PPC64 ABI*v2*, ppc_function_entry() returns local function entry, which seems global function entry + 2 insns. I'm not sure about implementation of the kallsyms on PPC64 ABIv2, but I guess we need global function entry for kallsyms. BTW, could you test this patch on the latest -next tree on PPC64 if possible? Thank you, -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-08 5:40 ` Masami Hiramatsu @ 2014-05-08 6:16 ` Ananth N Mavinakayanahalli 2014-05-09 8:06 ` Masami Hiramatsu 0 siblings, 1 reply; 26+ messages in thread From: Ananth N Mavinakayanahalli @ 2014-05-08 6:16 UTC (permalink / raw) To: Masami Hiramatsu Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, Linus Torvalds, rdunlap, Tony Luck, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller On Thu, May 08, 2014 at 02:40:00PM +0900, Masami Hiramatsu wrote: > (2014/05/08 13:47), Ananth N Mavinakayanahalli wrote: > > On Wed, May 07, 2014 at 08:55:51PM +0900, Masami Hiramatsu wrote: > > > > ... > > > >> +#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 constant_function_entry(fn) (((func_descr_t *)(fn))->entry) > >> +#else > >> +#define constant_function_entry(fn) ((unsigned long)(fn)) > >> +#endif > >> + > >> #endif /* __ASSEMBLY__ */ > > > > Hi Masami, > > > > You could just use ppc_function_entry() instead. > > No, I think ppc_function_entry() has two problems (on the latest -next kernel) > > At first, that is an inlined functions which is not applied in build time. > Since the NOKPROBE_SYMBOL() is used outside of any functions as like as > EXPORT_SYMBOL(), we can only use preprocessed macros. > Next, on PPC64 ABI*v2*, ppc_function_entry() returns local function entry, > which seems global function entry + 2 insns. I'm not sure about implementation > of the kallsyms on PPC64 ABIv2, but I guess we need global function entry > for kallsyms. ABIv2 does away with function descriptors and Anton fixed up that routine to handle the change (the +2 is an artefact of that). > BTW, could you test this patch on the latest -next tree on PPC64 if possible? I'll test it, but it may take a bit. Ananth ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-08 6:16 ` Ananth N Mavinakayanahalli @ 2014-05-09 8:06 ` Masami Hiramatsu 0 siblings, 0 replies; 26+ messages in thread From: Masami Hiramatsu @ 2014-05-09 8:06 UTC (permalink / raw) To: ananth Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, Linus Torvalds, rdunlap, Tony Luck, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller (2014/05/08 15:16), Ananth N Mavinakayanahalli wrote: > On Thu, May 08, 2014 at 02:40:00PM +0900, Masami Hiramatsu wrote: >> (2014/05/08 13:47), Ananth N Mavinakayanahalli wrote: >>> On Wed, May 07, 2014 at 08:55:51PM +0900, Masami Hiramatsu wrote: >>> >>> ... >>> >>>> +#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 constant_function_entry(fn) (((func_descr_t *)(fn))->entry) >>>> +#else >>>> +#define constant_function_entry(fn) ((unsigned long)(fn)) >>>> +#endif >>>> + >>>> #endif /* __ASSEMBLY__ */ >>> >>> Hi Masami, >>> >>> You could just use ppc_function_entry() instead. >> >> No, I think ppc_function_entry() has two problems (on the latest -next kernel) >> >> At first, that is an inlined functions which is not applied in build time. >> Since the NOKPROBE_SYMBOL() is used outside of any functions as like as >> EXPORT_SYMBOL(), we can only use preprocessed macros. >> Next, on PPC64 ABI*v2*, ppc_function_entry() returns local function entry, >> which seems global function entry + 2 insns. I'm not sure about implementation >> of the kallsyms on PPC64 ABIv2, but I guess we need global function entry >> for kallsyms. > > ABIv2 does away with function descriptors and Anton fixed up that > routine to handle the change (the +2 is an artefact of that). Hmm, do you mean that the address +2 is the actual entry point? I'd like to know which address is same as the address shown in /proc/kallsyms. >> BTW, could you test this patch on the latest -next tree on PPC64 if possible? > > I'll test it, but it may take a bit. Thanks for your help! > > Ananth > > -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-07 11:55 ` [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 Masami Hiramatsu 2014-05-07 11:59 ` Masami Hiramatsu 2014-05-08 4:47 ` Ananth N Mavinakayanahalli @ 2014-05-26 11:25 ` Suzuki K. Poulose 2014-05-26 11:48 ` Masami Hiramatsu 2014-05-27 6:31 ` [RFT PATCH -next v2] " Masami Hiramatsu 2 siblings, 2 replies; 26+ messages in thread From: Suzuki K. Poulose @ 2014-05-26 11:25 UTC (permalink / raw) To: Masami Hiramatsu, Benjamin Herrenschmidt, Paul Mackerras, Tony Luck Cc: Jeremy Fitzhardinge, linux-ia64, sparse, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, Linus Torvalds, rdunlap, Linux Kernel Mailing List, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller On 05/07/2014 05:25 PM, 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 it in NOKPROBE_SYMBOL(). > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > Reported-by: Tony Luck <tony.luck@gmail.com> > Cc: Tony Luck <tony.luck@intel.com> > Cc: Fenghua Yu <fenghua.yu@intel.com> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com> > Cc: Kevin Hao <haokexin@gmail.com> > Cc: linux-ia64@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > --- > arch/ia64/include/asm/types.h | 2 ++ > arch/powerpc/include/asm/types.h | 11 +++++++++++ > include/linux/kprobes.h | 3 ++- > include/linux/types.h | 4 ++++ > 4 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/arch/ia64/include/asm/types.h b/arch/ia64/include/asm/types.h > index 4c351b1..6ab7b6c 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 constant_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..fd297b8 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 constant_function_entry(fn) (((func_descr_t *)(fn))->entry) > +#else > +#define constant_function_entry(fn) ((unsigned long)(fn)) > +#endif > + > #endif /* __ASSEMBLY__ */ > > #endif /* _ASM_POWERPC_TYPES_H */ > diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h > index e059507..637eafe 100644 > --- a/include/linux/kprobes.h > +++ b/include/linux/kprobes.h > @@ -40,6 +40,7 @@ > #include <linux/rcupdate.h> > #include <linux/mutex.h> > #include <linux/ftrace.h> > +#include <linux/types.h> > > #ifdef CONFIG_KPROBES > #include <asm/kprobes.h> > @@ -485,7 +486,7 @@ static inline int enable_jprobe(struct jprobe *jp) > #define __NOKPROBE_SYMBOL(fname) \ > static unsigned long __used \ > __attribute__((section("_kprobe_blacklist"))) \ > - _kbl_addr_##fname = (unsigned long)fname; > + _kbl_addr_##fname = constant_function_entry(fname); > #define NOKPROBE_SYMBOL(fname) __NOKPROBE_SYMBOL(fname) Throws up build errors for me : CC kernel/notifier.o kernel/notifier.c:105:1: error: initializer element is not constant NOKPROBE_SYMBOL(notifier_call_chain); ^ kernel/notifier.c:188:1: error: initializer element is not constant NOKPROBE_SYMBOL(__atomic_notifier_call_chain); ^ kernel/notifier.c:196:1: error: initializer element is not constant NOKPROBE_SYMBOL(atomic_notifier_call_chain); ^ kernel/notifier.c:546:1: error: initializer element is not constant NOKPROBE_SYMBOL(notify_die); ^ make[1]: *** [kernel/notifier.o] Error 1 make: *** [kernel] Error 2 Thanks Suzuki ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-26 11:25 ` Suzuki K. Poulose @ 2014-05-26 11:48 ` Masami Hiramatsu 2014-05-27 6:31 ` [RFT PATCH -next v2] " Masami Hiramatsu 1 sibling, 0 replies; 26+ messages in thread From: Masami Hiramatsu @ 2014-05-26 11:48 UTC (permalink / raw) To: Suzuki K. Poulose Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller (2014/05/26 20:25), Suzuki K. Poulose wrote: > On 05/07/2014 05:25 PM, 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 it in NOKPROBE_SYMBOL(). >> >> >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> >> Reported-by: Tony Luck <tony.luck@gmail.com> >> Cc: Tony Luck <tony.luck@intel.com> >> Cc: Fenghua Yu <fenghua.yu@intel.com> >> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Cc: Paul Mackerras <paulus@samba.org> >> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com> >> Cc: Kevin Hao <haokexin@gmail.com> >> Cc: linux-ia64@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Cc: linuxppc-dev@lists.ozlabs.org >> --- >> arch/ia64/include/asm/types.h | 2 ++ >> arch/powerpc/include/asm/types.h | 11 +++++++++++ >> include/linux/kprobes.h | 3 ++- >> include/linux/types.h | 4 ++++ >> 4 files changed, 19 insertions(+), 1 deletion(-) >> >> diff --git a/arch/ia64/include/asm/types.h b/arch/ia64/include/asm/types.h >> index 4c351b1..6ab7b6c 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 constant_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..fd297b8 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 constant_function_entry(fn) (((func_descr_t *)(fn))->entry) >> +#else >> +#define constant_function_entry(fn) ((unsigned long)(fn)) >> +#endif >> + >> #endif /* __ASSEMBLY__ */ >> >> #endif /* _ASM_POWERPC_TYPES_H */ >> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h >> index e059507..637eafe 100644 >> --- a/include/linux/kprobes.h >> +++ b/include/linux/kprobes.h >> @@ -40,6 +40,7 @@ >> #include <linux/rcupdate.h> >> #include <linux/mutex.h> >> #include <linux/ftrace.h> >> +#include <linux/types.h> >> >> #ifdef CONFIG_KPROBES >> #include <asm/kprobes.h> >> @@ -485,7 +486,7 @@ static inline int enable_jprobe(struct jprobe *jp) >> #define __NOKPROBE_SYMBOL(fname) \ >> static unsigned long __used \ >> __attribute__((section("_kprobe_blacklist"))) \ >> - _kbl_addr_##fname = (unsigned long)fname; >> + _kbl_addr_##fname = constant_function_entry(fname); >> #define NOKPROBE_SYMBOL(fname) __NOKPROBE_SYMBOL(fname) > > > Throws up build errors for me : > > CC kernel/notifier.o > kernel/notifier.c:105:1: error: initializer element is not constant > NOKPROBE_SYMBOL(notifier_call_chain); > ^ > kernel/notifier.c:188:1: error: initializer element is not constant > NOKPROBE_SYMBOL(__atomic_notifier_call_chain); > ^ > kernel/notifier.c:196:1: error: initializer element is not constant > NOKPROBE_SYMBOL(atomic_notifier_call_chain); > ^ > kernel/notifier.c:546:1: error: initializer element is not constant > NOKPROBE_SYMBOL(notify_die); > ^ > make[1]: *** [kernel/notifier.o] Error 1 > make: *** [kernel] Error 2 Thanks for the test! >> +#define constant_function_entry(fn) (((func_descr_t *)(fn))->entry) Ah! right, This is not allowed for initialization... I'll update it to deref pointer when it is used. Thank you again! > > Thanks > Suzuki > > -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFT PATCH -next v2] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-26 11:25 ` Suzuki K. Poulose 2014-05-26 11:48 ` Masami Hiramatsu @ 2014-05-27 6:31 ` Masami Hiramatsu 2014-05-29 19:13 ` Suzuki K. Poulose 1 sibling, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2014-05-27 6:31 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Tony Luck Cc: Jeremy Fitzhardinge, linux-ia64, sparse, H. Peter Anvin, akataria, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Suzuki K. Poulose, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, Thomas Gleixner, Tony Luck, Kevin Hao, Linus Torvalds, rdunlap, Linux Kernel Mailing List, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller 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 V2: - Use function_entry() macro when lookin up symbols instead of storing it. - Update for the latest -next. Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> Reported-by: Tony Luck <tony.luck@gmail.com> Cc: Suzuki K. Poulose <suzuki@in.ibm.com> Cc: Tony Luck <tony.luck@intel.com> Cc: Fenghua Yu <fenghua.yu@intel.com> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Paul Mackerras <paulus@samba.org> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com> Cc: Kevin Hao <haokexin@gmail.com> Cc: linux-ia64@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org --- arch/ia64/include/asm/types.h | 2 ++ arch/powerpc/include/asm/types.h | 11 +++++++++++ include/linux/types.h | 4 ++++ kernel/kprobes.c | 4 +++- 4 files changed, 20 insertions(+), 1 deletion(-) 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 2ac9f13..3859c88 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -32,6 +32,7 @@ * <prasanna@in.ibm.com> added function-return probes. */ #include <linux/kprobes.h> +#include <linux/types.h> #include <linux/hash.h> #include <linux/init.h> #include <linux/slab.h> @@ -2042,7 +2043,8 @@ static int __init populate_kprobe_blacklist(unsigned long *start, unsigned long offset = 0, size = 0; for (iter = start; iter < end; iter++) { - if (!kallsyms_lookup_size_offset(*iter, &size, &offset)) { + if (!kallsyms_lookup_size_offset(function_entry(*iter), + &size, &offset)) { pr_err("Failed to find blacklist %p\n", (void *)*iter); continue; } ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next v2] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-27 6:31 ` [RFT PATCH -next v2] " Masami Hiramatsu @ 2014-05-29 19:13 ` Suzuki K. Poulose 2014-05-30 2:47 ` Masami Hiramatsu 0 siblings, 1 reply; 26+ messages in thread From: Suzuki K. Poulose @ 2014-05-29 19:13 UTC (permalink / raw) To: Masami Hiramatsu, Benjamin Herrenschmidt, Paul Mackerras, Tony Luck Cc: Jeremy Fitzhardinge, linux-ia64, sparse, H. Peter Anvin, akataria, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, Thomas Gleixner, Tony Luck, Kevin Hao, Linus Torvalds, rdunlap, Linux Kernel Mailing List, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller On 05/27/2014 12:01 PM, 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 V2: > - Use function_entry() macro when lookin up symbols instead > of storing it. > - Update for the latest -next. > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > Reported-by: Tony Luck <tony.luck@gmail.com> > Cc: Suzuki K. Poulose <suzuki@in.ibm.com> > Cc: Tony Luck <tony.luck@intel.com> > Cc: Fenghua Yu <fenghua.yu@intel.com> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com> > Cc: Kevin Hao <haokexin@gmail.com> > Cc: linux-ia64@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > --- > arch/ia64/include/asm/types.h | 2 ++ > arch/powerpc/include/asm/types.h | 11 +++++++++++ > include/linux/types.h | 4 ++++ > kernel/kprobes.c | 4 +++- > 4 files changed, 20 insertions(+), 1 deletion(-) > > 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 2ac9f13..3859c88 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -32,6 +32,7 @@ > * <prasanna@in.ibm.com> added function-return probes. > */ > #include <linux/kprobes.h> > +#include <linux/types.h> > #include <linux/hash.h> > #include <linux/init.h> > #include <linux/slab.h> > @@ -2042,7 +2043,8 @@ static int __init populate_kprobe_blacklist(unsigned long *start, > unsigned long offset = 0, size = 0; > > for (iter = start; iter < end; iter++) { > - if (!kallsyms_lookup_size_offset(*iter, &size, &offset)) { > + if (!kallsyms_lookup_size_offset(function_entry(*iter), > + &size, &offset)) { On powerpc we will be able to resolve the *iter to func_descr and won't get the below error with/without this patch. So we have to actually verify the kprobe_blacklist contents to make sure everything is alright. > pr_err("Failed to find blacklist %p\n", (void *)*iter); > continue; > } > There is a bug here. You need to set the ent->start using the function_entry(*iter) and not *iter. Or else you just avoid the 'Warning' and still have an invalid black list. As shown below : 2e:mon> ls kprobe_blacklist kprobe_blacklist: c00000000104dad0 2e:mon> d c00000000104dad0 10 c00000000104dad0: c0000003aff800a0 c0000003aff809a0 2e:mon> d c0000003aff800a0 20 (struct kprobe_blacklist *) c0000003aff800a0: c0000003aff800c0 c00000000104dad0 c0000003aff800b0: c0000000010ef138 c0000000010ef188 start ^^ end ^^ 2e:mon> la c0000000010ef138 (start) c0000000010ef138: notify_die+0x0/0x10 <- still points to the function descriptor 2e:mon> la c0000000010ef188 (end) c0000000010ef188: __blocking_notifier_call_chain+0x0/0x10 Following patch fixes the issue, with the patch : 1:mon> ls kprobe_blacklist kprobe_blacklist: c00000000104dad0 1:mon> d c00000000104dad0 10 c00000000104dad0: c0000003ae1a00a0 c0000003ae1a09a0 1:mon> d c0000003ae1a00a0 20 (struct kprobe_blacklist *) c0000003ae1a00a0: c0000003ae1a00c0 c00000000104dad0 c0000003ae1a00b0: c0000000000b14d0 c0000000000b1520 start ^^ end ^^ 1:mon> la c0000000000b14d0 c0000000000b14d0: .notify_die+0x0/0x50 1:mon> la c0000000000b1520 c0000000000b1520: .atomic_notifier_chain_register+0x0/0xa0 1:mon> di c0000000000b14d0 10 (.notify_die) c0000000000b14d0 7c0802a6 mflr r0 c0000000000b14d4 7c691b78 mr r9,r3 commit ed51674aca8e0496641f565421ab6691a873e80a Author: Suzuki K. Poulose <suzuki@in.ibm.com> Date: Fri May 30 00:23:01 2014 +0530 diff --git a/kernel/kprobes.c b/kernel/kprobes.c index 3859c88..b81d626 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -2043,7 +2043,8 @@ static int __init populate_kprobe_blacklist(unsigned long *start, unsigned long offset = 0, size = 0; for (iter = start; iter < end; iter++) { - if (!kallsyms_lookup_size_offset(function_entry(*iter), + unsigned long entry = function_entry(*iter); + if (!kallsyms_lookup_size_offset(entry, &size, &offset)) { pr_err("Failed to find blacklist %p\n", (void *)*iter); continue; @@ -2052,8 +2053,8 @@ static int __init populate_kprobe_blacklist(unsigned long *start, 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); } Thanks Suzuki ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next v2] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-29 19:13 ` Suzuki K. Poulose @ 2014-05-30 2:47 ` Masami Hiramatsu 2014-05-30 3:18 ` [RFT PATCH -next v3] " Masami Hiramatsu 0 siblings, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2014-05-30 2:47 UTC (permalink / raw) To: Suzuki K. Poulose Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, akataria, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, Thomas Gleixner, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller (2014/05/30 4:13), Suzuki K. Poulose wrote: >> @@ -2042,7 +2043,8 @@ static int __init populate_kprobe_blacklist(unsigned long *start, >> unsigned long offset = 0, size = 0; >> >> for (iter = start; iter < end; iter++) { >> - if (!kallsyms_lookup_size_offset(*iter, &size, &offset)) { >> + if (!kallsyms_lookup_size_offset(function_entry(*iter), >> + &size, &offset)) { > > On powerpc we will be able to resolve the *iter to func_descr and won't > get the below error with/without this patch. So we have to actually > verify the kprobe_blacklist contents to make sure everything is alright. > >> pr_err("Failed to find blacklist %p\n", (void *)*iter); >> continue; >> } >> > > There is a bug here. > You need to set the ent->start using the function_entry(*iter) and not > *iter. Or else you just avoid the 'Warning' and still have an invalid > black list. As shown below : > > 2e:mon> ls kprobe_blacklist > kprobe_blacklist: c00000000104dad0 > 2e:mon> d c00000000104dad0 10 > c00000000104dad0: c0000003aff800a0 c0000003aff809a0 > 2e:mon> d c0000003aff800a0 20 (struct kprobe_blacklist *) > c0000003aff800a0: c0000003aff800c0 c00000000104dad0 > c0000003aff800b0: c0000000010ef138 c0000000010ef188 > start ^^ end ^^ > 2e:mon> la c0000000010ef138 (start) > c0000000010ef138: notify_die+0x0/0x10 <- still points to the > function descriptor > 2e:mon> la c0000000010ef188 (end) > c0000000010ef188: __blocking_notifier_call_chain+0x0/0x10 > > > Following patch fixes the issue, with the patch : > > 1:mon> ls kprobe_blacklist > kprobe_blacklist: c00000000104dad0 > 1:mon> d c00000000104dad0 10 > c00000000104dad0: c0000003ae1a00a0 c0000003ae1a09a0 > 1:mon> d c0000003ae1a00a0 20 (struct kprobe_blacklist *) > c0000003ae1a00a0: c0000003ae1a00c0 c00000000104dad0 > c0000003ae1a00b0: c0000000000b14d0 c0000000000b1520 > start ^^ end ^^ > 1:mon> la c0000000000b14d0 > c0000000000b14d0: .notify_die+0x0/0x50 > 1:mon> la c0000000000b1520 > c0000000000b1520: .atomic_notifier_chain_register+0x0/0xa0 > > 1:mon> di c0000000000b14d0 10 (.notify_die) > c0000000000b14d0 7c0802a6 mflr r0 > c0000000000b14d4 7c691b78 mr r9,r3 > > commit ed51674aca8e0496641f565421ab6691a873e80a > Author: Suzuki K. Poulose <suzuki@in.ibm.com> > Date: Fri May 30 00:23:01 2014 +0530 > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index 3859c88..b81d626 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -2043,7 +2043,8 @@ static int __init > populate_kprobe_blacklist(unsigned long *start, > unsigned long offset = 0, size = 0; > > for (iter = start; iter < end; iter++) { > - if (!kallsyms_lookup_size_offset(function_entry(*iter), > + unsigned long entry = function_entry(*iter); > + if (!kallsyms_lookup_size_offset(entry, > &size, &offset)) { > pr_err("Failed to find blacklist %p\n", (void > *)*iter); > continue; > @@ -2052,8 +2053,8 @@ static int __init > populate_kprobe_blacklist(unsigned long *start, > 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; Oops! right, I missed it :( I'll update the patch including your fix and signed-off-by. thank you! -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-30 2:47 ` Masami Hiramatsu @ 2014-05-30 3:18 ` Masami Hiramatsu 2014-06-06 6:38 ` Masami Hiramatsu 0 siblings, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2014-05-30 3:18 UTC (permalink / raw) To: Benjamin Herrenschmidt, Suzuki K. Poulose, Tony Luck, Paul Mackerras Cc: Jeremy Fitzhardinge, linux-ia64, sparse, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, Linus Torvalds, rdunlap, Linux Kernel Mailing List, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller 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 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 <masami.hiramatsu.pt@hitachi.com> Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com> Reported-by: Tony Luck <tony.luck@gmail.com> Cc: Suzuki K. Poulose <suzuki@in.ibm.com> Cc: Tony Luck <tony.luck@intel.com> Cc: Fenghua Yu <fenghua.yu@intel.com> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Paul Mackerras <paulus@samba.org> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com> Cc: Kevin Hao <haokexin@gmail.com> Cc: linux-ia64@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org --- arch/ia64/include/asm/types.h | 2 ++ arch/powerpc/include/asm/types.h | 11 +++++++++++ include/linux/types.h | 4 ++++ kernel/kprobes.c | 11 +++++++---- 4 files changed, 24 insertions(+), 4 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 2ac9f13..3f2d6d4 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -32,6 +32,7 @@ * <prasanna@in.ibm.com> added function-return probes. */ #include <linux/kprobes.h> +#include <linux/types.h> #include <linux/hash.h> #include <linux/init.h> #include <linux/slab.h> @@ -2042,16 +2043,18 @@ static int __init populate_kprobe_blacklist(unsigned long *start, unsigned long 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); + unsigned long entry = function_entry(*iter); + if (!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); } ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-05-30 3:18 ` [RFT PATCH -next v3] " Masami Hiramatsu @ 2014-06-06 6:38 ` Masami Hiramatsu 2014-06-17 23:03 ` Tony Luck 2014-06-18 7:56 ` Michael Ellerman 0 siblings, 2 replies; 26+ messages in thread From: Masami Hiramatsu @ 2014-06-06 6:38 UTC (permalink / raw) To: Benjamin Herrenschmidt, Suzuki K. Poulose, Tony Luck, Paul Mackerras Cc: Jeremy Fitzhardinge, linux-ia64, sparse, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, Linus Torvalds, rdunlap, Linux Kernel Mailing List, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller Ping? I guess this should go to 3.16 branch, shouldn't it? (2014/05/30 12:18), 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 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 <masami.hiramatsu.pt@hitachi.com> > Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com> > Reported-by: Tony Luck <tony.luck@gmail.com> > Cc: Suzuki K. Poulose <suzuki@in.ibm.com> > Cc: Tony Luck <tony.luck@intel.com> > Cc: Fenghua Yu <fenghua.yu@intel.com> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com> > Cc: Kevin Hao <haokexin@gmail.com> > Cc: linux-ia64@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > --- > arch/ia64/include/asm/types.h | 2 ++ > arch/powerpc/include/asm/types.h | 11 +++++++++++ > include/linux/types.h | 4 ++++ > kernel/kprobes.c | 11 +++++++---- > 4 files changed, 24 insertions(+), 4 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 2ac9f13..3f2d6d4 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -32,6 +32,7 @@ > * <prasanna@in.ibm.com> added function-return probes. > */ > #include <linux/kprobes.h> > +#include <linux/types.h> > #include <linux/hash.h> > #include <linux/init.h> > #include <linux/slab.h> > @@ -2042,16 +2043,18 @@ static int __init populate_kprobe_blacklist(unsigned long *start, > unsigned long 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); > + unsigned long entry = function_entry(*iter); > + if (!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: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-06-06 6:38 ` Masami Hiramatsu @ 2014-06-17 23:03 ` Tony Luck 2014-06-18 7:56 ` Michael Ellerman 1 sibling, 0 replies; 26+ messages in thread From: Tony Luck @ 2014-06-17 23:03 UTC (permalink / raw) To: Masami Hiramatsu Cc: Jeremy Fitzhardinge, linux-ia64@vger.kernel.org, sparse, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Suzuki K. Poulose, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Kevin Hao, Linus Torvalds, rdunlap, Linux Kernel Mailing List, dl9pf, Andrew Morton, linuxppc-dev, David S. Miller On Thu, Jun 5, 2014 at 11:38 PM, Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote: > Ping? > > I guess this should go to 3.16 branch, shouldn't it? > > (2014/05/30 12:18), 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 Yes please ... just found this problem on ia64 in mainline and was happy to see this fix for it. Tested-by: Tony Luck <tony.luck@intel.com> -Tony ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-06-06 6:38 ` Masami Hiramatsu 2014-06-17 23:03 ` Tony Luck @ 2014-06-18 7:56 ` Michael Ellerman 2014-06-18 8:46 ` Masami Hiramatsu 1 sibling, 1 reply; 26+ messages in thread From: Michael Ellerman @ 2014-06-18 7:56 UTC (permalink / raw) To: Masami Hiramatsu Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Suzuki K. Poulose, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller On Fri, 2014-06-06 at 15:38 +0900, Masami Hiramatsu wrote: > Ping? > > I guess this should go to 3.16 branch, shouldn't it? > > 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 We already have ppc_function_entry(), can't you use that? cheers ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-06-18 7:56 ` Michael Ellerman @ 2014-06-18 8:46 ` Masami Hiramatsu 2014-06-19 1:30 ` Michael Ellerman 0 siblings, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2014-06-18 8:46 UTC (permalink / raw) To: Michael Ellerman Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Suzuki K. Poulose, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller (2014/06/18 16:56), Michael Ellerman wrote: > On Fri, 2014-06-06 at 15:38 +0900, Masami Hiramatsu wrote: >> Ping? >> >> I guess this should go to 3.16 branch, shouldn't it? > >>> 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 > > We already have ppc_function_entry(), can't you use that? I'd like to ask you whether the address which ppc_function_entry() returns on PPC ABIv2 is really same address in kallsyms or not. As you can see, kprobes uses function_entry() to get the actual entry address where kallsyms knows. I have not much information about that, but it seems that the "global entry point" is the address which kallsyms knows, isn't it? Thank you, -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-06-18 8:46 ` Masami Hiramatsu @ 2014-06-19 1:30 ` Michael Ellerman 2014-06-19 4:52 ` Masami Hiramatsu 0 siblings, 1 reply; 26+ messages in thread From: Michael Ellerman @ 2014-06-19 1:30 UTC (permalink / raw) To: Masami Hiramatsu Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Suzuki K. Poulose, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller On Wed, 2014-06-18 at 17:46 +0900, Masami Hiramatsu wrote: > (2014/06/18 16:56), Michael Ellerman wrote: > > On Fri, 2014-06-06 at 15:38 +0900, Masami Hiramatsu wrote: > >> Ping? > >> > >> I guess this should go to 3.16 branch, shouldn't it? > > > >>> 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 > > > > We already have ppc_function_entry(), can't you use that? > > I'd like to ask you whether the address which ppc_function_entry() returns on > PPC ABIv2 is really same address in kallsyms or not. > As you can see, kprobes uses function_entry() to get the actual entry address > where kallsyms knows. I have not much information about that, but it seems that > the "global entry point" is the address which kallsyms knows, isn't it? OK. I'm not sure off the top of my head which address kallsyms knows about, but yes it's likely that it is the global entry point. I recently sent a patch to add ppc_global_function_entry(), because we need it in the ftrace code. Once that is merged you could use that. How do you hit the original problem, you don't actually specify in your commit message? Something with kprobes obviously, but what exactly? I'll try and reproduce it here. cheers ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-06-19 1:30 ` Michael Ellerman @ 2014-06-19 4:52 ` Masami Hiramatsu 2014-06-19 6:40 ` Suzuki K. Poulose 0 siblings, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2014-06-19 4:52 UTC (permalink / raw) To: Michael Ellerman Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Suzuki K. Poulose, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller (2014/06/19 10:30), Michael Ellerman wrote: > On Wed, 2014-06-18 at 17:46 +0900, Masami Hiramatsu wrote: >> (2014/06/18 16:56), Michael Ellerman wrote: >>> On Fri, 2014-06-06 at 15:38 +0900, Masami Hiramatsu wrote: >>>> Ping? >>>> >>>> I guess this should go to 3.16 branch, shouldn't it? >>> >>>>> 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 >>> >>> We already have ppc_function_entry(), can't you use that? >> >> I'd like to ask you whether the address which ppc_function_entry() returns on >> PPC ABIv2 is really same address in kallsyms or not. >> As you can see, kprobes uses function_entry() to get the actual entry address >> where kallsyms knows. I have not much information about that, but it seems that >> the "global entry point" is the address which kallsyms knows, isn't it? > > OK. I'm not sure off the top of my head which address kallsyms knows about, but > yes it's likely that it is the global entry point. > > I recently sent a patch to add ppc_global_function_entry(), because we need it > in the ftrace code. Once that is merged you could use that. Yeah, I could use that. But since this is used in arch-independent code (e.g. IA64 needs similar macro), I think we'd better define function_entry() in asm/types.h for general use (for kallsyms), and rename ppc_function_entry to local_function_entry() in asm/code-patching.h. > How do you hit the original problem, you don't actually specify in your commit > message? Something with kprobes obviously, but what exactly? I'll try and > reproduce it here. Ah, those messages should be shown in dmesg when booting if it doesn't work, because the messages are printed by initialization process of kprobe blacklist. So, reproducing it is just enabling CONFIG_KPROBES and boot it. Thank you, -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-06-19 4:52 ` Masami Hiramatsu @ 2014-06-19 6:40 ` Suzuki K. Poulose 2014-06-19 7:26 ` Masami Hiramatsu 0 siblings, 1 reply; 26+ messages in thread From: Suzuki K. Poulose @ 2014-06-19 6:40 UTC (permalink / raw) To: Masami Hiramatsu, Michael Ellerman Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller On 06/19/2014 10:22 AM, Masami Hiramatsu wrote: > (2014/06/19 10:30), Michael Ellerman wrote: >> On Wed, 2014-06-18 at 17:46 +0900, Masami Hiramatsu wrote: >>> (2014/06/18 16:56), Michael Ellerman wrote: >>>> On Fri, 2014-06-06 at 15:38 +0900, Masami Hiramatsu wrote: >>>>> Ping? >>>>> >>>>> I guess this should go to 3.16 branch, shouldn't it? >>>> >>>>>> 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 >>>> >>>> We already have ppc_function_entry(), can't you use that? >>> >>> I'd like to ask you whether the address which ppc_function_entry() returns on >>> PPC ABIv2 is really same address in kallsyms or not. >>> As you can see, kprobes uses function_entry() to get the actual entry address >>> where kallsyms knows. I have not much information about that, but it seems that >>> the "global entry point" is the address which kallsyms knows, isn't it? >> >> OK. I'm not sure off the top of my head which address kallsyms knows about, but >> yes it's likely that it is the global entry point. >> >> I recently sent a patch to add ppc_global_function_entry(), because we need it >> in the ftrace code. Once that is merged you could use that. > > Yeah, I could use that. But since this is used in arch-independent code (e.g. IA64 > needs similar macro), I think we'd better define function_entry() in asm/types.h for > general use (for kallsyms), and rename ppc_function_entry to local_function_entry() > in asm/code-patching.h. > > >> How do you hit the original problem, you don't actually specify in your commit >> message? Something with kprobes obviously, but what exactly? I'll try and >> reproduce it here. > > Ah, those messages should be shown in dmesg when booting if it doesn't work, > because the messages are printed by initialization process of kprobe blacklist. > So, reproducing it is just enabling CONFIG_KPROBES and boot it. Well, we don't get those messages on Power, since the kallsyms has the entries for ".function_name". The correct way to verify is, either : 1) Dump the black_list via xmon ( see : https://lkml.org/lkml/2014/5/29/893 ) and verify the entries. or 2) Issue a kprobe on a black listed entry and hit a success,(which we will, since we don't check the actual function address). Thanks Suzuki > > Thank you, > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-06-19 6:40 ` Suzuki K. Poulose @ 2014-06-19 7:26 ` Masami Hiramatsu 2014-06-19 9:45 ` Suzuki K. Poulose 0 siblings, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2014-06-19 7:26 UTC (permalink / raw) To: Suzuki K. Poulose Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller (2014/06/19 15:40), Suzuki K. Poulose wrote: > On 06/19/2014 10:22 AM, Masami Hiramatsu wrote: >> (2014/06/19 10:30), Michael Ellerman wrote: >>> On Wed, 2014-06-18 at 17:46 +0900, Masami Hiramatsu wrote: >>>> (2014/06/18 16:56), Michael Ellerman wrote: >>>>> On Fri, 2014-06-06 at 15:38 +0900, Masami Hiramatsu wrote: >>>>>> Ping? >>>>>> >>>>>> I guess this should go to 3.16 branch, shouldn't it? >>>>> >>>>>>> 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 >>>>> >>>>> We already have ppc_function_entry(), can't you use that? >>>> >>>> I'd like to ask you whether the address which ppc_function_entry() returns on >>>> PPC ABIv2 is really same address in kallsyms or not. >>>> As you can see, kprobes uses function_entry() to get the actual entry address >>>> where kallsyms knows. I have not much information about that, but it seems that >>>> the "global entry point" is the address which kallsyms knows, isn't it? >>> >>> OK. I'm not sure off the top of my head which address kallsyms knows about, but >>> yes it's likely that it is the global entry point. >>> >>> I recently sent a patch to add ppc_global_function_entry(), because we need it >>> in the ftrace code. Once that is merged you could use that. >> >> Yeah, I could use that. But since this is used in arch-independent code (e.g. IA64 >> needs similar macro), I think we'd better define function_entry() in asm/types.h for >> general use (for kallsyms), and rename ppc_function_entry to local_function_entry() >> in asm/code-patching.h. >> >> >>> How do you hit the original problem, you don't actually specify in your commit >>> message? Something with kprobes obviously, but what exactly? I'll try and >>> reproduce it here. >> >> Ah, those messages should be shown in dmesg when booting if it doesn't work, >> because the messages are printed by initialization process of kprobe blacklist. >> So, reproducing it is just enabling CONFIG_KPROBES and boot it. > Well, we don't get those messages on Power, since the kallsyms has the > entries for ".function_name". The correct way to verify is, either : Hmm, that seems another issue on powerpc. Is that expected(and designed) behavior? And if so, how I can verify when initializing blacklist? (should I better use kallsyms_lookup() and kallsyms_lookup_name() for verification?) Thank you, > > 1) Dump the black_list via xmon ( see : > https://lkml.org/lkml/2014/5/29/893 ) and verify the entries. > > or > > 2) Issue a kprobe on a black listed entry and hit a success,(which we > will, since we don't check the actual function address). > > Thanks > Suzuki > > >> >> Thank you, >> -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-06-19 7:26 ` Masami Hiramatsu @ 2014-06-19 9:45 ` Suzuki K. Poulose 2014-06-19 11:01 ` Masami Hiramatsu 0 siblings, 1 reply; 26+ messages in thread From: Suzuki K. Poulose @ 2014-06-19 9:45 UTC (permalink / raw) To: Masami Hiramatsu Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller On 06/19/2014 12:56 PM, Masami Hiramatsu wrote: > (2014/06/19 15:40), Suzuki K. Poulose wrote: >> On 06/19/2014 10:22 AM, Masami Hiramatsu wrote: >>> (2014/06/19 10:30), Michael Ellerman wrote: >>>> On Wed, 2014-06-18 at 17:46 +0900, Masami Hiramatsu wrote: >>>>> (2014/06/18 16:56), Michael Ellerman wrote: >>>>>> On Fri, 2014-06-06 at 15:38 +0900, Masami Hiramatsu wrote: >>>>>>> Ping? >>>>>>> >>>>>>> I guess this should go to 3.16 branch, shouldn't it? >>>>>> >>>>>>>> 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 >>>>>> >>>>>> We already have ppc_function_entry(), can't you use that? >>>>> >>>>> I'd like to ask you whether the address which ppc_function_entry() returns on >>>>> PPC ABIv2 is really same address in kallsyms or not. >>>>> As you can see, kprobes uses function_entry() to get the actual entry address >>>>> where kallsyms knows. I have not much information about that, but it seems that >>>>> the "global entry point" is the address which kallsyms knows, isn't it? >>>> >>>> OK. I'm not sure off the top of my head which address kallsyms knows about, but >>>> yes it's likely that it is the global entry point. >>>> >>>> I recently sent a patch to add ppc_global_function_entry(), because we need it >>>> in the ftrace code. Once that is merged you could use that. >>> >>> Yeah, I could use that. But since this is used in arch-independent code (e.g. IA64 >>> needs similar macro), I think we'd better define function_entry() in asm/types.h for >>> general use (for kallsyms), and rename ppc_function_entry to local_function_entry() >>> in asm/code-patching.h. >>> >>> >>>> How do you hit the original problem, you don't actually specify in your commit >>>> message? Something with kprobes obviously, but what exactly? I'll try and >>>> reproduce it here. >>> >>> Ah, those messages should be shown in dmesg when booting if it doesn't work, >>> because the messages are printed by initialization process of kprobe blacklist. >>> So, reproducing it is just enabling CONFIG_KPROBES and boot it. >> Well, we don't get those messages on Power, since the kallsyms has the >> entries for ".function_name". The correct way to verify is, either : > > Hmm, that seems another issue on powerpc. Is that expected(and designed) > behavior? AFAIK, yes, it is. To be more precise : we have 'foo' and '.foo' for a function foo(), where 'foo' points to the function_entry and '.foo' points to the actual function. So, a kallsyms_lookup_size_offset() on both 'foo' and '.foo' will return a hit. So, if we make sure we use the value of '.foo' (by using the appropriate macros) we should be fine. And if so, how I can verify when initializing blacklist? > (should I better use kallsyms_lookup() and kallsyms_lookup_name() for > verification?) One way to verify would be to make sure the symbol starts with '.' from the result of the current kallsyms_lookup_size_offset() for PPC. Thanks Suzuki > > Thank you, > >> >> 1) Dump the black_list via xmon ( see : >> https://lkml.org/lkml/2014/5/29/893 ) and verify the entries. >> >> or >> >> 2) Issue a kprobe on a black listed entry and hit a success,(which we >> will, since we don't check the actual function address). >> >> Thanks >> Suzuki >> >> >>> >>> Thank you, >>> ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-06-19 9:45 ` Suzuki K. Poulose @ 2014-06-19 11:01 ` Masami Hiramatsu 2014-06-19 11:20 ` Masami Hiramatsu 0 siblings, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2014-06-19 11:01 UTC (permalink / raw) To: Suzuki K. Poulose Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller (2014/06/19 18:45), Suzuki K. Poulose wrote: > On 06/19/2014 12:56 PM, Masami Hiramatsu wrote: >> (2014/06/19 15:40), Suzuki K. Poulose wrote: >>> On 06/19/2014 10:22 AM, Masami Hiramatsu wrote: >>>> (2014/06/19 10:30), Michael Ellerman wrote: >>>>> On Wed, 2014-06-18 at 17:46 +0900, Masami Hiramatsu wrote: >>>>>> (2014/06/18 16:56), Michael Ellerman wrote: >>>>>>> On Fri, 2014-06-06 at 15:38 +0900, Masami Hiramatsu wrote: >>>>>>>> Ping? >>>>>>>> >>>>>>>> I guess this should go to 3.16 branch, shouldn't it? >>>>>>> >>>>>>>>> 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 >>>>>>> >>>>>>> We already have ppc_function_entry(), can't you use that? >>>>>> >>>>>> I'd like to ask you whether the address which ppc_function_entry() returns on >>>>>> PPC ABIv2 is really same address in kallsyms or not. >>>>>> As you can see, kprobes uses function_entry() to get the actual entry address >>>>>> where kallsyms knows. I have not much information about that, but it seems that >>>>>> the "global entry point" is the address which kallsyms knows, isn't it? >>>>> >>>>> OK. I'm not sure off the top of my head which address kallsyms knows about, but >>>>> yes it's likely that it is the global entry point. >>>>> >>>>> I recently sent a patch to add ppc_global_function_entry(), because we need it >>>>> in the ftrace code. Once that is merged you could use that. >>>> >>>> Yeah, I could use that. But since this is used in arch-independent code (e.g. IA64 >>>> needs similar macro), I think we'd better define function_entry() in asm/types.h for >>>> general use (for kallsyms), and rename ppc_function_entry to local_function_entry() >>>> in asm/code-patching.h. >>>> >>>> >>>>> How do you hit the original problem, you don't actually specify in your commit >>>>> message? Something with kprobes obviously, but what exactly? I'll try and >>>>> reproduce it here. >>>> >>>> Ah, those messages should be shown in dmesg when booting if it doesn't work, >>>> because the messages are printed by initialization process of kprobe blacklist. >>>> So, reproducing it is just enabling CONFIG_KPROBES and boot it. >>> Well, we don't get those messages on Power, since the kallsyms has the >>> entries for ".function_name". The correct way to verify is, either : >> >> Hmm, that seems another issue on powerpc. Is that expected(and designed) >> behavior? > AFAIK, yes, it is. > To be more precise : > > we have 'foo' and '.foo' for a function foo(), where 'foo' points to the > function_entry and '.foo' points to the actual function. Ah, I see. So if we run func_ptr p = foo; return p == kallsyms_lookup_name(".foo"); it returns true. > So, a kallsyms_lookup_size_offset() on both 'foo' and '.foo' will return > a hit. So, if we make sure we use the value of '.foo' (by using the > appropriate macros) we should be fine. > > And if so, how I can verify when initializing blacklist? >> (should I better use kallsyms_lookup() and kallsyms_lookup_name() for >> verification?) > One way to verify would be to make sure the symbol starts with '.' from > the result of the current kallsyms_lookup_size_offset() for PPC. OK, I'll do that as another enhancement, since the bug reported here will be fixed with our patch. Anyway, this patch itself should go into 3.16 tree to fix actual bug. Thanks, -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Re: Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-06-19 11:01 ` Masami Hiramatsu @ 2014-06-19 11:20 ` Masami Hiramatsu 2014-06-20 0:37 ` Michael Ellerman 0 siblings, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2014-06-19 11:20 UTC (permalink / raw) To: Suzuki K. Poulose Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller (2014/06/19 20:01), Masami Hiramatsu wrote: >>>>> Ah, those messages should be shown in dmesg when booting if it doesn't work, >>>>> because the messages are printed by initialization process of kprobe blacklist. >>>>> So, reproducing it is just enabling CONFIG_KPROBES and boot it. >>>> Well, we don't get those messages on Power, since the kallsyms has the >>>> entries for ".function_name". The correct way to verify is, either : >>> >>> Hmm, that seems another issue on powerpc. Is that expected(and designed) >>> behavior? >> AFAIK, yes, it is. >> To be more precise : >> >> we have 'foo' and '.foo' for a function foo(), where 'foo' points to the >> function_entry and '.foo' points to the actual function. > > Ah, I see. So if we run > > func_ptr p = foo; > return p == kallsyms_lookup_name(".foo"); > > it returns true. One more thing I should know, is the address of ".function_name" within the kernel text? In other words, does kernel_text_address() return true for that address? If not, it's easy to verify the address. Thank you, -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Re: Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-06-19 11:20 ` Masami Hiramatsu @ 2014-06-20 0:37 ` Michael Ellerman 2014-06-20 2:13 ` Masami Hiramatsu 0 siblings, 1 reply; 26+ messages in thread From: Michael Ellerman @ 2014-06-20 0:37 UTC (permalink / raw) To: Masami Hiramatsu Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Suzuki K. Poulose, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller On Thu, 2014-06-19 at 20:20 +0900, Masami Hiramatsu wrote: > (2014/06/19 20:01), Masami Hiramatsu wrote: > > >>>>> Ah, those messages should be shown in dmesg when booting if it doesn't work, > >>>>> because the messages are printed by initialization process of kprobe blacklist. > >>>>> So, reproducing it is just enabling CONFIG_KPROBES and boot it. > >>>> Well, we don't get those messages on Power, since the kallsyms has the > >>>> entries for ".function_name". The correct way to verify is, either : > >>> > >>> Hmm, that seems another issue on powerpc. Is that expected(and designed) > >>> behavior? > >> AFAIK, yes, it is. > >> To be more precise : > >> > >> we have 'foo' and '.foo' for a function foo(), where 'foo' points to the > >> function_entry and '.foo' points to the actual function. > > > > Ah, I see. So if we run > > > > func_ptr p = foo; > > return p == kallsyms_lookup_name(".foo"); > > > > it returns true. > > One more thing I should know, is the address of ".function_name" within the > kernel text? In other words, does kernel_text_address() return true for that > address? If not, it's easy to verify the address. Yes. That is the text address, kernel_text_address() should definitely return true. On 64-bit, ABIv1, "foo" points to the function descriptor, in the ".opd" section. ".foo" points to the actual text of the function, in ".text". On 64-bit, ABIv2, "foo" points to the text in ".text". There are no dot symbols. cheers ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFT PATCH -next v3] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 2014-06-20 0:37 ` Michael Ellerman @ 2014-06-20 2:13 ` Masami Hiramatsu 0 siblings, 0 replies; 26+ messages in thread From: Masami Hiramatsu @ 2014-06-20 2:13 UTC (permalink / raw) To: Michael Ellerman Cc: Jeremy Fitzhardinge, linux-ia64, sparse, Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar, Suzuki K. Poulose, Fenghua Yu, Arnd Bergmann, Rusty Russell, Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao, linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton, Linus Torvalds, David S. Miller (2014/06/20 9:37), Michael Ellerman wrote: > On Thu, 2014-06-19 at 20:20 +0900, Masami Hiramatsu wrote: >> (2014/06/19 20:01), Masami Hiramatsu wrote: >> >>>>>>> Ah, those messages should be shown in dmesg when booting if it doesn't work, >>>>>>> because the messages are printed by initialization process of kprobe blacklist. >>>>>>> So, reproducing it is just enabling CONFIG_KPROBES and boot it. >>>>>> Well, we don't get those messages on Power, since the kallsyms has the >>>>>> entries for ".function_name". The correct way to verify is, either : >>>>> >>>>> Hmm, that seems another issue on powerpc. Is that expected(and designed) >>>>> behavior? >>>> AFAIK, yes, it is. >>>> To be more precise : >>>> >>>> we have 'foo' and '.foo' for a function foo(), where 'foo' points to the >>>> function_entry and '.foo' points to the actual function. >>> >>> Ah, I see. So if we run >>> >>> func_ptr p = foo; >>> return p == kallsyms_lookup_name(".foo"); >>> >>> it returns true. >> >> One more thing I should know, is the address of ".function_name" within the >> kernel text? In other words, does kernel_text_address() return true for that >> address? If not, it's easy to verify the address. > > Yes. That is the text address, kernel_text_address() should definitely return > true. > > On 64-bit, ABIv1, "foo" points to the function descriptor, in the ".opd" > section. > > ".foo" points to the actual text of the function, in ".text". Hmm, I misunderstood that. Anyway, we can verify it by kernel_text_address(). > > On 64-bit, ABIv2, "foo" points to the text in ".text". There are no dot > symbols. OK, in that case, kernel_text_address() check is still available. :) Thank you, -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2014-06-20 2:13 UTC | newest] Thread overview: 26+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <536A16A6.5040709@hitachi.com> 2014-05-07 11:55 ` [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64 Masami Hiramatsu 2014-05-07 11:59 ` Masami Hiramatsu 2014-05-14 8:19 ` Masami Hiramatsu 2014-05-08 4:47 ` Ananth N Mavinakayanahalli 2014-05-08 5:40 ` Masami Hiramatsu 2014-05-08 6:16 ` Ananth N Mavinakayanahalli 2014-05-09 8:06 ` Masami Hiramatsu 2014-05-26 11:25 ` Suzuki K. Poulose 2014-05-26 11:48 ` Masami Hiramatsu 2014-05-27 6:31 ` [RFT PATCH -next v2] " Masami Hiramatsu 2014-05-29 19:13 ` Suzuki K. Poulose 2014-05-30 2:47 ` Masami Hiramatsu 2014-05-30 3:18 ` [RFT PATCH -next v3] " Masami Hiramatsu 2014-06-06 6:38 ` Masami Hiramatsu 2014-06-17 23:03 ` Tony Luck 2014-06-18 7:56 ` Michael Ellerman 2014-06-18 8:46 ` Masami Hiramatsu 2014-06-19 1:30 ` Michael Ellerman 2014-06-19 4:52 ` Masami Hiramatsu 2014-06-19 6:40 ` Suzuki K. Poulose 2014-06-19 7:26 ` Masami Hiramatsu 2014-06-19 9:45 ` Suzuki K. Poulose 2014-06-19 11:01 ` Masami Hiramatsu 2014-06-19 11:20 ` Masami Hiramatsu 2014-06-20 0:37 ` Michael Ellerman 2014-06-20 2:13 ` Masami Hiramatsu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).