kernel/kprobes: A few trivial updates to jprobes

PATCH In review Naveen N. Rao 3 participants 8 messages Jul 7, 2017 – Jul 9, 2017

Subsystems (3)

PowerPC Tracing kprobes

lore

Share & tools

lore

Naveen N. Rao

author

2017-07-07 17:07 UTC #0 lore raw

v2 cover kernel/kprobes: A few trivial updates to jprobes

Here is v2 of the patch:
https://www.mail-archive.com/[email protected]/msg1434133.html

As suggested, the first two patches first clean up the existing jprobe registration functions and rename function_offset_within_entry() to kprobe_on_func_entry(). The last patch has been updated accordingly.

Thanks,
Naveen

Naveen N. Rao (3):
kernel/kprobes: Rename [arch_]function_offset_within_entry() to

    [arch_]kprobe_on_func_entry()

kernel/kprobes: Simplify register_jprobes() kernel/kprobes: Ensure that jprobe probepoints are at function entry

 arch/powerpc/kernel/kprobes.c |  2 +-
 include/linux/kprobes.h       |  4 ++--
 kernel/kprobes.c              | 42 ++++++++++++++++++++++++------------------
 kernel/trace/trace_kprobe.c   |  2 +-
 4 files changed, 28 insertions(+), 22 deletions(-)

Signature · 1 line

2.13.2

Naveen N. Rao

author

2017-07-07 17:07 UTC #1 lore raw

PATCH v2 1/3 kernel/kprobes: Rename [arch_]function_offset_within_entry() to [arch_]kprobe_on_func_entry()

Rename function_offset_within_entry() to scope it to kprobe namespace by using kprobe_ prefix, and to also simplify it.

Signed-off-by Naveen N. Rao <[email protected]>

---
 arch/powerpc/kernel/kprobes.c | 2 +-
 include/linux/kprobes.h       | 4 ++--
 kernel/kprobes.c              | 8 ++++----
 kernel/trace/trace_kprobe.c   | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

arch/powerpc/kernel/kprobes.c +1 -1

index 01addfb0ed0a..586508e949f0 100644
@@ -221,7 +221,7 @@ static nokprobe_inline void set_current_kprobe(struct kprobe *p, struct pt_regs
 	kcb->kprobe_saved_msr = regs->msr;
 }
 
-bool arch_function_offset_within_entry(unsigned long offset)
+bool arch_kprobe_on_func_entry(unsigned long offset)
 {
 #ifdef PPC64_ELF_ABI_v2
 #ifdef CONFIG_KPROBES_ON_FTRACE

include/linux/kprobes.h +2 -2

index 541df0b5b815..bd2684700b74 100644
@@ -267,8 +267,8 @@ extern int arch_init_kprobes(void);
 extern void show_registers(struct pt_regs *regs);
 extern void kprobes_inc_nmissed_count(struct kprobe *p);
 extern bool arch_within_kprobe_blacklist(unsigned long addr);
-extern bool arch_function_offset_within_entry(unsigned long offset);
-extern bool function_offset_within_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset);
+extern bool arch_kprobe_on_func_entry(unsigned long offset);
+extern bool kprobe_on_func_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset);
 
 extern bool within_kprobe_blacklist(unsigned long addr);

kernel/kprobes.c +4 -4

index 6756d750b31b..a519219169fd 100644
@@ -1888,12 +1888,12 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs)
 }
 NOKPROBE_SYMBOL(pre_handler_kretprobe);
 
-bool __weak arch_function_offset_within_entry(unsigned long offset)
+bool __weak arch_kprobe_on_func_entry(unsigned long offset)
 {
 	return !offset;
 }
 
