[PATCH]Kprobes IA64 - Fix race when break hits and kprobe not found

[PATCH]Kprobes IA64 - Fix race when break hits and kprobe not found

[Keshavamurthy Anil S]

This patch addresses a potential race condition for a case where
Kprobe has been removed right after another CPU has taken
a break hit.

The way this is addressed here is when the CPU that has taken a break hit
does not find its corresponding kprobe, then we check to see if the
original instruction got replaced with other than break. If it got
replaced with other than break instruction, then we continue to execute
from the replaced instruction, else if we find that it is still a break, then
we let the kernel handle this, as this might be the break instruction inserted by
other than kprobe(may be kernel debugger).

This patch applies on top of  "Prasanna S Panchamukhi's" recent postings
Kprobes: Prevent possible race condition ia64 changes

Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
=================================== arch/ia64/kernel/kprobes.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+)

Index: linux-2.6.13-rc1-mm1-systemtap/arch/ia64/kernel/kprobes.c
=================================--- linux-2.6.13-rc1-mm1-systemtap.orig/arch/ia64/kernel/kprobes.c
+++ linux-2.6.13-rc1-mm1-systemtap/arch/ia64/kernel/kprobes.c
@@ -554,6 +554,38 @@ static void __kprobes prepare_ss(struct 
 	ia64_psr(regs)->ss = 1;
 }
 
+static int __kprobes is_ia64_break_inst(struct pt_regs *regs)
+{
+	unsigned int slot = ia64_psr(regs)->ri;
+	unsigned int template, major_opcode;
+	unsigned long kprobe_inst;
+	unsigned long *kprobe_addr = (unsigned long *)regs->cr_iip;
+	bundle_t bundle;
+
+	memcpy(&bundle, kprobe_addr, sizeof(bundle_t));
+	template = bundle.quad0.template;
+
+	/* Move to slot 2, if bundle is MLX type and kprobe slot is 1 */
+	if (slot = 1 && bundle_encoding[template][1] = L)
+  		slot++;
+
+	/* Get Kprobe probe instruction at given slot*/
+	get_kprobe_inst(&bundle, slot, &kprobe_inst, &major_opcode);
+
+	/* For break instruction,
+	 * Bits 37:40 Major opcode to be zero
+	 * Bits 27:32 X6 to be zero
+	 * Bits 32:35 X3 to be zero
+	 */
+	if (major_opcode || ((kprobe_inst >> 27) & 0x1FF) ) {
+		/* Not a break instruction */
+		return 0;
+	}
+
+	/* Is a break instruction */
+	return 1;
+}
+
 static int __kprobes pre_kprobes_handler(struct die_args *args)
 {
 	struct kprobe *p;
@@ -601,6 +633,19 @@ static int __kprobes pre_kprobes_handler
 	p = get_kprobe(addr);
 	if (!p) {
 		unlock_kprobes();
+		if (!is_ia64_break_inst(regs)) {
+			/*
+			 * The breakpoint instruction was removed right
+			 * after we hit it.  Another cpu has removed
+			 * either a probepoint or a debugger breakpoint
+			 * at this address.  In either case, no further
+			 * handling of this interrupt is appropriate.
+			 */
+			ret = 1;
+
+		}
+
+		/* Not one of our break, let kernel handle it */
 		goto no_kprobe;
 	}
 

Re: [PATCH]Kprobes IA64 - Fix race when break hits and kprobe not

[Andrew Morton]

Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> wrote:
>
> This patch applies on top of  "Prasanna S Panchamukhi's" recent postings
>  Kprobes: Prevent possible race condition ia64 changes

I am not aware of such a patch.  Your patch hit a reject when I tried to
apply it to Linus's tree.  So I don't know what's going on..

Re: [PATCH]Kprobes IA64 - Fix race when break hits and kprobe not found

[Keshavamurthy Anil S]

On Tue, Jul 12, 2005 at 07:02:31PM -0700, Andrew Morton wrote:
> Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> wrote:
> >
> > This patch applies on top of  "Prasanna S Panchamukhi's" recent postings
> >  Kprobes: Prevent possible race condition ia64 changes
> 
> I am not aware of such a patch.  Your patch hit a reject when I tried to
> apply it to Linus's tree.  So I don't know what's going on..

Andrew,
	I thought you might have applied http://marc.theaimsgroup.com/?l=linux-kernel&m\x112082142008250&w=2 hence I generated my patch on top of this.

No problem, I will generate this patch against Linus's tree (as this does not really depend on any other patches) and submit it again in couple of hours. 
Let me know if this is okay.

-thanks,
-Anil

Re: [PATCH]Kprobes IA64 - Fix race when break hits and kprobe not found

[Keshavamurthy Anil S]

On Tue, Jul 12, 2005 at 07:02:31PM -0700, Andrew Morton wrote:
> Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> wrote:
> >
> > This patch applies on top of  "Prasanna S Panchamukhi's" recent postings
> >  Kprobes: Prevent possible race condition ia64 changes
> 
> I am not aware of such a patch.  Your patch hit a reject when I tried to
> apply it to Linus's tree.  So I don't know what's going on..

Andrew,
	This should apply cleanly ontop of Linus's tree.

----------------------------------------------------------
This patch addresses a potential race condition for a case where
Kprobe has been removed right after another CPU has taken
a break hit.

The way this is addressed here is when the CPU that has taken a break hit
does not find its corresponding kprobe, then we check to see if the
original instruction got replaced with other than break. If it got
replaced with other than break instruction, then we continue to execute
from the replaced instruction, else if we find that it is still a break, then
we let the kernel handle this, as this might be the break instruction inserted by
other than kprobe(may be kernel debugger).

This patch applies on top of  "Prasanna S Panchamukhi's" recent postings
Kprobes: Prevent possible race condition ia64 changes

Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
=================================== arch/ia64/kernel/kprobes.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+)

