multip register_uprobe

multip register_uprobe

[Wenji Huang]

Hi,

   I am creating some test scripts based on sarikar's framework.
  
   There is one problem about register multiple times.
       .................
        probes->vaddr = vaddr;
        probes->pid = pid;
        probes->handler = handler;
        for (i=0; i < 3; i++) {
             ret = register_uprobe(probes);
             if (ret!=0)  //of course, the first time should succeed and 
will failed in second time, ret = -16
               {
 I tried several possibilities:
                 * return ret; //RESULT: the module can't be removed, 
the probed process will hang, can't be killed/continued
                 * return 0; //RESULT: same as above
                 * unregister_uprobe(probes);kfree(probes);break; 
//RESULT: same as above
                 * kfree(probes);break; //RESULT: the module can be 
removed, the probed process will hang, can't be killed/continued
               }
         }
        return 0;
        .................

  But, no problem in "register once and unregister multiple times".  

  Environment:
    kernel: 2.6.21
    utrace: http://people.redhat.com/roland/utrace/2.6.21/
    uprobe: mail by Jim, on 2007/05/26
    All the tests in srikar's testsuite passed in the box.

    Any analysis? or need to update utrace/uprobe? Is there any website 
or cvs to get the resource?

Regards,  
-- 
 ***Wenji Huang
*   

Re: multip register_uprobe

[Srikar Dronamraju]

Hi Wenji,


>   I am creating some test scripts based on sarikar's framework.
>  
>   There is one problem about register multiple times.
>       .................
>        probes->vaddr = vaddr;
>        probes->pid = pid;
>        probes->handler = handler;
>        for (i=0; i < 3; i++) {
>             ret = register_uprobe(probes);
>             if (ret!=0)  //of course, the first time should succeed and 
> will failed in second time, ret = -16
>               {
> I tried several possibilities:
>                 * return ret; //RESULT: the module can't be removed, 
> the probed process will hang, can't be killed/continued
>                 * return 0; //RESULT: same as above
>                 * unregister_uprobe(probes);kfree(probes);break; 
> //RESULT: same as above
>                 * kfree(probes);break; //RESULT: the module can be 
> removed, the probed process will hang, can't be killed/continued
>               }
>         }
>        return 0;


uprobes doesn't allow to register using the same probe structure. 
Thats currently documented in the Documentation/uprobes.txt
Under Uprobes features and Limitations. 

Uprobes may produce unexpected results if you:
- assign non-zero values to reserved members of struct uprobe;
- change the contents of a uprobe or uretprobe object while it is
registered; or
- attempt to register a uprobe or uretprobe that is already registered.

Please do let me know if I it doesn't answer or if I have misunderstood 
your query.

--
Thanks and Regards
Srikar 

Re: multip register_uprobe

[Jim Keniston]

On Fri, 2007-07-06 at 18:05 +0530, Srikar Dronamraju wrote:
> Hi Wenji,
> 
> 
> >   I am creating some test scripts based on sarikar's framework.
> >  
> >   There is one problem about register multiple times.
> >       .................
> >        probes->vaddr = vaddr;
> >        probes->pid = pid;
> >        probes->handler = handler;
> >        for (i=0; i < 3; i++) {
> >             ret = register_uprobe(probes);
> >             if (ret!=0)  //of course, the first time should succeed and 
> > will failed in second time, ret = -16
> >               {
> > I tried several possibilities:
> >                 * return ret; //RESULT: the module can't be removed, 
> > the probed process will hang, can't be killed/continued
The above approach is incorrect because you fail the insmod but leave
the probe registered.  (I know, you're trying lots of different
things...)

> >                 * return 0; //RESULT: same as above
The above approach should work, assuming your cleanup function
unregisters the probe.

You have found a bug.  Thanks!  I will post a fix shortly.

> >                 * unregister_uprobe(probes);kfree(probes);break; 
> > //RESULT: same as above
The above approach should work, so long as your cleanup function "knows"
that the probes object has been kfreed, and doesn't try to unregister
it.

> >                 * kfree(probes);break; //RESULT: the module can be 
> > removed, the probed process will hang, can't be killed/continued
The above approach is incorrect because you kfree a uprobe that's still
registered.

> >               }
> >         }
> >        return 0;

> > But, no problem in "register once and unregister multiple times".
Yeah, unregistering an already-unregistered probe is treated as a no-op.
This allows your cleanup function to unregister its probes even if the
probed process has exited (at which time uprobes unregisters them on its
own).

> 
> 
> uprobes doesn't allow to register using the same probe structure. 
> Thats currently documented in the Documentation/uprobes.txt
> Under Uprobes features and Limitations. 
> 
> Uprobes may produce unexpected results if you:
> - assign non-zero values to reserved members of struct uprobe;
> - change the contents of a uprobe or uretprobe object while it is
> registered; or
> - attempt to register a uprobe or uretprobe that is already registered.

