| Message ID | 46b6f6aaec8dd297c2d8e811c82dc2303ff13508.1498732172.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, 29 Jun 2017 16:11:04 +0530 "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote: > Currently, we assume that the function pointer we receive in > ppc_function_entry() points to a function descriptor. However, this is > not always the case. In particular, assembly symbols without the right > annotation do not have an associated function descriptor. Some of these > symbols are added to the kprobe blacklist using _ASM_NOKPROBE_SYMBOL(). > > When such addresses are subsequently processed through > arch_deref_entry_point() in populate_kprobe_blacklist(), we see the > below errors during bootup: > [ 0.663963] Failed to find blacklist at 7d9b02a648029b6c > [ 0.663970] Failed to find blacklist at a14d03d0394a0001 > [ 0.663972] Failed to find blacklist at 7d5302a6f94d0388 > [ 0.663973] Failed to find blacklist at 48027d11e8610178 > [ 0.663974] Failed to find blacklist at f8010070f8410080 > [ 0.663976] Failed to find blacklist at 386100704801f89d > [ 0.663977] Failed to find blacklist at 7d5302a6f94d00b0 > > Fix this by checking if the function pointer we receive in > ppc_function_entry() already points to kernel text. If so, we just > return it as is. If not, we assume that this is a function descriptor > and proceed to dereference it. > > Suggested-by: Nicholas Piggin <npiggin@gmail.com> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > --- > arch/powerpc/include/asm/code-patching.h | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h > index abef812de7f8..5482928eea1b 100644 > --- a/arch/powerpc/include/asm/code-patching.h > +++ b/arch/powerpc/include/asm/code-patching.h > @@ -83,8 +83,16 @@ static inline unsigned long ppc_function_entry(void *func) > * 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. > + * > + * However, we may also receive pointer to an assembly symbol. To > + * detect that, we first check if the function pointer we receive > + * already points to kernel/module text and we only dereference it > + * if it doesn't. > */ > - return ((func_descr_t *)func)->entry; > + if (kernel_text_address((unsigned long)func)) > + return (unsigned long)func; > + else > + return ((func_descr_t *)func)->entry; This seems good to me now. I guess it should do the right thing with modules too, looking at kernel_text_address implementation. Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
On 2017/06/29 08:49PM, Nicholas Piggin wrote: > On Thu, 29 Jun 2017 16:11:04 +0530 > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote: > > > Currently, we assume that the function pointer we receive in > > ppc_function_entry() points to a function descriptor. However, this is > > not always the case. In particular, assembly symbols without the right > > annotation do not have an associated function descriptor. Some of these > > symbols are added to the kprobe blacklist using _ASM_NOKPROBE_SYMBOL(). > > > > When such addresses are subsequently processed through > > arch_deref_entry_point() in populate_kprobe_blacklist(), we see the > > below errors during bootup: > > [ 0.663963] Failed to find blacklist at 7d9b02a648029b6c > > [ 0.663970] Failed to find blacklist at a14d03d0394a0001 > > [ 0.663972] Failed to find blacklist at 7d5302a6f94d0388 > > [ 0.663973] Failed to find blacklist at 48027d11e8610178 > > [ 0.663974] Failed to find blacklist at f8010070f8410080 > > [ 0.663976] Failed to find blacklist at 386100704801f89d > > [ 0.663977] Failed to find blacklist at 7d5302a6f94d00b0 > > > > Fix this by checking if the function pointer we receive in > > ppc_function_entry() already points to kernel text. If so, we just > > return it as is. If not, we assume that this is a function descriptor > > and proceed to dereference it. > > > > Suggested-by: Nicholas Piggin <npiggin@gmail.com> > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > > --- > > arch/powerpc/include/asm/code-patching.h | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h > > index abef812de7f8..5482928eea1b 100644 > > --- a/arch/powerpc/include/asm/code-patching.h > > +++ b/arch/powerpc/include/asm/code-patching.h > > @@ -83,8 +83,16 @@ static inline unsigned long ppc_function_entry(void *func) > > * 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. > > + * > > + * However, we may also receive pointer to an assembly symbol. To > > + * detect that, we first check if the function pointer we receive > > + * already points to kernel/module text and we only dereference it > > + * if it doesn't. > > */ > > - return ((func_descr_t *)func)->entry; > > + if (kernel_text_address((unsigned long)func)) > > + return (unsigned long)func; > > + else > > + return ((func_descr_t *)func)->entry; > > This seems good to me now. I guess it should do the right thing with > modules too, looking at kernel_text_address implementation. Yes, I've tested that by exercizing the jprobe sample module. We use this to lookup the address of the jprobe handler which happens to be in the jprobe module and it is working properly: $ sudo modprobe jprobe_example $ dmesg | grep Planted [ 4817.649072] Planted jprobe at c0000000000ed7b0, handler addr d0000000074b03e8 $ sudo grep j_do_fork /proc/kallsyms d0000000074b03e8 d j_do_fork [jprobe_example] d0000000074b0000 t .j_do_fork [jprobe_example] > > Reviewed-by: Nicholas Piggin <npiggin@gmail.com> > Thanks for the review, - Naveen
diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h index abef812de7f8..5482928eea1b 100644 --- a/arch/powerpc/include/asm/code-patching.h +++ b/arch/powerpc/include/asm/code-patching.h @@ -83,8 +83,16 @@ static inline unsigned long ppc_function_entry(void *func) * 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. + * + * However, we may also receive pointer to an assembly symbol. To + * detect that, we first check if the function pointer we receive + * already points to kernel/module text and we only dereference it + * if it doesn't. */ - return ((func_descr_t *)func)->entry; + if (kernel_text_address((unsigned long)func)) + return (unsigned long)func; + else + return ((func_descr_t *)func)->entry; #else return (unsigned long)func; #endif
Currently, we assume that the function pointer we receive in ppc_function_entry() points to a function descriptor. However, this is not always the case. In particular, assembly symbols without the right annotation do not have an associated function descriptor. Some of these symbols are added to the kprobe blacklist using _ASM_NOKPROBE_SYMBOL(). When such addresses are subsequently processed through arch_deref_entry_point() in populate_kprobe_blacklist(), we see the below errors during bootup: [ 0.663963] Failed to find blacklist at 7d9b02a648029b6c [ 0.663970] Failed to find blacklist at a14d03d0394a0001 [ 0.663972] Failed to find blacklist at 7d5302a6f94d0388 [ 0.663973] Failed to find blacklist at 48027d11e8610178 [ 0.663974] Failed to find blacklist at f8010070f8410080 [ 0.663976] Failed to find blacklist at 386100704801f89d [ 0.663977] Failed to find blacklist at 7d5302a6f94d00b0 Fix this by checking if the function pointer we receive in ppc_function_entry() already points to kernel text. If so, we just return it as is. If not, we assume that this is a function descriptor and proceed to dereference it. Suggested-by: Nicholas Piggin <npiggin@gmail.com> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> --- arch/powerpc/include/asm/code-patching.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)