Index: linux-2.6.13-rc2-mm2/arch/ia64/kernel/kprobes.c
=================================--- linux-2.6.13-rc2-mm2.orig/arch/ia64/kernel/kprobes.c
+++ linux-2.6.13-rc2-mm2/arch/ia64/kernel/kprobes.c
@@ -545,6 +545,38 @@ static void prepare_ss(struct kprobe *p,
 	ia64_psr(regs)->ss = 1;
 }
 
+static int is_ia64_break_inst(struct pt_regs *regs)
+{
+	unsigned int slot = ia64_psr(regs)->ri;
+	unsigned int template, major_opcode;
+	unsigned long kprobe_inst;
+	unsigned long *kprobe_addr = (unsigned long *)regs->cr_iip;
+	bundle_t bundle;
+
+	memcpy(&bundle, kprobe_addr, sizeof(bundle_t));
+	template = bundle.quad0.template;
+
+	/* Move to slot 2, if bundle is MLX type and kprobe slot is 1 */
+ 	if (slot = 1 && bundle_encoding[template][1] = L)
+   		slot++;
+
+	/* Get Kprobe probe instruction at given slot*/
+	get_kprobe_inst(&bundle, slot, &kprobe_inst, &major_opcode);
+
+ 	/* For break instruction,
+ 	 * Bits 37:40 Major opcode to be zero
+	 * Bits 27:32 X6 to be zero
+	 * Bits 32:35 X3 to be zero
+	 */
+	if (major_opcode || ((kprobe_inst >> 27) & 0x1FF) ) {
+		/* Not a break instruction */
+		return 0;
+	}
+
+ 	/* Is a break instruction */
+	return 1;
+}
+
 static int pre_kprobes_handler(struct die_args *args)
 {
 	struct kprobe *p;
@@ -592,6 +624,19 @@ static int pre_kprobes_handler(struct di
 	p = get_kprobe(addr);
 	if (!p) {
 		unlock_kprobes();
+		if (!is_ia64_break_inst(regs)) {
+			/*
+			 * The breakpoint instruction was removed right
+			 * after we hit it.  Another cpu has removed
+			 * either a probepoint or a debugger breakpoint
+			 * at this address.  In either case, no further
+			 * handling of this interrupt is appropriate.
+			 */
+			ret = 1;
+
+		}
+
+		/* Not one of our break, let kernel handle it */
 		goto no_kprobe;
 	}