Correct.  But uprobes should handle such a failure more gracefully.  It
tries to, but in this failure mode, it neglects to unlock uproc->rwsem.

> 
> Please do let me know if I it doesn't answer or if I have misunderstood 
> your query.
> 
> --
> Thanks and Regards
> Srikar 

Thanks.
Jim

Re: multip register_uprobe

[Jim Keniston]

[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]

On Fri, 2007-07-06 at 09:37 -0700, Jim Keniston wrote:
> On Fri, 2007-07-06 at 18:05 +0530, Srikar Dronamraju wrote:
> > Hi Wenji,
> > 
> > 
> > >   I am creating some test scripts based on sarikar's framework.
> > >  
> > >   There is one problem about register multiple times.
> > >       .................
> > >        probes->vaddr = vaddr;
> > >        probes->pid = pid;
> > >        probes->handler = handler;
> > >        for (i=0; i < 3; i++) {
> > >             ret = register_uprobe(probes);
> > >             if (ret!=0)  //of course, the first time should succeed and 
> > > will failed in second time, ret = -16
> > >               {
> > > I tried several possibilities:
...
> 
> > >                 * return 0; //RESULT: same as above
> The above approach should work, assuming your cleanup function
> unregisters the probe.
> 
> You have found a bug.  Thanks!  I will post a fix shortly.
> 

Attached are a patch with the fix and a module (wenji3x.c) that
demonstrates the bug (or fix).  This patch applies atop the May 26 (GMT)
uprobes patch set.

Thanks again.
Jim

[-- Attachment #2: reused-probe-fix.patch --]
[-- Type: text/x-patch, Size: 818 bytes --]


If register_uprobe() or register_uretprobe() fails while attempting to
register a probe on a process that already has one or more u[ret]probes
in place, subsequent uprobes activity on that process (probe hits,
register/unregister attempts) may hang.  Here's a fix.

---

 kernel/uprobes.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff -puN kernel/uprobes.c~reused-probe-fix kernel/uprobes.c
--- linux-2.6.21-rc6/kernel/uprobes.c~reused-probe-fix	2007-07-06 09:16:49.000000000 -0700
+++ linux-2.6.21-rc6-jimk/kernel/uprobes.c	2007-07-06 09:18:56.000000000 -0700
@@ -830,8 +830,10 @@ fail_uproc:
 	if (uproc_is_new) {
 		uprobe_free_process(uproc);
 		mutex_unlock(&uproc_mutex);
-	} else
+	} else {
+		up_write(&uproc->rwsem);
 		uprobe_put_process(uproc);
+	}
 
 fail_tsk:
 	put_task_struct(p);
_

[-- Attachment #3: wenji3x.c --]
[-- Type: text/x-csrc, Size: 1659 bytes --]

/* uprobe_example.c, modified to try to register the same probe 3 times */
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/uprobes.h>

/*
 * Usage: insmod wenji3x.ko pid=<pid> vaddr=<address> [verbose=0]
 * where <pid> identifies the probed process and <address> is the virtual
 * address of the probed instruction.
 *
 * insmod should report success (0) for the first registration attempt
 * and failure (-16 = EBUSY) for the second and third attempts.  The
 * probe should work.
 */

static int pid = 0;
module_param(pid, int, 0);
MODULE_PARM_DESC(pid, "pid");

static int verbose = 1;
module_param(verbose, int, 0);
MODULE_PARM_DESC(verbose, "verbose");

static long vaddr = 0;
module_param(vaddr, long, 0);
MODULE_PARM_DESC(vaddr, "vaddr");

static int nhits;
static struct uprobe usp;

static void uprobe_handler(struct uprobe *u, struct pt_regs *regs)
{
	nhits++;
	if (verbose)
		printk(KERN_INFO "Hit #%d on probepoint at %#lx\n",
			nhits, u->vaddr);
}

int __init init_module(void)
{
	int ret, try, success = 0;
	usp.pid = pid;
	usp.vaddr = vaddr;
	usp.handler = uprobe_handler;
	printk(KERN_INFO "Registering uprobe on pid %d, vaddr %#lx\n",
		usp.pid, usp.vaddr);
	for (try = 1; try <= 3; try++) {
		ret = register_uprobe(&usp);
		printk(KERN_ERR "Try #%d: register_uprobe() returned %d\n",
			try, ret);
		if (ret == 0)
			success++;
	}
	return (success ? 0 : ret);
}

void __exit cleanup_module(void)
{
	printk(KERN_INFO "Unregistering uprobe on pid %d, vaddr %#lx\n",
		usp.pid, usp.vaddr);
	printk(KERN_INFO "Probepoint was hit %d times\n", nhits);
	unregister_uprobe(&usp);
}
MODULE_LICENSE("GPL");

Re: multip register_uprobe

[Wenji Huang]

Hi, Jim

   Got it, I also tried the several cases by changing hander/vaddr, but 
using same probe. Also works, no problem.

Thanks,
Wenji

Jim Keniston wrote:

>On Fri, 2007-07-06 at 09:37 -0700, Jim Keniston wrote:
>  
>
>>On Fri, 2007-07-06 at 18:05 +0530, Srikar Dronamraju wrote:
>>    
>>
>>>Hi Wenji,
>>>
>>>
>>>      
>>>
>>>>  I am creating some test scripts based on sarikar's framework.
>>>> 
>>>>  There is one problem about register multiple times.
>>>>      .................
>>>>       probes->vaddr = vaddr;
>>>>       probes->pid = pid;
>>>>       probes->handler = handler;
>>>>       for (i=0; i < 3; i++) {
>>>>            ret = register_uprobe(probes);
>>>>            if (ret!=0)  //of course, the first time should succeed and 
>>>>will failed in second time, ret = -16
>>>>              {
>>>>I tried several possibilities:
>>>>        
>>>>
>...
>  
>
>>>>                * return 0; //RESULT: same as above
>>>>        
>>>>
>>The above approach should work, assuming your cleanup function
>>unregisters the probe.
>>
>>You have found a bug.  Thanks!  I will post a fix shortly.
>>
>>    
>>
>
>Attached are a patch with the fix and a module (wenji3x.c) that
>demonstrates the bug (or fix).  This patch applies atop the May 26 (GMT)
>uprobes patch set.
>
>Thanks again.
>Jim
>  
>
>------------------------------------------------------------------------
>
>
>If register_uprobe() or register_uretprobe() fails while attempting to
>register a probe on a process that already has one or more u[ret]probes
>in place, subsequent uprobes activity on that process (probe hits,
>register/unregister attempts) may hang.  Here's a fix.
>
>---
>
> kernel/uprobes.c |    4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff -puN kernel/uprobes.c~reused-probe-fix kernel/uprobes.c
>--- linux-2.6.21-rc6/kernel/uprobes.c~reused-probe-fix	2007-07-06 09:16:49.000000000 -0700
>+++ linux-2.6.21-rc6-jimk/kernel/uprobes.c	2007-07-06 09:18:56.000000000 -0700
>@@ -830,8 +830,10 @@ fail_uproc:
> 	if (uproc_is_new) {
> 		uprobe_free_process(uproc);
> 		mutex_unlock(&uproc_mutex);
>-	} else
>+	} else {
>+		up_write(&uproc->rwsem);
> 		uprobe_put_process(uproc);
>+	}
> 
> fail_tsk:
> 	put_task_struct(p);
>_
>  
>
>------------------------------------------------------------------------
>
>/* uprobe_example.c, modified to try to register the same probe 3 times */
>#include <linux/module.h>
>#include <linux/kernel.h>
>#include <linux/init.h>
>#include <linux/uprobes.h>
>
>/*
> * Usage: insmod wenji3x.ko pid=<pid> vaddr=<address> [verbose=0]
> * where <pid> identifies the probed process and <address> is the virtual
> * address of the probed instruction.
> *
> * insmod should report success (0) for the first registration attempt
> * and failure (-16 = EBUSY) for the second and third attempts.  The
> * probe should work.
> */
>
>static int pid = 0;
>module_param(pid, int, 0);
>MODULE_PARM_DESC(pid, "pid");
>
>static int verbose = 1;
>module_param(verbose, int, 0);
>MODULE_PARM_DESC(verbose, "verbose");
>
>static long vaddr = 0;
>module_param(vaddr, long, 0);
>MODULE_PARM_DESC(vaddr, "vaddr");
>
>static int nhits;
>static struct uprobe usp;
>
>static void uprobe_handler(struct uprobe *u, struct pt_regs *regs)
>{
>	nhits++;
>	if (verbose)
>		printk(KERN_INFO "Hit #%d on probepoint at %#lx\n",
>			nhits, u->vaddr);
>}
>
>int __init init_module(void)
>{
>	int ret, try, success = 0;
>	usp.pid = pid;
>	usp.vaddr = vaddr;
>	usp.handler = uprobe_handler;
>	printk(KERN_INFO "Registering uprobe on pid %d, vaddr %#lx\n",
>		usp.pid, usp.vaddr);
>	for (try = 1; try <= 3; try++) {
>		ret = register_uprobe(&usp);
>		printk(KERN_ERR "Try #%d: register_uprobe() returned %d\n",
>			try, ret);
>		if (ret == 0)
>			success++;
>	}
>	return (success ? 0 : ret);
>}
>
>void __exit cleanup_module(void)
>{
>	printk(KERN_INFO "Unregistering uprobe on pid %d, vaddr %#lx\n",
>		usp.pid, usp.vaddr);
>	printk(KERN_INFO "Probepoint was hit %d times\n", nhits);
>	unregister_uprobe(&usp);
>}
>MODULE_LICENSE("GPL");
>  
>