[PATCH 5/7] kprobes: Remove called_from argument

[PATCH 5/7] kprobes: Remove called_from argument

[Masami Hiramatsu]

Remove called_from argument from kprobes which had been used for preventing
self-refering of kernel module. However, since we don't keep module's refcount
after registering kprobe any more, there is no reason to check that.

This patch also simplifies registering/unregistering functions because we don't
need to use __builtin_return_address(0) which was passed to called_from.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
---
 kernel/kprobes.c |   70 +++++++++++--------------------------------------------
 1 file changed, 14 insertions(+), 56 deletions(-)

Index: 2.6.28-rc4/kernel/kprobes.c
===================================================================
--- 2.6.28-rc4.orig/kernel/kprobes.c
+++ 2.6.28-rc4/kernel/kprobes.c
@@ -641,8 +641,7 @@ static kprobe_opcode_t __kprobes *kprobe
 	return (kprobe_opcode_t *)(((char *)addr) + p->offset);
 }

-static int __kprobes __register_kprobe(struct kprobe *p,
-	unsigned long called_from)
+int __kprobes register_kprobe(struct kprobe *p)
 {
 	int ret = 0;
 	struct kprobe *old_p;
@@ -673,13 +672,10 @@ static int __kprobes __register_kprobe(s
 		 * We must hold a refcount of the probed module while updating
 		 * its code to prohibit unexpected unloading.
 		 */
-		if (calling_mod != probed_mod) {
-			if (unlikely(!try_module_get(probed_mod))) {
-				preempt_enable();
-				return -EINVAL;
-			}
-		} else
-			probed_mod = NULL;
+		if (unlikely(!try_module_get(probed_mod))) {
+			preempt_enable();
+			return -EINVAL;
+		}
 	}
 	preempt_enable();

@@ -773,15 +769,14 @@ static void __kprobes __unregister_kprob
 	}
 }

