Re: [Fwd: Re: [PATCH] Return probe]

[Richard J Moore <richardj_moore at uk dot ibm dot com>]

Do you take into account the problems I documented in my technical
disclosure on implementing return points? These are:

1) What happens if the return address is modified by the called program
(legitimately or otherwise)?
2) What happens if a multi-level function return takes place? This is quite
possible with optimization and some finite-state-machine implementations.
3) What happens in the function call is register based (PPC, zSeries etc,
optimized IA32 etc...)

- -
Richard J Moore
IBM Advanced Linux Response Team - Linux Technology Centre
MOBEX: 264807; Mobile (+44) (0)7739-875237
Office: (+44) (0)1962-817072


                                                                           
             Hien Nguyen                                                   
             <hien@us.ibm.co                                               
             m>                                                         To 
             Sent by:                   prasanna@in.ibm.com                
             systemtap-owner                                            cc 
             @sources.redhat            jkenisto@us.ibm.com, Vara Prasad   
             .com                       <varap@us.ibm.com>, SystemTAP      
                                        <systemtap@sources.redhat.com>     
                                                                       bcc 
             19/04/2005                                                    
             17:11                                                 Subject 
                                        Re: [Fwd: Re: [PATCH] Return       
                                        probe]                             
                                                                           
                                                                           




See my reply below for unregister_kretprobe.

Prasanna S Panchamukhi wrote:

>Hi Jim, Hien,
>
>Please see my comments below.
>
>
>
>>+/*
>>+ * This function is called from do_exit or do_execv when task tk's stack
is
>>+ * about to be recycled. Recycle any function-return probe instances
>>+ * associated with this task. These represent probed functions that have

>>+ * been called but may never return.
>>+ */
>>+void kprobe_flush_task(struct task_struct *tk)
>>+{
>>+          unsigned long flags = 0;
>>+          struct kretprobe_instance *ri;
>>+          struct task_struct *tsk;
>>+          struct hlist_head *head;
>>+          struct hlist_node *node;
>>+
>>+          if (!arch_supports_kretprobes) {
>>+                      return;
>>+          }
>>+          spin_lock_irqsave(&kprobe_lock, flags);
>>+          head = &kretprobe_inst_table[hash_ptr(tk, RPROBE_HASH_BITS)];
>>+          hlist_for_each_entry(ri, node, head, hlist) {
>>+                      tsk = arch_get_kprobe_task(ri->stack_addr);
>>+                      if (tsk == tk) {
>>+                                  /* Put the original return address
back into stack */
>>+                                  *((unsigned long *)(ri->stack_addr)) =
(unsigned long) ri->ret_addr;
>>+                                  hlist_del_rcu(&ri->hlist);
>>+                                  recycle_kretprobe_instance(ri);
>>+                      }
>>+          }
>>+          spin_unlock_irqrestore(&kprobe_lock, flags);
>>+}
>>+
>>
>>
>
>The current implementation modifies the return address on the stack, hence
the
>above routine called for every do_exit, do_execv. How much performance
impact
>will this cause?
>
>
>
>>+
>>+          rp_tmp = kmalloc(sizeof(struct kretprobe), GFP_KERNEL);
>>+          BUG_ON(rp_tmp == NULL);
>>+
>>+          spin_lock_irqsave(&kprobe_lock, flags);
>>+          old_p = get_kprobe(rp->kp.addr);
>>+          if (old_p && (old_p->pre_handler == aggr_pre_handler)) {
>>+                      list_del(&rp->kp.list);
>>+                      if (list_empty(&old_p->list)) {
>>+                                  remove_kprobe(old_p, flags);
>>+                                  kfree(old_p);
>>+                      }
>>+          } else if (old_p == &rp->kp) {
>>+                      remove_kprobe(&rp->kp, flags);
>>+          }
>>
>>
>
>The patch by Ananth provides multiple probes feature at a given address
and
>handles all the above cases. Can you pls check if the above checks can be
>removed and use multiple probes interface.
>
>
>
I need to get the hold of the lock before unregistering, I could not the
current unregister_kprobe / unregistering_kprobe_single /
unregister_aggr_kprobe as is without any modification, hence this code
is here.

>Thanks
>Prasanna
>
>
>