[PATCH] ppc64: kprobes breaks BUG() handling

[PATCH] ppc64: kprobes breaks BUG() handling

[Anton Blanchard]

Hi,

I was running some tests and noticed BUG() handling wasnt working as
expected. The kprobes code has some code to check for breakpoint
removal races and only checks for one opcode. It turns out there are
many forms of the breakpoint instruction, comparing against one is not
good enough.

For the momemt remove the code in question so BUG()s work again and we
can discuss a better solution (I thought kprobes was emulating
instructions or running them out of line).

Anton

Signed-off-by: Anton Blanchard <anton@samba.org>

diff -puN arch/ppc64/kernel/kprobes.c~fix_breakage arch/ppc64/kernel/kprobes.c
--- foobar2/arch/ppc64/kernel/kprobes.c~fix_breakage	2005-01-10 21:41:43.157624895 +1100
+++ foobar2-anton/arch/ppc64/kernel/kprobes.c	2005-01-10 21:42:04.560169990 +1100
@@ -99,6 +99,7 @@ static inline int kprobe_handler(struct 
 	p = get_kprobe(addr);
 	if (!p) {
 		unlock_kprobes();
+#if 0
 		if (*addr != BREAKPOINT_INSTRUCTION) {
 			/*
 			 * The breakpoint instruction was removed right
@@ -109,6 +110,7 @@ static inline int kprobe_handler(struct 
 			 */
 			ret = 1;
 		}
+#endif
 		/* Not one of ours: let kernel handle it */
 		goto no_kprobe;
 	}
_

Re: [PATCH] ppc64: kprobes breaks BUG() handling

[Ananth N Mavinakayanahalli]

On Mon, Jan 10, 2005 at 10:19:37PM +1100, Anton Blanchard wrote:
> 
> Hi,
> 
> I was running some tests and noticed BUG() handling wasnt working as
> expected. The kprobes code has some code to check for breakpoint
> removal races and only checks for one opcode. It turns out there are
> many forms of the breakpoint instruction, comparing against one is not
> good enough.

Ah yes! I see that BUG() uses twi. I will try and send out a patch for
this soon.

Thanks,
Ananth

Re: [PATCH] ppc64: kprobes breaks BUG() handling

[Ananth N Mavinakayanahalli]

On Mon, Jan 10, 2005 at 10:19:37PM +1100, Anton Blanchard wrote:

Hi Anton,
 
> I was running some tests and noticed BUG() handling wasnt working as
> expected. The kprobes code has some code to check for breakpoint
> removal races and only checks for one opcode. It turns out there are
> many forms of the breakpoint instruction, comparing against one is not
> good enough.

Here is a patch containing checks for the other trap variants (twi, td,
tdi). Logic is similar to the one used in IS_MTMSRD() and IS_RFID(). 

Please let me know if this fixes the issue.

Thanks,
Ananth


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

diff -Naurp temp/linux-2.6.11-rc1/arch/ppc64/kernel/kprobes.c linux-2.6.11-rc1/arch/ppc64/kernel/kprobes.c
--- temp/linux-2.6.11-rc1/arch/ppc64/kernel/kprobes.c	2005-01-12 09:32:08.000000000 +0530
+++ linux-2.6.11-rc1/arch/ppc64/kernel/kprobes.c	2005-01-12 15:37:24.165968408 +0530
@@ -99,8 +99,16 @@ static inline int kprobe_handler(struct 
 	p = get_kprobe(addr);
 	if (!p) {
 		unlock_kprobes();
-#if 0
 		if (*addr != BREAKPOINT_INSTRUCTION) {
+			/* 
+			 * PPC64 has multiple variants of the "trap"
+			 * instruction. If the current instruction is a
+			 * trap variant, it could belong to someone else
+			 */
+			kprobe_opcode_t cur_insn = *addr;
+			if (IS_TWI(cur_insn) || IS_TD(cur_insn) || 
+					IS_TDI(cur_insn))
+		       		goto no_kprobe;
 			/*
 			 * The breakpoint instruction was removed right
 			 * after we hit it.  Another cpu has removed
@@ -110,7 +118,6 @@ static inline int kprobe_handler(struct 
 			 */
 			ret = 1;
 		}
-#endif
 		/* Not one of ours: let kernel handle it */
 		goto no_kprobe;
 	}
diff -Naurp temp/linux-2.6.11-rc1/include/asm-ppc64/kprobes.h linux-2.6.11-rc1/include/asm-ppc64/kprobes.h
--- temp/linux-2.6.11-rc1/include/asm-ppc64/kprobes.h	2005-01-12 09:30:44.000000000 +0530
+++ linux-2.6.11-rc1/include/asm-ppc64/kprobes.h	2005-01-12 15:39:24.100977920 +0530
@@ -35,6 +35,10 @@ typedef unsigned int kprobe_opcode_t;
 #define BREAKPOINT_INSTRUCTION	0x7fe00008	/* trap */
 #define MAX_INSN_SIZE 1
 
+#define IS_TD(instr)		(((instr) & 0xfc0007fe) == 0x7c000088)
+#define IS_TDI(instr)		(((instr) & 0xfc000000) == 0x08000000)
+#define IS_TWI(instr)		(((instr) & 0xfc000000) == 0x0c000000)
+
 #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)((func_descr_t *)pentry)
 
 /* Architecture specific copy of original instruction */