-bool function_offset_within_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset)
+bool kprobe_on_func_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset)
 {
 	kprobe_opcode_t *kp_addr = _kprobe_addr(addr, sym, offset);

@@ -1901,7 +1901,7 @@ bool function_offset_within_entry(kprobe_opcode_t *addr, const char *sym, unsign
 		return false;
 
 	if (!kallsyms_lookup_size_offset((unsigned long)kp_addr, NULL, &offset) ||
-						!arch_function_offset_within_entry(offset))
+						!arch_kprobe_on_func_entry(offset))
 		return false;
 
 	return true;

@@ -1914,7 +1914,7 @@ int register_kretprobe(struct kretprobe *rp)
 	int i;
 	void *addr;
 
-	if (!function_offset_within_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset))
+	if (!kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset))
 		return -EINVAL;
 
 	if (kretprobe_blacklist_size) {

kernel/trace/trace_kprobe.c +1 -1

index b53c8d369163..2c5221819be5 100644
@@ -720,7 +720,7 @@ static int create_trace_kprobe(int argc, char **argv)
 			return ret;
 		}
 		if (offset && is_return &&
-		    !function_offset_within_entry(NULL, symbol, offset)) {
+		    !kprobe_on_func_entry(NULL, symbol, offset)) {
 			pr_info("Given offset is not valid for return probe.\n");
 			return -EINVAL;
 		}

Signature · 1 line

2.13.2

tip-bot for Naveen N. Rao

merge bot

2017-07-08 11:13 UTC #2 lore raw

[tip:perf/urgent] kprobes: Rename [arch_]function_offset_within_entry() to [arch_]kprobe_on_func_entry()

kprobes: Rename [arch_]function_offset_within_entry() to [arch_]kprobe_on_func_entry()

Commit-ID:  659b957f20c78fd470083c80af5e79eedfb39e5b
Gitweb:     http://git.kernel.org/tip/659b957f20c78fd470083c80af5e79eedfb39e5b
Author:     Naveen N. Rao <[email protected]>
AuthorDate: Fri, 7 Jul 2017 22:37:24 +0530
Committer:  Ingo Molnar <[email protected]>
CommitDate: Sat, 8 Jul 2017 11:05:34 +0200

kprobes: Rename [arch_]function_offset_within_entry() to [arch_]kprobe_on_func_entry()

Rename function_offset_within_entry() to scope it to kprobe namespace by using kprobe_ prefix, and to also simplify it.

Signed-off-by Naveen N. Rao <[email protected]>

Cc Ananth N Mavinakayanahalli <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/3aa6c7e2e4fb6e00f3c24fa306496a66edb558ea.1499443367.git.naveen.n.rao@linux.vnet.ibm.com open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/powerpc/kernel/kprobes.c | 2 +-
 include/linux/kprobes.h       | 4 ++--
 kernel/kprobes.c              | 8 ++++----
 kernel/trace/trace_kprobe.c   | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

arch/powerpc/kernel/kprobes.c +1 -1

index 01addfb..586508e 100644
@@ -221,7 +221,7 @@ static nokprobe_inline void set_current_kprobe(struct kprobe *p, struct pt_regs
 	kcb->kprobe_saved_msr = regs->msr;
 }
 
-bool arch_function_offset_within_entry(unsigned long offset)
+bool arch_kprobe_on_func_entry(unsigned long offset)
 {
 #ifdef PPC64_ELF_ABI_v2
 #ifdef CONFIG_KPROBES_ON_FTRACE

include/linux/kprobes.h +2 -2

index 541df0b..bd26847 100644
@@ -267,8 +267,8 @@ extern int arch_init_kprobes(void);
 extern void show_registers(struct pt_regs *regs);
 extern void kprobes_inc_nmissed_count(struct kprobe *p);
 extern bool arch_within_kprobe_blacklist(unsigned long addr);
-extern bool arch_function_offset_within_entry(unsigned long offset);
-extern bool function_offset_within_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset);
+extern bool arch_kprobe_on_func_entry(unsigned long offset);
+extern bool kprobe_on_func_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset);
 
 extern bool within_kprobe_blacklist(unsigned long addr);

kernel/kprobes.c +4 -4

index 6756d75..a519219 100644
@@ -1888,12 +1888,12 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs)
 }
 NOKPROBE_SYMBOL(pre_handler_kretprobe);
 
