[PATCH] powerpc: kprobes: invoke handlers directly

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE3545d

4 messages, 4 authors, 2016-11-21 · open the first message on its own page

[PATCH] powerpc: kprobes: invoke handlers directly

From: Naveen N. Rao <hidden>
Date: 2016-11-18 11:39:57

... rather than through notify_die(), to reduce path taken for handling
kprobes. Similar to commit 6f6343f53d13 ("kprobes/x86: Call exception
handlers directly from do_int3/do_debug").

While at it, rename post_kprobe_handler() to kprobe_post_handler() for
more uniform naming.

Reported-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/include/asm/kprobes.h |  2 ++
 arch/powerpc/kernel/kprobes.c      | 29 +++++++----------------------
 arch/powerpc/kernel/traps.c        | 20 ++++++++++++++++++++
 3 files changed, 29 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/kprobes.h b/arch/powerpc/include/asm/kprobes.h
index 2c9759bd..47c60d0 100644
--- a/arch/powerpc/include/asm/kprobes.h
+++ b/arch/powerpc/include/asm/kprobes.h
@@ -126,6 +126,8 @@ struct kprobe_ctlblk {
 
 extern int kprobe_exceptions_notify(struct notifier_block *self,
 					unsigned long val, void *data);
+extern int kprobe_handler(struct pt_regs *regs);
+extern int kprobe_post_handler(struct pt_regs *regs);
 extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
 #endif /* __KERNEL__ */
 #endif	/* _ASM_POWERPC_KPROBES_H */
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 9479d8e..ad108b8 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -140,13 +140,16 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
 	regs->link = (unsigned long)kretprobe_trampoline;
 }
 
-static int __kprobes kprobe_handler(struct pt_regs *regs)
+int __kprobes kprobe_handler(struct pt_regs *regs)
 {
 	struct kprobe *p;
 	int ret = 0;
 	unsigned int *addr = (unsigned int *)regs->nip;
 	struct kprobe_ctlblk *kcb;
 
+	if (user_mode(regs))
+		return 0;
+
 	/*
 	 * We don't want to be preempted for the entire
 	 * duration of kprobe processing
@@ -359,12 +362,12 @@ static int __kprobes trampoline_probe_handler(struct kprobe *p,
  * single-stepped a copy of the instruction.  The address of this
  * copy is p->ainsn.insn.
  */
-static int __kprobes post_kprobe_handler(struct pt_regs *regs)
+int __kprobes kprobe_post_handler(struct pt_regs *regs)
 {
 	struct kprobe *cur = kprobe_running();
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
-	if (!cur)
+	if (!cur || user_mode(regs))
 		return 0;
 
 	/* make sure we got here for instruction we have a kprobe on */
@@ -470,25 +473,7 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
 				       unsigned long val, void *data)
 {
-	struct die_args *args = (struct die_args *)data;
-	int ret = NOTIFY_DONE;
-
-	if (args->regs && user_mode(args->regs))
-		return ret;
-
-	switch (val) {
-	case DIE_BPT:
-		if (kprobe_handler(args->regs))
-			ret = NOTIFY_STOP;
-		break;
-	case DIE_SSTEP:
-		if (post_kprobe_handler(args->regs))
-			ret = NOTIFY_STOP;
-		break;
-	default:
-		break;
-	}
-	return ret;
+	return NOTIFY_DONE;
 }
 
 unsigned long arch_deref_entry_point(void *entry)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 91d278c..dbcdf48 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -824,6 +824,11 @@ void single_step_exception(struct pt_regs *regs)
 
 	clear_single_step(regs);
 
+#ifdef CONFIG_KPROBES
+	if (kprobe_post_handler(regs))
+		return;
+#endif
+
 	if (notify_die(DIE_SSTEP, "single_step", regs, 5,
 					5, SIGTRAP) == NOTIFY_STOP)
 		goto bail;
@@ -1177,6 +1182,11 @@ void program_check_exception(struct pt_regs *regs)
 		if (debugger_bpt(regs))
 			goto bail;
 
+#ifdef CONFIG_KPROBES
+		if (kprobe_handler(regs))
+			goto bail;
+#endif
+
 		/* trap exception */
 		if (notify_die(DIE_BPT, "breakpoint", regs, 5, 5, SIGTRAP)
 				== NOTIFY_STOP)
@@ -1745,6 +1755,11 @@ void DebugException(struct pt_regs *regs, unsigned long debug_status)
 			return;
 		}
 
+#ifdef CONFIG_KPROBES
+		if (kprobe_post_handler(regs))
+			return;
+#endif
+
 		if (notify_die(DIE_SSTEP, "block_step", regs, 5,
 			       5, SIGTRAP) == NOTIFY_STOP) {
 			return;
@@ -1759,6 +1774,11 @@ void DebugException(struct pt_regs *regs, unsigned long debug_status)
 		/* Clear the instruction completion event */
 		mtspr(SPRN_DBSR, DBSR_IC);
 
+#ifdef CONFIG_KPROBES
+		if (kprobe_post_handler(regs))
+			return;
+#endif
+
 		if (notify_die(DIE_SSTEP, "single_step", regs, 5,
 			       5, SIGTRAP) == NOTIFY_STOP) {
 			return;
-- 
2.10.2

Re: [PATCH] powerpc: kprobes: invoke handlers directly

From: Masami Hiramatsu <mhiramat@kernel.org>
Date: 2016-11-18 14:48:49

On Fri, 18 Nov 2016 17:09:26 +0530
"Naveen N. Rao" [off-list ref] wrote:
... rather than through notify_die(), to reduce path taken for handling
kprobes. Similar to commit 6f6343f53d13 ("kprobes/x86: Call exception
handlers directly from do_int3/do_debug").

While at it, rename post_kprobe_handler() to kprobe_post_handler() for
more uniform naming.
Looks good to me.

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!
quoted hunk
Reported-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/include/asm/kprobes.h |  2 ++
 arch/powerpc/kernel/kprobes.c      | 29 +++++++----------------------
 arch/powerpc/kernel/traps.c        | 20 ++++++++++++++++++++
 3 files changed, 29 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/kprobes.h b/arch/powerpc/include/asm/kprobes.h
index 2c9759bd..47c60d0 100644
--- a/arch/powerpc/include/asm/kprobes.h
+++ b/arch/powerpc/include/asm/kprobes.h
@@ -126,6 +126,8 @@ struct kprobe_ctlblk {
 
 extern int kprobe_exceptions_notify(struct notifier_block *self,
 					unsigned long val, void *data);
+extern int kprobe_handler(struct pt_regs *regs);
+extern int kprobe_post_handler(struct pt_regs *regs);
 extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
 #endif /* __KERNEL__ */
 #endif	/* _ASM_POWERPC_KPROBES_H */
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 9479d8e..ad108b8 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -140,13 +140,16 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
 	regs->link = (unsigned long)kretprobe_trampoline;
 }
 
-static int __kprobes kprobe_handler(struct pt_regs *regs)
+int __kprobes kprobe_handler(struct pt_regs *regs)
 {
 	struct kprobe *p;
 	int ret = 0;
 	unsigned int *addr = (unsigned int *)regs->nip;
 	struct kprobe_ctlblk *kcb;
 
+	if (user_mode(regs))
+		return 0;
+
 	/*
 	 * We don't want to be preempted for the entire
 	 * duration of kprobe processing
@@ -359,12 +362,12 @@ static int __kprobes trampoline_probe_handler(struct kprobe *p,
  * single-stepped a copy of the instruction.  The address of this
  * copy is p->ainsn.insn.
  */
-static int __kprobes post_kprobe_handler(struct pt_regs *regs)
+int __kprobes kprobe_post_handler(struct pt_regs *regs)
 {
 	struct kprobe *cur = kprobe_running();
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
-	if (!cur)
+	if (!cur || user_mode(regs))
 		return 0;
 
 	/* make sure we got here for instruction we have a kprobe on */
@@ -470,25 +473,7 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
 				       unsigned long val, void *data)
 {
-	struct die_args *args = (struct die_args *)data;
-	int ret = NOTIFY_DONE;
-
-	if (args->regs && user_mode(args->regs))
-		return ret;
-
-	switch (val) {
-	case DIE_BPT:
-		if (kprobe_handler(args->regs))
-			ret = NOTIFY_STOP;
-		break;
-	case DIE_SSTEP:
-		if (post_kprobe_handler(args->regs))
-			ret = NOTIFY_STOP;
-		break;
-	default:
-		break;
-	}
-	return ret;
+	return NOTIFY_DONE;
 }
 
 unsigned long arch_deref_entry_point(void *entry)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 91d278c..dbcdf48 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -824,6 +824,11 @@ void single_step_exception(struct pt_regs *regs)
 
 	clear_single_step(regs);
 
+#ifdef CONFIG_KPROBES
+	if (kprobe_post_handler(regs))
+		return;
+#endif
+
 	if (notify_die(DIE_SSTEP, "single_step", regs, 5,
 					5, SIGTRAP) == NOTIFY_STOP)
 		goto bail;
@@ -1177,6 +1182,11 @@ void program_check_exception(struct pt_regs *regs)
 		if (debugger_bpt(regs))
 			goto bail;
 
+#ifdef CONFIG_KPROBES
+		if (kprobe_handler(regs))
+			goto bail;
+#endif
+
 		/* trap exception */
 		if (notify_die(DIE_BPT, "breakpoint", regs, 5, 5, SIGTRAP)
 				== NOTIFY_STOP)
@@ -1745,6 +1755,11 @@ void DebugException(struct pt_regs *regs, unsigned long debug_status)
 			return;
 		}
 
+#ifdef CONFIG_KPROBES
+		if (kprobe_post_handler(regs))
+			return;
+#endif
+
 		if (notify_die(DIE_SSTEP, "block_step", regs, 5,
 			       5, SIGTRAP) == NOTIFY_STOP) {
 			return;
@@ -1759,6 +1774,11 @@ void DebugException(struct pt_regs *regs, unsigned long debug_status)
 		/* Clear the instruction completion event */
 		mtspr(SPRN_DBSR, DBSR_IC);
 
+#ifdef CONFIG_KPROBES
+		if (kprobe_post_handler(regs))
+			return;
+#endif
+
 		if (notify_die(DIE_SSTEP, "single_step", regs, 5,
 			       5, SIGTRAP) == NOTIFY_STOP) {
 			return;
-- 
2.10.2

-- 
Masami Hiramatsu [off-list ref]

Re: [PATCH] powerpc: kprobes: invoke handlers directly

From: Ananth N Mavinakayanahalli <hidden>
Date: 2016-11-21 04:23:58

On Fri, Nov 18, 2016 at 05:09:26PM +0530, Naveen N. Rao wrote:
... rather than through notify_die(), to reduce path taken for handling
kprobes. Similar to commit 6f6343f53d13 ("kprobes/x86: Call exception
handlers directly from do_int3/do_debug").

While at it, rename post_kprobe_handler() to kprobe_post_handler() for
more uniform naming.

Reported-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Naveen N. Rao <redacted>
Acked-by: Ananth N Mavinakayanahalli <redacted>

Re: [PATCH] powerpc: kprobes: invoke handlers directly

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-11-21 10:24:26

"Naveen N. Rao" [off-list ref] writes:
+#ifdef CONFIG_KPROBES
+	if (kprobe_post_handler(regs))
+		return;
+#endif
...
+#ifdef CONFIG_KPROBES
+		if (kprobe_handler(regs))
+			goto bail;
+#endif
...
+#ifdef CONFIG_KPROBES
+		if (kprobe_post_handler(regs))
+			return;
+#endif
...
+#ifdef CONFIG_KPROBES
+		if (kprobe_post_handler(regs))
+			return;
+#endif
+
Sorry too many ifdefs.

Can you please add empty static inline versions for CONFIG_KPROBES=n
that always return false. The compiler will be smart enough to elide
them entirely.

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help