-static int __kprobes __register_kprobes(struct kprobe **kps, int num,
-	unsigned long called_from)
+int __kprobes register_kprobes(struct kprobe **kps, int num)
 {
 	int i, ret = 0;

 	if (num <= 0)
 		return -EINVAL;
 	for (i = 0; i < num; i++) {
-		ret = __register_kprobe(kps[i], called_from);
+		ret = register_kprobe(kps[i]);
 		if (ret < 0) {
 			if (i > 0)
 				unregister_kprobes(kps, i);
@@ -791,26 +786,11 @@ static int __kprobes __register_kprobes(
 	return ret;
 }

-/*
- * Registration and unregistration functions for kprobe.
- */
-int __kprobes register_kprobe(struct kprobe *p)
-{
-	return __register_kprobes(&p, 1,
-				  (unsigned long)__builtin_return_address(0));
-}
-
 void __kprobes unregister_kprobe(struct kprobe *p)
 {
 	unregister_kprobes(&p, 1);
 }

-int __kprobes register_kprobes(struct kprobe **kps, int num)
-{
-	return __register_kprobes(kps, num,
-				  (unsigned long)__builtin_return_address(0));
-}
-
 void __kprobes unregister_kprobes(struct kprobe **kps, int num)
 {
 	int i;
@@ -839,8 +819,7 @@ unsigned long __weak arch_deref_entry_po
 	return (unsigned long)entry;
 }

-static int __kprobes __register_jprobes(struct jprobe **jps, int num,
-	unsigned long called_from)
+int __kprobes register_jprobes(struct jprobe **jps, int num)
 {
 	struct jprobe *jp;
 	int ret = 0, i;
@@ -858,7 +837,7 @@ static int __kprobes __register_jprobes(
 			/* Todo: Verify probepoint is a function entry point */
 			jp->kp.pre_handler = setjmp_pre_handler;
 			jp->kp.break_handler = longjmp_break_handler;
-			ret = __register_kprobe(&jp->kp, called_from);
+			ret = register_kprobe(&jp->kp);
 		}
 		if (ret < 0) {
 			if (i > 0)
@@ -871,8 +850,7 @@ static int __kprobes __register_jprobes(

 int __kprobes register_jprobe(struct jprobe *jp)
 {
-	return __register_jprobes(&jp, 1,
-		(unsigned long)__builtin_return_address(0));
+	return register_jprobes(&jp, 1);
 }

 void __kprobes unregister_jprobe(struct jprobe *jp)
@@ -880,12 +858,6 @@ void __kprobes unregister_jprobe(struct
 	unregister_jprobes(&jp, 1);
 }

-int __kprobes register_jprobes(struct jprobe **jps, int num)
-{
-	return __register_jprobes(jps, num,
-		(unsigned long)__builtin_return_address(0));
-}
-
 void __kprobes unregister_jprobes(struct jprobe **jps, int num)
 {
 	int i;
@@ -948,8 +920,7 @@ static int __kprobes pre_handler_kretpro
 	return 0;
 }

-static int __kprobes __register_kretprobe(struct kretprobe *rp,
-					  unsigned long called_from)
+int __kprobes register_kretprobe(struct kretprobe *rp)
 {
 	int ret = 0;
 	struct kretprobe_instance *inst;
@@ -995,21 +966,20 @@ static int __kprobes __register_kretprob

 	rp->nmissed = 0;
 	/* Establish function entry probe point */
-	ret = __register_kprobe(&rp->kp, called_from);
+	ret = register_kprobe(&rp->kp);
 	if (ret != 0)
 		free_rp_inst(rp);
 	return ret;
 }

-static int __kprobes __register_kretprobes(struct kretprobe **rps, int num,
-	unsigned long called_from)
+int __kprobes register_kretprobes(struct kretprobe **rps, int num)
 {
 	int ret = 0, i;

 	if (num <= 0)
 		return -EINVAL;
 	for (i = 0; i < num; i++) {
-		ret = __register_kretprobe(rps[i], called_from);
+		ret = register_kretprobe(rps[i]);
 		if (ret < 0) {
 			if (i > 0)
 				unregister_kretprobes(rps, i);
@@ -1019,23 +989,11 @@ static int __kprobes __register_kretprob
 	return ret;
 }

-int __kprobes register_kretprobe(struct kretprobe *rp)
-{
-	return __register_kretprobes(&rp, 1,
-			(unsigned long)__builtin_return_address(0));
-}
-
 void __kprobes unregister_kretprobe(struct kretprobe *rp)
 {
 	unregister_kretprobes(&rp, 1);
 }

-int __kprobes register_kretprobes(struct kretprobe **rps, int num)
-{
-	return __register_kretprobes(rps, num,
-			(unsigned long)__builtin_return_address(0));
-}
-
 void __kprobes unregister_kretprobes(struct kretprobe **rps, int num)
 {
 	int i;

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com

Re: [PATCH 5/7] kprobes: Remove called_from argument

[Ananth N Mavinakayanahalli]

On Tue, Nov 11, 2008 at 03:57:13PM -0500, Masami Hiramatsu wrote:
> Remove called_from argument from kprobes which had been used for preventing
> self-refering of kernel module. However, since we don't keep module's refcount
> after registering kprobe any more, there is no reason to check that.
> 
> This patch also simplifies registering/unregistering functions because we don't
> need to use __builtin_return_address(0) which was passed to called_from.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
> ---
>  kernel/kprobes.c |   70 +++++++++++--------------------------------------------
>  1 file changed, 14 insertions(+), 56 deletions(-)
> 
> Index: 2.6.28-rc4/kernel/kprobes.c
> ===================================================================
> --- 2.6.28-rc4.orig/kernel/kprobes.c
> +++ 2.6.28-rc4/kernel/kprobes.c
> @@ -641,8 +641,7 @@ static kprobe_opcode_t __kprobes *kprobe
>  	return (kprobe_opcode_t *)(((char *)addr) + p->offset);
>  }
> 
> -static int __kprobes __register_kprobe(struct kprobe *p,
> -	unsigned long called_from)
> +int __kprobes register_kprobe(struct kprobe *p)
>  {
>  	int ret = 0;
>  	struct kprobe *old_p;
> @@ -673,13 +672,10 @@ static int __kprobes __register_kprobe(s
>  		 * We must hold a refcount of the probed module while updating
>  		 * its code to prohibit unexpected unloading.
>  		 */
> -		if (calling_mod != probed_mod) {
> -			if (unlikely(!try_module_get(probed_mod))) {
> -				preempt_enable();
> -				return -EINVAL;
> -			}
> -		} else
> -			probed_mod = NULL;
> +		if (unlikely(!try_module_get(probed_mod))) {
> +			preempt_enable();
> +			return -EINVAL;
> +		}

You also need the following to get this to compile, don't you?

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Index: linux-2.6.28-rc4/kernel/kprobes.c
===================================================================
--- linux-2.6.28-rc4.orig/kernel/kprobes.c	2008-11-12 11:13:30.000000000 +0530
+++ linux-2.6.28-rc4/kernel/kprobes.c	2008-11-12 12:18:44.000000000 +0530
@@ -666,8 +666,6 @@
 	 */
 	probed_mod = __module_text_address((unsigned long) p->addr);
 	if (probed_mod) {
-		struct module *calling_mod;
-		calling_mod = __module_text_address(called_from);
 		/*
 		 * We must hold a refcount of the probed module while updating
 		 * its code to prohibit unexpected unloading.

Re: [PATCH 5/7] kprobes: Remove called_from argument

[Masami Hiramatsu]

Hi Ananth,

Ananth N Mavinakayanahalli wrote:
> 
> You also need the following to get this to compile, don't you?

Oops, Indeed.
Thank you very much!
Where I've lost this parts...?

> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Acked-by: Masami Hiramatsu <mhiramat@redhat.com>

> 
> Index: linux-2.6.28-rc4/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.28-rc4.orig/kernel/kprobes.c	2008-11-12 11:13:30.000000000 +0530
> +++ linux-2.6.28-rc4/kernel/kprobes.c	2008-11-12 12:18:44.000000000 +0530
> @@ -666,8 +666,6 @@
>  	 */
>  	probed_mod = __module_text_address((unsigned long) p->addr);
>  	if (probed_mod) {
> -		struct module *calling_mod;
> -		calling_mod = __module_text_address(called_from);
>  		/*
>  		 * We must hold a refcount of the probed module while updating
>  		 * its code to prohibit unexpected unloading.

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com

Re: [PATCH 5/7] kprobes: Remove called_from argument

[Ananth N Mavinakayanahalli]

On Tue, Nov 11, 2008 at 03:57:13PM -0500, Masami Hiramatsu wrote:
> Remove called_from argument from kprobes which had been used for preventing
> self-refering of kernel module. However, since we don't keep module's refcount
> after registering kprobe any more, there is no reason to check that.
> 
> This patch also simplifies registering/unregistering functions because we don't
> need to use __builtin_return_address(0) which was passed to called_from.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
> ---
>  kernel/kprobes.c |   70 +++++++++++--------------------------------------------
>  1 file changed, 14 insertions(+), 56 deletions(-)

Subject to inclusion of the two-liner I sent out earlier today on top of this one
(needed anyway to get this to compile)..

Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>