-bool __weak arch_function_offset_within_entry(unsigned long offset)
+bool __weak arch_kprobe_on_func_entry(unsigned long offset)
 {
 	return !offset;
 }
 
-bool function_offset_within_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset)
+bool kprobe_on_func_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset)
 {
 	kprobe_opcode_t *kp_addr = _kprobe_addr(addr, sym, offset);

@@ -1901,7 +1901,7 @@ bool function_offset_within_entry(kprobe_opcode_t *addr, const char *sym, unsign
 		return false;
 
 	if (!kallsyms_lookup_size_offset((unsigned long)kp_addr, NULL, &offset) ||
-						!arch_function_offset_within_entry(offset))
+						!arch_kprobe_on_func_entry(offset))
 		return false;
 
 	return true;

@@ -1914,7 +1914,7 @@ int register_kretprobe(struct kretprobe *rp)
 	int i;
 	void *addr;
 
-	if (!function_offset_within_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset))
+	if (!kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset))
 		return -EINVAL;
 
 	if (kretprobe_blacklist_size) {

kernel/trace/trace_kprobe.c +1 -1

index b53c8d3..2c52218 100644
@@ -720,7 +720,7 @@ static int create_trace_kprobe(int argc, char **argv)
 			return ret;
 		}
 		if (offset && is_return &&
-		    !function_offset_within_entry(NULL, symbol, offset)) {
+		    !kprobe_on_func_entry(NULL, symbol, offset)) {
 			pr_info("Given offset is not valid for return probe.\n");
 			return -EINVAL;
 		}

Naveen N. Rao

author

2017-07-07 17:08 UTC #3 lore raw

PATCH v2 2/3 kernel/kprobes: Simplify register_jprobes()

In reply to Naveen N. Rao

Re-factor jprobe registration functions as the current version is getting too unwieldy. Move the actual jprobe registration to register_jprobe() and re-organize code accordingly.

Signed-off-by Naveen N. Rao <[email protected]>

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

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

