kprobes: Remove called_from argument

PATCH In review Masami Hiramatsu 2 participants 4 messages Nov 11, 2008 – Nov 12, 2008 kprobes

Share & tools

Masami Hiramatsu

author

2008-11-11 20:58 UTC #0 lore raw

PATCH 5/7 kprobes: Remove called_from argument

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 <[email protected]>

---
 kernel/kprobes.c |   70 +++++++++++--------------------------------------------
 1 file changed, 14 insertions(+), 56 deletions(-)

kernel/kprobes.c +14 ~~-56~~

Index: 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;

Signature · 7 lines

Masami Hiramatsu

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

e-mail: [email protected]

Ananth N Mavinakayanahalli

2008-11-12 06:53 UTC #1 lore raw

Quoted 31 lines · 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 <[email protected]>
---
 kernel/kprobes.c |   70 +++++++++++--------------------------------------------
 1 file changed, 14 insertions(+), 56 deletions(-)
Index: 2.6.28-rc4/kernel/kprobes.c
===================================================================
kernel/kprobes.c +1 -3
@@ -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) {

kernel/kprobes.c static int __kprobes __register_kprobe(s · line 678

Quoted 10 lines

kernel/kprobes.c +4 -6

-			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 <[email protected]>

kernel/kprobes.c +0 -2

Index: linux-2.6.28-rc4/kernel/kprobes.c
===================================================================
@@ -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

author Acked-by

2008-11-12 15:37 UTC #2 lore raw

Hi Ananth,

Quoted 2 lines · Ananth N Mavinakayanahalli

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...?

Quoted 1 line

Signed-off-by Ananth N Mavinakayanahalli <[email protected]>

Acked-by Masami Hiramatsu <[email protected]>

Quoted 14 lines

Index: linux-2.6.28-rc4/kernel/kprobes.c
===================================================================

kernel/kprobes.c +0 -2

@@ -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.

Signature · 7 lines

Masami Hiramatsu

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

e-mail: [email protected]

Ananth N Mavinakayanahalli

Acked-by on v1 5/7

2008-11-12 15:59 UTC #3 lore raw

In reply to Masami Hiramatsu

Quoted 11 lines · 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 <[email protected]>
---
 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 <[email protected]>