kprobes: Unify kretprobe trampoline handlers
Subsystems (13)
Masami Hiramatsu
maintainerv2 cover kprobes: Unify kretprobe trampoline handlers
Hi,
Here is the 2nd version of the series to unify the kretprobe trampoline handler implementation across all architectures which are currently kprobes supported. Previous version is here;
https://lkml.kernel.org/r/159844957216.510284.17683703701627367133.stgit@devnote2
This series removes the in_nmi() check from pre_kretprobe_handler() since we can avoid double-lock deadlock from NMI by kprobe_busy_begin/end(). In this version, I also add a patch to use kfree_rcu() for freeing kretprobe instance objects so that we don't call kfree() in NMI context directly.
The unified generic kretprobe trampoline handler is based on x86 code, which already support frame-pointer checker. The checker is enabled on arm and arm64 too because I can test it. For other architecutres, currently the checker is not enabled. If someone wants to enable it, please set the correct frame pointer to ri->fp and pass it to kretprobe_trampoline_handler() as the 3rd parameter, instead of NULL.
Thank you,
---
Masami Hiramatsu (15):
kprobes: Add generic kretprobe trampoline handler x86/kprobes: Use generic kretprobe trampoline handler
arm: kprobes: Use generic kretprobe trampoline handler
arm64: kprobes: Use generic kretprobe trampoline handler
arc: kprobes: Use generic kretprobe trampoline handler
csky: kprobes: Use generic kretprobe trampoline handler
ia64: kprobes: Use generic kretprobe trampoline handler
mips: kprobes: Use generic kretprobe trampoline handler
parisc: kprobes: Use generic kretprobe trampoline handler
powerpc: kprobes: Use generic kretprobe trampoline handler
s390: kprobes: Use generic kretprobe trampoline handler
sh: kprobes: Use generic kretprobe trampoline handler
sparc: kprobes: Use generic kretprobe trampoline handler
kprobes: Remove NMI context check
kprobes: Free kretprobe_instance with rcu callback
arch/arc/kernel/kprobes.c | 55 +--------------- arch/arm/probes/kprobes/core.c | 79 +---------------------- arch/arm64/kernel/probes/kprobes.c | 79 +---------------------- arch/csky/kernel/probes/kprobes.c | 78 +--------------------- arch/ia64/kernel/kprobes.c | 79 +---------------------- arch/mips/kernel/kprobes.c | 55 +--------------- arch/parisc/kernel/kprobes.c | 78 ++-------------------- arch/powerpc/kernel/kprobes.c | 55 +--------------- arch/s390/kernel/kprobes.c | 81 +---------------------- arch/sh/kernel/kprobes.c | 59 +---------------- arch/sparc/kernel/kprobes.c | 52 +-------------- arch/x86/kernel/kprobes/core.c | 109 +------------------------------ include/linux/kprobes.h | 35 +++++++++- kernel/kprobes.c | 126 +++++++++++++++++++++++++++++------- 14 files changed, 182 insertions(+), 838 deletions(-)
--
Masami Hiramatsu (Linaro) <[email protected]>
Masami Hiramatsu
maintainerPATCH v2 1/15 kprobes: Add generic kretprobe trampoline handler
Add a generic kretprobe trampoline handler for unifying the all cloned /arch/* kretprobe trampoline handlers.
The generic kretprobe trampoline handler is based on the x86 implementation, because it is the latest implementation. It has frame pointer checking, kprobe_busy_begin/end and return address fixup for user handlers.
--- include/linux/kprobes.h | 32 +++++++++++++-- kernel/kprobes.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 4 deletions(-)
include/linux/kprobes.h +28 -4
@@ -187,10 +187,38 @@ static inline int kprobes_built_in(void)return 1 ; } +extern struct kprobe kprobe_busy; +void kprobe_busy_begin (void ); +void kprobe_busy_end (void ); +#ifdef CONFIG_KRETPROBES extern void arch_prepare_kretprobe (struct kretprobe_instance *ri,struct pt_regs *regs);extern int arch_trampoline_kprobe (struct kprobe *p); + +/* If the trampoline handler called from a kprobe, use this version */ +unsigned long __kretprobe_trampoline_handler (struct pt_regs *regs, +unsigned long trampoline_address, +void *frame_pointer); + +static nokprobe_inline +unsigned long kretprobe_trampoline_handler (struct pt_regs *regs, +unsigned long trampoline_address, +void *frame_pointer) +{ +unsigned long ret; +/* +* Set a dummy kprobe for avoiding kretprobe recursion. +* Since kretprobe never runs in kprobe handler, any kprobe must not +* be running at this point. +*/ +kprobe_busy_begin (); + ret =__kretprobe_trampoline_handler (regs, trampoline_address, frame_pointer); +kprobe_busy_end (); + +return ret; +} +#else /* CONFIG_KRETPROBES */ static inline void arch_prepare_kretprobe (struct kretprobe *rp,struct pt_regs *regs)
@@ -354,10 +382,6 @@ static inline struct kprobe_ctlblk *get_kprobe_ctlblk(void)return this_cpu_ptr (&kprobe_ctlblk); } -extern struct kprobe kprobe_busy; -void kprobe_busy_begin (void ); -void kprobe_busy_end (void ); -kprobe_opcode_t *kprobe_lookup_name (const char *name,unsigned int offset);int register_kprobe (struct kprobe *p);void unregister_kprobe (struct kprobe *p);
kernel/kprobes.c +101 -0
107 lines not on this page · raw message
Masami Hiramatsu
maintainerPATCH v2 2/15 x86/kprobes: Use generic kretprobe trampoline handler
In reply to Masami Hiramatsu--- arch/x86/kernel/kprobes/core.c | 109 +--------------------------------------- 1 file changed, 4 insertions(+), 105 deletions(-)
arch/x86/kernel/kprobes/core.c +4 -105
@@ -767,124 +767,23 @@ asm(NOKPROBE_SYMBOL (kretprobe_trampoline);STACK_FRAME_NON_STANDARD (kretprobe_trampoline); +/* * Called from kretprobe_trampoline */ __used __visible void *trampoline_handler(struct pt_regs *regs) { -struct kretprobe_instance *ri =NULL ; -struct hlist_head *head, empty_rp; -struct hlist_node *tmp; -unsigned long flags, orig_ret_address =0 ; -unsigned long trampoline_address = (unsigned long )&kretprobe_trampoline; -kprobe_opcode_t *correct_ret_addr =NULL ; -void *frame_pointer; -bool skipped = false; - -/* -* Set a dummy kprobe for avoiding kretprobe recursion. -* Since kretprobe never run in kprobe handler, kprobe must not -* be running at this point. -*/ -kprobe_busy_begin (); - -INIT_HLIST_HEAD (&empty_rp); -kretprobe_hash_lock (current, &head, &flags);/* fixup registers */ regs->cs = __KERNEL_CS;#ifdef CONFIG_X86_32 regs->cs |=get_kernel_rpl (); regs->gs =0 ;#endif -/* We use pt_regs->sp for return address holder. */ - frame_pointer = ®s->sp; - regs->ip = trampoline_address; + regs->ip = (unsigned long )&kretprobe_trampoline; regs->orig_ax = ~0UL ; -/* -* It is possible to have multiple instances associated with a given -* task either because multiple functions in the call path have -* return probes installed on them, and/or more than one -* return probe was registered for a target function. -* -* We can handle this because: -* - instances are always pushed into the head of the list -* - when multiple return probes are registered for the same -* function, the (chronologically) first instance's ret_addr -* will be the real return address, and all the rest will -* point to kretprobe_trampoline. -*/ -hlist_for_each_entry (ri, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; -/* -* Return probes must be pushed on this hash list correct -* order (same as return order) so that it can be popped -* correctly. However, if we find it is pushed it incorrect -* order, this means we find a function which should not be -* probed, because the wrong order entry is pushed on the -* path of processing other kretprobe itself. -*/ -if (ri->fp != frame_pointer) { -if (!skipped) -pr_warn ("kretprobe is stacked incorrectly. Trying to fixup.\n" ); - skipped = true; -continue ; - } - - orig_ret_address = (unsigned long )ri->ret_addr; -if (skipped) -pr_warn ("%ps must be blacklisted because of incorrect kretprobe order\n" , - ri->rp->kp.addr); - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_assert (ri, orig_ret_address, trampoline_address); - - correct_ret_addr = ri->ret_addr; -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; -if (ri->fp != frame_pointer) -continue ; - - orig_ret_address = (unsigned long )ri->ret_addr; -if (ri->rp && ri->rp->handler) { -__this_cpu_write (current_kprobe, &ri->rp->kp); - ri->ret_addr = correct_ret_addr; - ri->rp->handler (ri, regs); -__this_cpu_write (current_kprobe, &kprobe_busy); - } - -recycle_rp_inst (ri, &empty_rp); - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_hash_unlock (current, &flags); - -kprobe_busy_end (); - -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - } -return (void *)orig_ret_address; +return (void *)kretprobe_trampoline_handler (regs, + (unsigned long )&kretprobe_trampoline, ®s->sp); }NOKPROBE_SYMBOL (trampoline_handler);
Masami Hiramatsu
maintainerPATCH v2 3/15 arm: kprobes: Use generic kretprobe trampoline handler
In reply to Masami HiramatsuUse the generic kretprobe trampoline handler. Use regs->ARM_fp for framepointer verification.
---
Changes in v2:
- Fix to cast frame_pointer type to void *.
--- arch/arm/probes/kprobes/core.c | 79 ++-------------------------------------- 1 file changed, 4 insertions(+), 75 deletions(-)
arch/arm/probes/kprobes/core.c +4 -75
@@ -413,87 +413,16 @@ void __naked __kprobes kretprobe_trampoline(void)/* Called from kretprobe_trampoline */ static __used __kprobes void *trampoline_handler (struct pt_regs *regs) { -struct kretprobe_instance *ri =NULL ; -struct hlist_head *head, empty_rp; -struct hlist_node *tmp; -unsigned long flags, orig_ret_address =0 ; -unsigned long trampoline_address = (unsigned long )&kretprobe_trampoline; -kprobe_opcode_t *correct_ret_addr =NULL ; - -INIT_HLIST_HEAD (&empty_rp); -kretprobe_hash_lock (current, &head, &flags); - -/* -* It is possible to have multiple instances associated with a given -* task either because multiple functions in the call path have -* a return probe installed on them, and/or more than one return -* probe was registered for a target function. -* -* We can handle this because: -* - instances are always inserted at the head of the list -* - when multiple return probes are registered for the same -* function, the first instance's ret_addr will point to the -* real return address, and all the rest will point to -* kretprobe_trampoline -*/ -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - - orig_ret_address = (unsigned long )ri->ret_addr; - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_assert (ri, orig_ret_address, trampoline_address); - - correct_ret_addr = ri->ret_addr; -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - - orig_ret_address = (unsigned long )ri->ret_addr; -if (ri->rp && ri->rp->handler) { -__this_cpu_write (current_kprobe, &ri->rp->kp); -get_kprobe_ctlblk ()->kprobe_status =KPROBE_HIT_ACTIVE ; - ri->ret_addr = correct_ret_addr; - ri->rp->handler (ri, regs); -__this_cpu_write (current_kprobe,NULL ); - } - -recycle_rp_inst (ri, &empty_rp); - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_hash_unlock (current, &flags); - -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - } - -return (void *)orig_ret_address; +return (void *)kretprobe_trampoline_handler (regs, + (unsigned long )&kretprobe_trampoline, + (void *)regs->ARM_fp); }void __kprobes arch_prepare_kretprobe (struct kretprobe_instance *ri,struct pt_regs *regs) { ri->ret_addr = (kprobe_opcode_t *)regs->ARM_lr; + ri->fp = (void *)regs->ARM_fp;/* Replace the return addr with trampoline addr. */ regs->ARM_lr = (unsigned long )&kretprobe_trampoline;
Masami Hiramatsu
maintainerPATCH v2 4/15 arm64: kprobes: Use generic kretprobe trampoline handler
In reply to Masami HiramatsuUse the generic kretprobe trampoline handler, and use the kernel_stack_pointer(regs) for framepointer verification.
--- arch/arm64/kernel/probes/kprobes.c | 79 ++---------------------------------- 1 file changed, 4 insertions(+), 75 deletions(-)
arch/arm64/kernel/probes/kprobes.c +4 -75
@@ -464,87 +464,16 @@ int __init arch_populate_kprobe_blacklist(void)void __kprobes __used *trampoline_probe_handler (struct pt_regs *regs) { -struct kretprobe_instance *ri =NULL ; -struct hlist_head *head, empty_rp; -struct hlist_node *tmp; -unsigned long flags, orig_ret_address =0 ; -unsigned long trampoline_address = - (unsigned long )&kretprobe_trampoline; -kprobe_opcode_t *correct_ret_addr =NULL ; - -INIT_HLIST_HEAD (&empty_rp); -kretprobe_hash_lock (current, &head, &flags); - -/* -* It is possible to have multiple instances associated with a given -* task either because multiple functions in the call path have -* return probes installed on them, and/or more than one -* return probe was registered for a target function. -* -* We can handle this because: -* - instances are always pushed into the head of the list -* - when multiple return probes are registered for the same -* function, the (chronologically) first instance's ret_addr -* will be the real return address, and all the rest will -* point to kretprobe_trampoline. -*/ -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - - orig_ret_address = (unsigned long )ri->ret_addr; - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_assert (ri, orig_ret_address, trampoline_address); - - correct_ret_addr = ri->ret_addr; -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - - orig_ret_address = (unsigned long )ri->ret_addr; -if (ri->rp && ri->rp->handler) { -__this_cpu_write (current_kprobe, &ri->rp->kp); -get_kprobe_ctlblk ()->kprobe_status =KPROBE_HIT_ACTIVE ; - ri->ret_addr = correct_ret_addr; - ri->rp->handler (ri, regs); -__this_cpu_write (current_kprobe,NULL ); - } - -recycle_rp_inst (ri, &empty_rp); - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_hash_unlock (current, &flags); - -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - } -return (void *)orig_ret_address; +return (void *)kretprobe_trampoline_handler (regs, + (unsigned long )&kretprobe_trampoline, + (void *)kernel_stack_pointer (regs)); }void __kprobes arch_prepare_kretprobe (struct kretprobe_instance *ri,struct pt_regs *regs) { ri->ret_addr = (kprobe_opcode_t *)regs->regs[30 ]; + ri->fp = (void *)kernel_stack_pointer (regs);/* replace return addr (x30) with trampoline */ regs->regs[30 ] = (long )&kretprobe_trampoline;
Masami Hiramatsu
maintainerPATCH v2 5/15 arc: kprobes: Use generic kretprobe trampoline handler
In reply to Masami Hiramatsu--- arch/arc/kernel/kprobes.c | 55 ++------------------------------------------- 1 file changed, 3 insertions(+), 52 deletions(-)
arch/arc/kernel/kprobes.c +3 -52
@@ -388,6 +388,7 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, { ri->ret_addr = (kprobe_opcode_t *) regs->blink; + ri->fp =NULL ;/* Replace the return addr with trampoline addr */ regs->blink = (unsigned long )&kretprobe_trampoline;
@@ -396,58 +397,8 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,static int __kprobes trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { -struct kretprobe_instance *ri =NULL ; -struct hlist_head *head, empty_rp; -struct hlist_node *tmp; -unsigned long flags, orig_ret_address =0 ; -unsigned long trampoline_address = (unsigned long )&kretprobe_trampoline; - -INIT_HLIST_HEAD (&empty_rp); -kretprobe_hash_lock (current, &head, &flags); - -/* -* It is possible to have multiple instances associated with a given -* task either because an multiple functions in the call path -* have a return probe installed on them, and/or more than one return -* return probe was registered for a target function. -* -* We can handle this because: -* - instances are always inserted at the head of the list -* - when multiple return probes are registered for the same -* function, the first instance's ret_addr will point to the -* real return address, and all the rest will point to -* kretprobe_trampoline -*/ -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - -if (ri->rp && ri->rp->handler) - ri->rp->handler (ri, regs); - - orig_ret_address = (unsigned long )ri->ret_addr; -recycle_rp_inst (ri, &empty_rp); - -if (orig_ret_address != trampoline_address) { -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - } - -kretprobe_assert (ri, orig_ret_address, trampoline_address); - regs->ret = orig_ret_address; - -kretprobe_hash_unlock (current, &flags); - -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - } + regs->ret =__kretprobe_trampoline_handler (regs, + (unsigned long )&kretprobe_trampoline,NULL );/* By returning a non zero value, we are telling the kprobe handler * that we don't want the post_handler to run
Masami Hiramatsu
maintainerPATCH v2 6/15 csky: kprobes: Use generic kretprobe trampoline handler
In reply to Masami Hiramatsu--- arch/csky/kernel/probes/kprobes.c | 78 +------------------------------------ 1 file changed, 3 insertions(+), 75 deletions(-)
arch/csky/kernel/probes/kprobes.c +3 -75
@@ -404,87 +404,15 @@ int __init arch_populate_kprobe_blacklist(void)void __kprobes __used *trampoline_probe_handler (struct pt_regs *regs) { -struct kretprobe_instance *ri =NULL ; -struct hlist_head *head, empty_rp; -struct hlist_node *tmp; -unsigned long flags, orig_ret_address =0 ; -unsigned long trampoline_address = - (unsigned long )&kretprobe_trampoline; -kprobe_opcode_t *correct_ret_addr =NULL ; - -INIT_HLIST_HEAD (&empty_rp); -kretprobe_hash_lock (current, &head, &flags); - -/* -* It is possible to have multiple instances associated with a given -* task either because multiple functions in the call path have -* return probes installed on them, and/or more than one -* return probe was registered for a target function. -* -* We can handle this because: -* - instances are always pushed into the head of the list -* - when multiple return probes are registered for the same -* function, the (chronologically) first instance's ret_addr -* will be the real return address, and all the rest will -* point to kretprobe_trampoline. -*/ -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - - orig_ret_address = (unsigned long )ri->ret_addr; - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_assert (ri, orig_ret_address, trampoline_address); - - correct_ret_addr = ri->ret_addr; -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - - orig_ret_address = (unsigned long )ri->ret_addr; -if (ri->rp && ri->rp->handler) { -__this_cpu_write (current_kprobe, &ri->rp->kp); -get_kprobe_ctlblk ()->kprobe_status =KPROBE_HIT_ACTIVE ; - ri->ret_addr = correct_ret_addr; - ri->rp->handler (ri, regs); -__this_cpu_write (current_kprobe,NULL ); - } - -recycle_rp_inst (ri, &empty_rp); - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_hash_unlock (current, &flags); - -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - } -return (void *)orig_ret_address; +return (void *)kretprobe_trampoline_handler (regs, + (unsigned long )&kretprobe_trampoline,NULL ); }void __kprobes arch_prepare_kretprobe (struct kretprobe_instance *ri,struct pt_regs *regs) { ri->ret_addr = (kprobe_opcode_t *)regs->lr; + ri->fp =NULL ; regs->lr = (unsigned long ) &kretprobe_trampoline; }
Masami Hiramatsu
maintainerPATCH v2 7/15 ia64: kprobes: Use generic kretprobe trampoline handler
In reply to Masami Hiramatsu--- arch/ia64/kernel/kprobes.c | 79 ++------------------------------------------ 1 file changed, 4 insertions(+), 75 deletions(-)
arch/ia64/kernel/kprobes.c +4 -75
@@ -396,83 +396,11 @@ static void kretprobe_trampoline(void) { } -/* -* At this point the target function has been tricked into -* returning into our trampoline. Lookup the associated instance -* and then: -* - call the handler function -* - cleanup by marking the instance as unused -* - long jump back to the original return address -*/ int __kprobes trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { -struct kretprobe_instance *ri =NULL ; -struct hlist_head *head, empty_rp; -struct hlist_node *tmp; -unsigned long flags, orig_ret_address =0 ; -unsigned long trampoline_address = - ((struct fnptr *)kretprobe_trampoline)->ip; - -INIT_HLIST_HEAD (&empty_rp); -kretprobe_hash_lock (current, &head, &flags); - -/* -* It is possible to have multiple instances associated with a given -* task either because an multiple functions in the call path -* have a return probe installed on them, and/or more than one return -* return probe was registered for a target function. -* -* We can handle this because: -* - instances are always inserted at the head of the list -* - when multiple return probes are registered for the same -* function, the first instance's ret_addr will point to the -* real return address, and all the rest will point to -* kretprobe_trampoline -*/ -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - - orig_ret_address = (unsigned long )ri->ret_addr; -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - - regs->cr_iip = orig_ret_address; - -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - -if (ri->rp && ri->rp->handler) - ri->rp->handler (ri, regs); - - orig_ret_address = (unsigned long )ri->ret_addr; -recycle_rp_inst (ri, &empty_rp); - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } -kretprobe_assert (ri, orig_ret_address, trampoline_address); - -kretprobe_hash_unlock (current, &flags); - -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - } + regs->cr_iip =__kretprobe_trampoline_handler (regs, + (unsigned long )kretprobe_trampoline, +NULL );/* * By returning a non-zero value, we are telling * kprobe_handler() that we don't want the post_handler
@@ -485,6 +413,7 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs) { ri->ret_addr = (kprobe_opcode_t *)regs->b0; + ri->fp =NULL ;/* Replace the return addr with trampoline addr */ regs->b0 = ((struct fnptr *)kretprobe_trampoline)->ip;
Masami Hiramatsu
maintainerPATCH v2 8/15 mips: kprobes: Use generic kretprobe trampoline handler
In reply to Masami Hiramatsu--- arch/mips/kernel/kprobes.c | 55 +++----------------------------------------- 1 file changed, 4 insertions(+), 51 deletions(-)
arch/mips/kernel/kprobes.c +4 -51
@@ -477,6 +477,7 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs) { ri->ret_addr = (kprobe_opcode_t *) regs->regs[31 ]; + ri->fp =NULL ;/* Replace the return addr with trampoline addr */ regs->regs[31 ] = (unsigned long )kretprobe_trampoline;
@@ -488,57 +489,9 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,static int __kprobes trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { -struct kretprobe_instance *ri =NULL ; -struct hlist_head *head, empty_rp; -struct hlist_node *tmp; -unsigned long flags, orig_ret_address =0 ; -unsigned long trampoline_address = (unsigned long )kretprobe_trampoline; - -INIT_HLIST_HEAD (&empty_rp); -kretprobe_hash_lock (current, &head, &flags); - -/* -* It is possible to have multiple instances associated with a given -* task either because an multiple functions in the call path -* have a return probe installed on them, and/or more than one return -* return probe was registered for a target function. -* -* We can handle this because: -* - instances are always inserted at the head of the list -* - when multiple return probes are registered for the same -* function, the first instance's ret_addr will point to the -* real return address, and all the rest will point to -* kretprobe_trampoline -*/ -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - -if (ri->rp && ri->rp->handler) - ri->rp->handler (ri, regs); - - orig_ret_address = (unsigned long )ri->ret_addr; -recycle_rp_inst (ri, &empty_rp); - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_assert (ri, orig_ret_address, trampoline_address); -instruction_pointer (regs) = orig_ret_address; - -kretprobe_hash_unlock (current, &flags); - -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - } +instruction_pointer (regs) =__kretprobe_trampoline_handler (regs, + (unsigned long )kretprobe_trampoline, +NULL );/* * By returning a non-zero value, we are telling * kprobe_handler() that we don't want the post_handler
Masami Hiramatsu
maintainerPATCH v2 9/15 parisc: kprobes: Use generic kretprobe trampoline handler
In reply to Masami Hiramatsu--- arch/parisc/kernel/kprobes.c | 78 +++--------------------------------------- 1 file changed, 6 insertions(+), 72 deletions(-)
arch/parisc/kernel/kprobes.c +6 -72
@@ -191,80 +191,13 @@ static struct kprobe trampoline_p = {static int __kprobes trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { -struct kretprobe_instance *ri =NULL ; -struct hlist_head *head, empty_rp; -struct hlist_node *tmp; -unsigned long flags, orig_ret_address =0 ; -unsigned long trampoline_address = (unsigned long )trampoline_p.addr; -kprobe_opcode_t *correct_ret_addr =NULL ; - -INIT_HLIST_HEAD (&empty_rp); -kretprobe_hash_lock (current, &head, &flags); - -/* -* It is possible to have multiple instances associated with a given -* task either because multiple functions in the call path have -* a return probe installed on them, and/or more than one return -* probe was registered for a target function. -* -* We can handle this because: -* - instances are always inserted at the head of the list -* - when multiple return probes are registered for the same -* function, the first instance's ret_addr will point to the -* real return address, and all the rest will point to -* kretprobe_trampoline -*/ -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - - orig_ret_address = (unsigned long )ri->ret_addr; - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_assert (ri, orig_ret_address, trampoline_address); - - correct_ret_addr = ri->ret_addr; -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - - orig_ret_address = (unsigned long )ri->ret_addr; -if (ri->rp && ri->rp->handler) { -__this_cpu_write (current_kprobe, &ri->rp->kp); -get_kprobe_ctlblk ()->kprobe_status =KPROBE_HIT_ACTIVE ; - ri->ret_addr = correct_ret_addr; - ri->rp->handler (ri, regs); -__this_cpu_write (current_kprobe,NULL ); - } - -recycle_rp_inst (ri, &empty_rp); - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } +unsigned long orig_ret_address; -kretprobe_hash_unlock (current, &flags); - -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - } + orig_ret_address =__kretprobe_trampoline_handler (regs, + (unsigned long )trampoline_p.addr, +NULL );instruction_pointer_set (regs, orig_ret_address); +return 1 ; }
@@ -272,6 +205,7 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs) { ri->ret_addr = (kprobe_opcode_t *)regs->gr[2 ]; + ri->fp =NULL ;/* Replace the return addr with trampoline addr. */ regs->gr[2 ] = (unsigned long )trampoline_p.addr;
Masami Hiramatsu
maintainerPATCH v2 10/15 powerpc: kprobes: Use generic kretprobe trampoline handler
In reply to Masami Hiramatsu---
Changes in v2:
Fix to use correct trampoline_address.
--- arch/powerpc/kernel/kprobes.c | 55 ++++------------------------------------- 1 file changed, 5 insertions(+), 50 deletions(-)
arch/powerpc/kernel/kprobes.c +5 -50
@@ -218,6 +218,7 @@ bool arch_kprobe_on_func_entry(unsigned long offset)void arch_prepare_kretprobe (struct kretprobe_instance *ri,struct pt_regs *regs) { ri->ret_addr = (kprobe_opcode_t *)regs->link; + ri->fp =NULL ;/* Replace the return addr with trampoline addr */ regs->link = (unsigned long )kretprobe_trampoline;
@@ -396,50 +397,11 @@ asm(".global kretprobe_trampoline\n"*/ static int trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { -struct kretprobe_instance *ri =NULL ; -struct hlist_head *head, empty_rp; -struct hlist_node *tmp; -unsigned long flags, orig_ret_address =0 ; -unsigned long trampoline_address =(unsigned long )&kretprobe_trampoline; - -INIT_HLIST_HEAD (&empty_rp); -kretprobe_hash_lock (current, &head, &flags); - -/* -* It is possible to have multiple instances associated with a given -* task either because an multiple functions in the call path -* have a return probe installed on them, and/or more than one return -* return probe was registered for a target function. -* -* We can handle this because: -* - instances are always inserted at the head of the list -* - when multiple return probes are registered for the same -* function, the first instance's ret_addr will point to the -* real return address, and all the rest will point to -* kretprobe_trampoline -*/ -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - -if (ri->rp && ri->rp->handler) - ri->rp->handler (ri, regs); - - orig_ret_address = (unsigned long )ri->ret_addr; -recycle_rp_inst (ri, &empty_rp); - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_assert (ri, orig_ret_address, trampoline_address); +unsigned long orig_ret_address; + orig_ret_address =__kretprobe_trampoline_handler (regs, + (unsigned long )&kretprobe_trampoline, +NULL );/* * We get here through one of two paths: * 1. by taking a trap -> kprobe_handler() -> here
@@ -458,13 +420,6 @@ static int trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) regs->nip = orig_ret_address -4 ; regs->link = orig_ret_address; -kretprobe_hash_unlock (current, &flags); - -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - } -return 0 ; }NOKPROBE_SYMBOL (trampoline_probe_handler);
Masami Hiramatsu
maintainerPATCH v2 11/15 s390: kprobes: Use generic kretprobe trampoline handler
In reply to Masami Hiramatsu--- arch/s390/kernel/kprobes.c | 81 ++------------------------------------------ 1 file changed, 4 insertions(+), 77 deletions(-)
arch/s390/kernel/kprobes.c +4 -77
@@ -228,6 +228,7 @@ NOKPROBE_SYMBOL(pop_kprobe);void arch_prepare_kretprobe (struct kretprobe_instance *ri,struct pt_regs *regs) { ri->ret_addr = (kprobe_opcode_t *) regs->gprs[14 ]; + ri->fp =NULL ;/* Replace the return addr with trampoline addr */ regs->gprs[14 ] = (unsigned long ) &kretprobe_trampoline;
@@ -331,83 +332,9 @@ static void __used kretprobe_trampoline_holder(void)*/ static int trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { -struct kretprobe_instance *ri; -struct hlist_head *head, empty_rp; -struct hlist_node *tmp; -unsigned long flags, orig_ret_address; -unsigned long trampoline_address; -kprobe_opcode_t *correct_ret_addr; - -INIT_HLIST_HEAD (&empty_rp); -kretprobe_hash_lock (current, &head, &flags); - -/* -* It is possible to have multiple instances associated with a given -* task either because an multiple functions in the call path -* have a return probe installed on them, and/or more than one return -* return probe was registered for a target function. -* -* We can handle this because: -* - instances are always inserted at the head of the list -* - when multiple return probes are registered for the same -* function, the first instance's ret_addr will point to the -* real return address, and all the rest will point to -* kretprobe_trampoline -*/ - ri =NULL ; - orig_ret_address =0 ; - correct_ret_addr =NULL ; - trampoline_address = (unsigned long ) &kretprobe_trampoline; -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - - orig_ret_address = (unsigned long ) ri->ret_addr; - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_assert (ri, orig_ret_address, trampoline_address); - - correct_ret_addr = ri->ret_addr; -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - - orig_ret_address = (unsigned long ) ri->ret_addr; - -if (ri->rp && ri->rp->handler) { - ri->ret_addr = correct_ret_addr; - ri->rp->handler (ri, regs); - } - -recycle_rp_inst (ri, &empty_rp); - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - - regs->psw.addr = orig_ret_address; - -kretprobe_hash_unlock (current, &flags); - -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - } + regs->psw.addr =__kretprobe_trampoline_handler (regs, + (unsigned long ) &kretprobe_trampoline, +NULL );/* * By returning a non-zero value, we are telling * kprobe_handler() that we don't want the post_handler
Masami Hiramatsu
maintainerPATCH v2 12/15 sh: kprobes: Use generic kretprobe trampoline handler
In reply to Masami Hiramatsu--- arch/sh/kernel/kprobes.c | 59 +++------------------------------------------- 1 file changed, 4 insertions(+), 55 deletions(-)
arch/sh/kernel/kprobes.c +4 -55
@@ -204,6 +204,7 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs) { ri->ret_addr = (kprobe_opcode_t *) regs->pr; + ri->fp =NULL ;/* Replace the return addr with trampoline addr */ regs->pr = (unsigned long )kretprobe_trampoline;
@@ -302,62 +303,10 @@ static void __used kretprobe_trampoline_holder(void)*/ int __kprobes trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { -struct kretprobe_instance *ri =NULL ; -struct hlist_head *head, empty_rp; -struct hlist_node *tmp; -unsigned long flags, orig_ret_address =0 ; -unsigned long trampoline_address = (unsigned long )&kretprobe_trampoline; + regs->pc =__kretprobe_trampoline_handler (regs, + (unsigned long )&kretprobe_trampoline,NULL ); -INIT_HLIST_HEAD (&empty_rp); -kretprobe_hash_lock (current, &head, &flags); - -/* -* It is possible to have multiple instances associated with a given -* task either because an multiple functions in the call path -* have a return probe installed on them, and/or more then one return -* return probe was registered for a target function. -* -* We can handle this because: -* - instances are always inserted at the head of the list -* - when multiple return probes are registered for the same -* function, the first instance's ret_addr will point to the -* real return address, and all the rest will point to -* kretprobe_trampoline -*/ -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - -if (ri->rp && ri->rp->handler) { -__this_cpu_write (current_kprobe, &ri->rp->kp); - ri->rp->handler (ri, regs); -__this_cpu_write (current_kprobe,NULL ); - } - - orig_ret_address = (unsigned long )ri->ret_addr; -recycle_rp_inst (ri, &empty_rp); - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_assert (ri, orig_ret_address, trampoline_address); - - regs->pc = orig_ret_address; -kretprobe_hash_unlock (current, &flags); - -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - } - -return orig_ret_address; +return 1 ; }static int __kprobes post_kprobe_handler (struct pt_regs *regs)
Masami Hiramatsu
maintainerPATCH v2 13/15 sparc: kprobes: Use generic kretprobe trampoline handler
In reply to Masami Hiramatsu--- arch/sparc/kernel/kprobes.c | 52 +++---------------------------------------- 1 file changed, 4 insertions(+), 48 deletions(-)
arch/sparc/kernel/kprobes.c +4 -48
@@ -453,6 +453,7 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs) { ri->ret_addr = (kprobe_opcode_t *)(regs->u_regs[UREG_RETPC ] +8 ); + ri->fp =NULL ;/* Replace the return addr with trampoline addr */ regs->u_regs[UREG_RETPC] =
@@ -465,58 +466,13 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,static int __kprobes trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { -struct kretprobe_instance *ri =NULL ; -struct hlist_head *head, empty_rp; -struct hlist_node *tmp; -unsigned long flags, orig_ret_address =0 ; -unsigned long trampoline_address =(unsigned long )&kretprobe_trampoline; +unsigned long orig_ret_address =0 ; -INIT_HLIST_HEAD (&empty_rp); -kretprobe_hash_lock (current, &head, &flags); - -/* -* It is possible to have multiple instances associated with a given -* task either because an multiple functions in the call path -* have a return probe installed on them, and/or more than one return -* return probe was registered for a target function. -* -* We can handle this because: -* - instances are always inserted at the head of the list -* - when multiple return probes are registered for the same -* function, the first instance's ret_addr will point to the -* real return address, and all the rest will point to -* kretprobe_trampoline -*/ -hlist_for_each_entry_safe (ri, tmp, head, hlist) { -if (ri->task != current) -/* another task is sharing our hash bucket */ -continue ; - -if (ri->rp && ri->rp->handler) - ri->rp->handler (ri, regs); - - orig_ret_address = (unsigned long )ri->ret_addr; -recycle_rp_inst (ri, &empty_rp); - -if (orig_ret_address != trampoline_address) -/* -* This is the real return address. Any other -* instances associated with this task are for -* other calls deeper on the call stack -*/ -break ; - } - -kretprobe_assert (ri, orig_ret_address, trampoline_address); + orig_ret_address =__kretprobe_trampoline_handler (regs, + (unsigned long )&kretprobe_trampoline,NULL ); regs->tpc = orig_ret_address; regs->tnpc = orig_ret_address +4 ; -kretprobe_hash_unlock (current, &flags); - -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - }/* * By returning a non-zero value, we are telling * kprobe_handler() that we don't want the post_handler
Masami Hiramatsu
maintainerPATCH v2 14/15 kprobes: Remove NMI context check
In reply to Masami HiramatsuSince the commit 9b38cc704e84 ("kretprobe: Prevent triggering kretprobe from within kprobe_flush_task") sets a dummy current kprobe in the trampoline handler by kprobe_busy_begin/end(), it is not possible to run a kretprobe pre handler in kretprobe trampoline handler context even with the NMI. If the NMI interrupts a kretprobe_trampoline_handler() and it hits a kretprobe, the 2nd kretprobe will detect recursion correctly and it will be skipped.
This means we have almost no double-lock issue on kretprobes by NMI.
The last one point is in cleanup_rp_inst() which also takes kretprobe_table_lock without setting up current kprobes. So adding kprobe_busy_begin/end() there allows us to remove in_nmi() check.
The above commit applies kprobe_busy_begin/end() on x86, but now all arch implementation are unified to generic one, we can safely remove the in_nmi() check from arch independent code.
--- kernel/kprobes.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
kernel/kprobes.c +4 -12
@@ -1359,7 +1359,8 @@ static void cleanup_rp_inst(struct kretprobe *rp)struct hlist_node *next;struct hlist_head *head; -/* No race here */ +/* To avoid recursive kretprobe by NMI, set kprobe busy here */ +kprobe_busy_begin ();for (hash =0 ; hash <KPROBE_TABLE_SIZE ; hash++) {kretprobe_table_lock (hash, &flags); head = &kretprobe_inst_table[hash];
@@ -1369,6 +1370,8 @@ static void cleanup_rp_inst(struct kretprobe *rp) }kretprobe_table_unlock (hash, &flags); } +kprobe_busy_end (); +free_rp_inst (rp); }NOKPROBE_SYMBOL (cleanup_rp_inst);
@@ -2038,17 +2041,6 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs)unsigned long hash, flags =0 ;struct kretprobe_instance *ri; -/* -* To avoid deadlocks, prohibit return probing in NMI contexts, -* just skip the probe and increase the (inexact) 'nmissed' -* statistical counter, so that the user is informed that -* something happened: -*/ -if (unlikely (in_nmi ())) { - rp->nmissed++; -return 0 ; - } -/* TODO: consider to only swap the RA after the last pre_handler fired */ hash =hash_ptr (current,KPROBE_HASH_BITS );raw_spin_lock_irqsave (&rp->lock, flags);
Masami Hiramatsu
maintainerPATCH v2 15/15 kprobes: Free kretprobe_instance with rcu callback
In reply to Masami HiramatsuFree kretprobe_instance with rcu callback instead of directly freeing the object in the kretprobe handler context.
This will make kretprobe run safer in NMI context.
--- include/linux/kprobes.h | 3 ++- kernel/kprobes.c | 25 ++++++------------------- 2 files changed, 8 insertions(+), 20 deletions(-)
include/linux/kprobes.h +2 -1
@@ -160,6 +160,7 @@ struct kretprobe_instance {struct kretprobe *rp;kprobe_opcode_t *ret_addr;struct task_struct *task; +struct rcu_head rcu;void *fp;char data[]; };
@@ -395,7 +396,7 @@ int register_kretprobes(struct kretprobe **rps, int num);void unregister_kretprobes (struct kretprobe **rps,int num);void kprobe_flush_task (struct task_struct *tk); -void recycle_rp_inst (struct kretprobe_instance *ri,struct hlist_head *head); +void recycle_rp_inst (struct kretprobe_instance *ri);int disable_kprobe (struct kprobe *kp);int enable_kprobe (struct kprobe *kp);
kernel/kprobes.c +6 -19
@@ -1223,8 +1223,7 @@ void kprobes_inc_nmissed_count(struct kprobe *p) }NOKPROBE_SYMBOL (kprobes_inc_nmissed_count); -void recycle_rp_inst (struct kretprobe_instance *ri, -struct hlist_head *head) +void recycle_rp_inst (struct kretprobe_instance *ri) {struct kretprobe *rp = ri->rp;
@@ -1236,8 +1235,7 @@ void recycle_rp_inst(struct kretprobe_instance *ri,hlist_add_head (&ri->hlist, &rp->free_instances);raw_spin_unlock (&rp->lock); }else -/* Unregistering */ -hlist_add_head (&ri->hlist, head); +kfree_rcu (ri, rcu); }NOKPROBE_SYMBOL (recycle_rp_inst);
@@ -1313,7 +1311,7 @@ void kprobe_busy_end(void)void kprobe_flush_task (struct task_struct *tk) {struct kretprobe_instance *ri; -struct hlist_head *head, empty_rp; +struct hlist_head *head;struct hlist_node *tmp;unsigned long hash, flags =0 ;
@@ -1323,19 +1321,14 @@ void kprobe_flush_task(struct task_struct *tk)kprobe_busy_begin (); -INIT_HLIST_HEAD (&empty_rp); hash =hash_ptr (tk,KPROBE_HASH_BITS ); head = &kretprobe_inst_table[hash];kretprobe_table_lock (hash, &flags);hlist_for_each_entry_safe (ri, tmp, head, hlist) {if (ri->task == tk) -recycle_rp_inst (ri, &empty_rp); +recycle_rp_inst (ri); }kretprobe_table_unlock (hash, &flags); -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - }kprobe_busy_end (); }
@@ -1936,13 +1929,12 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, void *frame_pointer) {struct kretprobe_instance *ri =NULL ; -struct hlist_head *head, empty_rp; +struct hlist_head *head;struct hlist_node *tmp;unsigned long flags, orig_ret_address =0 ;kprobe_opcode_t *correct_ret_addr =NULL ;bool skipped = false; -INIT_HLIST_HEAD (&empty_rp);kretprobe_hash_lock (current, &head, &flags);/*
@@ -2009,7 +2001,7 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs,__this_cpu_write (current_kprobe, &kprobe_busy); } -recycle_rp_inst (ri, &empty_rp); +recycle_rp_inst (ri);if (orig_ret_address != trampoline_address)/*
@@ -2022,11 +2014,6 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs,kretprobe_hash_unlock (current, &flags); -hlist_for_each_entry_safe (ri, tmp, &empty_rp, hlist) { -hlist_del (&ri->hlist); -kfree (ri); - } -return orig_ret_address; }NOKPROBE_SYMBOL (__kretprobe_trampoline_handler)
Re: [PATCH v2 15/15] kprobes: Free kretprobe_instance with rcu callback
Quoted 16 lines · Masami Hiramatsu
Free
kretprobe_instancewith rcu callback instead of directly freeing the object in the kretprobe handler context.This will make kretprobe run safer in NMI context.
Signed-off-by Masami Hiramatsu <[email protected]>--- include/linux/kprobes.h | 3 ++- kernel/kprobes.c | 25 ++++++------------------- 2 files changed, 8 insertions(+), 20 deletions(-)include/linux/kprobes.h +0
-0
include/linux/kprobes.h struct kretprobe_instance { · line 166
Quoted 7 lines
include/linux/kprobes.h +1
-0@@ -160,6 +160,7 @@ struct kretprobe_instance {struct kretprobe *rp;kprobe_opcode_t *ret_addr;struct task_struct *task; +struct rcu_head rcu;void *fp;char data[]; };
You can stick the rcu_head in a union with hlist.
Masami Hiramatsu
maintainerRe: [PATCH v2 15/15] kprobes: Free kretprobe_instance with rcu callback
Quoted 12 lines · On Thu, 27 Aug 2020 13:48:07 +0200 [email protected] wrote:
Quoted 11 lines · Masami Hiramatsu
Free
kretprobe_instancewith rcu callback instead of directly freeing the object in the kretprobe handler context.This will make kretprobe run safer in NMI context.
Signed-off-by Masami Hiramatsu <[email protected]>--- include/linux/kprobes.h | 3 ++- kernel/kprobes.c | 25 ++++++------------------- 2 files changed, 8 insertions(+), 20 deletions(-)
Quoted 14 lines
include/linux/kprobes.h struct kretprobe_instance { · line 166
Quoted 12 lines
include/linux/kprobes.h +1
-0@@ -160,6 +160,7 @@ struct kretprobe_instance {struct kretprobe *rp;kprobe_opcode_t *ret_addr;struct task_struct *task; +struct rcu_head rcu;void *fp;char data[]; };You can stick the
rcu_headin a union with hlist.
Indeed. OK, I'll update it.
Thank you!
Signature · 1 line
Masami Hiramatsu <[email protected]>
Re: [PATCH v2 15/15] kprobes: Free kretprobe_instance with rcu callback
In reply to Masami HiramatsuQuoted 1 line · Masami Hiramatsu
+void
recycle_rp_inst(struct kretprobe_instance *ri)
Also note, that at this point there is no external caller of this function anymore.
Masami Hiramatsu
maintainerRe: [PATCH v2 15/15] kprobes: Free kretprobe_instance with rcu callback
Quoted 6 lines · On Thu, 27 Aug 2020 13:49:19 +0200 [email protected] wrote:
Quoted 1 line · Masami Hiramatsu
+void
recycle_rp_inst(struct kretprobe_instance *ri)Also note, that at this point there is no external caller of this function anymore.
OK, actually, there are more local functions are exported without any reason. Let's make all static.
Thank you,
Signature · 1 line
Masami Hiramatsu <[email protected]>