index a519219169fd..db3cd3e60bdd 100644
@@ -1771,24 +1771,13 @@ unsigned long __weak arch_deref_entry_point(void *entry)
 
 int register_jprobes(struct jprobe **jps, int num)
 {
-	struct jprobe *jp;
 	int ret = 0, i;
 
 	if (num <= 0)
 		return -EINVAL;
+
 	for (i = 0; i < num; i++) {
-		unsigned long addr, offset;
-		jp = jps[i];
-		addr = arch_deref_entry_point(jp->entry);
-
-		/* Verify probepoint is a function entry point */
-		if (kallsyms_lookup_size_offset(addr, NULL, &offset) &&
-		    offset == 0) {
-			jp->kp.pre_handler = setjmp_pre_handler;
-			jp->kp.break_handler = longjmp_break_handler;
-			ret = register_kprobe(&jp->kp);
-		} else
-			ret = -EINVAL;
+		ret = register_jprobe(jps[i]);
 
 		if (ret < 0) {
 			if (i > 0)

@@ -1796,13 +1785,26 @@ int register_jprobes(struct jprobe **jps, int num)
 			break;
 		}
 	}
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(register_jprobes);
 
 int register_jprobe(struct jprobe *jp)
 {
-	return register_jprobes(&jp, 1);
+	unsigned long addr, offset;
+	struct kprobe *kp = &jp->kp;
+
+	/* Verify probepoint is a function entry point */
+	addr = arch_deref_entry_point(jp->entry);
+
+	if (kallsyms_lookup_size_offset(addr, NULL, &offset) && offset == 0) {
+		kp->pre_handler = setjmp_pre_handler;
+		kp->break_handler = longjmp_break_handler;
+		return register_kprobe(kp);
+	}
+
+	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(register_jprobe);

Signature · 1 line

2.13.2

tip-bot for Naveen N. Rao

merge bot

2017-07-08 11:14 UTC #4 lore raw

[tip:perf/urgent] kprobes: Simplify register_jprobes()

kprobes: Simplify register_jprobes()

Commit-ID:  0f73ff80b751b39ff539a550e65c5bd131ff0316
Gitweb:     http://git.kernel.org/tip/0f73ff80b751b39ff539a550e65c5bd131ff0316
Author:     Naveen N. Rao <[email protected]>
AuthorDate: Fri, 7 Jul 2017 22:37:25 +0530
Committer:  Ingo Molnar <[email protected]>
CommitDate: Sat, 8 Jul 2017 11:05:34 +0200

kprobes: Simplify register_jprobes()

Re-factor jprobe registration functions as the current version is getting too unwieldy. Move the actual jprobe registration to register_jprobe() and re-organize code accordingly.

Signed-off-by Naveen N. Rao <[email protected]>

Cc Ananth N Mavinakayanahalli <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Masami Hiramatsu <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/089cae4bfe73767f765291ee0e6fb0c3d240e5f1.1499443367.git.naveen.n.rao@linux.vnet.ibm.com open here

Signed-off-by Ingo Molnar <[email protected]>

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

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

index a519219..db3cd3e 100644
@@ -1771,24 +1771,13 @@ unsigned long __weak arch_deref_entry_point(void *entry)
 
 int register_jprobes(struct jprobe **jps, int num)
 {
-	struct jprobe *jp;
 	int ret = 0, i;
 
 	if (num <= 0)
 		return -EINVAL;
+
 	for (i = 0; i < num; i++) {
-		unsigned long addr, offset;
-		jp = jps[i];
-		addr = arch_deref_entry_point(jp->entry);
-
-		/* Verify probepoint is a function entry point */
-		if (kallsyms_lookup_size_offset(addr, NULL, &offset) &&
-		    offset == 0) {
-			jp->kp.pre_handler = setjmp_pre_handler;
-			jp->kp.break_handler = longjmp_break_handler;
-			ret = register_kprobe(&jp->kp);
-		} else
-			ret = -EINVAL;
+		ret = register_jprobe(jps[i]);
 
 		if (ret < 0) {
 			if (i > 0)

@@ -1796,13 +1785,26 @@ int register_jprobes(struct jprobe **jps, int num)
 			break;
 		}
 	}
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(register_jprobes);
 
 int register_jprobe(struct jprobe *jp)
 {
-	return register_jprobes(&jp, 1);
+	unsigned long addr, offset;
+	struct kprobe *kp = &jp->kp;
+
+	/* Verify probepoint is a function entry point */
+	addr = arch_deref_entry_point(jp->entry);
+
+	if (kallsyms_lookup_size_offset(addr, NULL, &offset) && offset == 0) {
+		kp->pre_handler = setjmp_pre_handler;
+		kp->break_handler = longjmp_break_handler;
+		return register_kprobe(kp);
+	}
+
+	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(register_jprobe);

Naveen N. Rao

author

2017-07-07 17:08 UTC #5 lore raw

PATCH v2 3/3 kernel/kprobes: Ensure that jprobe probepoints are at function entry

In reply to Naveen N. Rao

Similar to commit 90ec5e89e393c ("kretprobes: Ensure probe location is at function entry"), ensure that the jprobe probepoint is at function entry.

Signed-off-by Naveen N. Rao <[email protected]>

---
 kernel/kprobes.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

kernel/kprobes.c +6 -2

index db3cd3e60bdd..a1606a4224e1 100644
@@ -1795,10 +1795,14 @@ int register_jprobe(struct jprobe *jp)
 	unsigned long addr, offset;
 	struct kprobe *kp = &jp->kp;
 
-	/* Verify probepoint is a function entry point */
+	/*
+	 * Verify probepoint as well as the jprobe handler are
+	 * valid function entry points.
+	 */
 	addr = arch_deref_entry_point(jp->entry);
 
-	if (kallsyms_lookup_size_offset(addr, NULL, &offset) && offset == 0) {
+	if (kallsyms_lookup_size_offset(addr, NULL, &offset) && offset == 0 &&
+	    kprobe_on_func_entry(kp->addr, kp->symbol_name, kp->offset)) {
 		kp->pre_handler = setjmp_pre_handler;
 		kp->break_handler = longjmp_break_handler;
 		return register_kprobe(kp);

Signature · 1 line

2.13.2

tip-bot for Naveen N. Rao

merge bot

2017-07-08 11:14 UTC #6 lore raw

[tip:perf/urgent] kprobes: Ensure that jprobe probepoints are at function entry

kprobes: Ensure that jprobe probepoints are at function entry

Commit-ID:  dbf580623d5fee785218d1a47a2bcdf36d85c0e9
Gitweb:     http://git.kernel.org/tip/dbf580623d5fee785218d1a47a2bcdf36d85c0e9
Author:     Naveen N. Rao <[email protected]>
AuthorDate: Fri, 7 Jul 2017 22:37:26 +0530
Committer:  Ingo Molnar <[email protected]>
CommitDate: Sat, 8 Jul 2017 11:05:35 +0200

kprobes: Ensure that jprobe probepoints are at function entry

Similar to commit 90ec5e89e393c ("kretprobes: Ensure probe location is at function entry"), ensure that the jprobe probepoint is at function entry.

Signed-off-by Naveen N. Rao <[email protected]>

Cc Ananth N Mavinakayanahalli <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Masami Hiramatsu <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/a4525af6c5a42df385efa31251246cf7cca73598.1499443367.git.naveen.n.rao@linux.vnet.ibm.com open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/kprobes.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

kernel/kprobes.c +6 -2

index db3cd3e..a1606a4 100644
@@ -1795,10 +1795,14 @@ int register_jprobe(struct jprobe *jp)
 	unsigned long addr, offset;
 	struct kprobe *kp = &jp->kp;
 
-	/* Verify probepoint is a function entry point */
+	/*
+	 * Verify probepoint as well as the jprobe handler are
+	 * valid function entry points.
+	 */
 	addr = arch_deref_entry_point(jp->entry);
 
-	if (kallsyms_lookup_size_offset(addr, NULL, &offset) && offset == 0) {
+	if (kallsyms_lookup_size_offset(addr, NULL, &offset) && offset == 0 &&
+	    kprobe_on_func_entry(kp->addr, kp->symbol_name, kp->offset)) {
 		kp->pre_handler = setjmp_pre_handler;
 		kp->break_handler = longjmp_break_handler;
 		return register_kprobe(kp);

Masami Hiramatsu

maintainer Acked-by on the v2 series

2017-07-09 12:28 UTC #7 lore raw

In reply to Naveen N. Rao

Quoted 6 lines · On Fri, 7 Jul 2017 22:37:23 +0530 "Naveen N. Rao"…

Here is v2 of the patch:
https://www.mail-archive.com/[email protected]/msg1434133.html
As suggested, the first two patches first clean up the existing jprobe registration functions and rename function_offset_within_entry() to kprobe_on_func_entry(). The last patch has been updated accordingly.

This series looks good to me.

Acked-by Masami Hiramatsu <[email protected]>

Thanks!

Quoted 19 lines

Thanks,
Naveen
Naveen N. Rao (3):
kernel/kprobes: Rename [arch_]function_offset_within_entry() to
    [arch_]kprobe_on_func_entry()
kernel/kprobes: Simplify register_jprobes() kernel/kprobes: Ensure that jprobe probepoints are at function entry
 arch/powerpc/kernel/kprobes.c |  2 +-
 include/linux/kprobes.h       |  4 ++--
 kernel/kprobes.c              | 42 ++++++++++++++++++++++++------------------
 kernel/trace/trace_kprobe.c   |  2 +-
 4 files changed, 28 insertions(+), 22 deletions(-)
Signature · 1 line
2.13.2

Signature · 1 line

Masami Hiramatsu <[email protected]>