Re: [patch] kprobes: dont steal interrupts from vm86

[Stas Sergeev <stsp@aknet.ru>]

[-- Attachment #1: Type: text/plain, Size: 1519 bytes --]

Hello.

Prasanna S Panchamukhi wrote:
> Yes, there is a small bug in kprobes. Kprobes int3 handler
> was returning wrong value. Please check out if the patch
> attached with this mail fixes your problem.
> Please let me know if you have any issues.
Yes. After several days of debugging,
I am pointing to this problem again.
Unfortunately your patch appeared not
to work. It only masks the problem.
I was surprised that you check VM_MASK
after you already used "addr" a couple
of times - this "addr" is completely
bogus and should not be used. Now this
turned out more important. The problem
is that the "addr" calculated only from
the value of EIP, is bogus not only when
VM flag is set. It is also bogus if the
program uses segmentation and the
CS_base!=0. I have many of the like
programs here and they all are broken
because kprobes still steal the int3 from
them. They do not use V86, but they use
segments instead of the flat layout, so
the address cannot be calculated by the
EIP value.
I would suggest something like the attached
patch. I know nothing about kprobes (sorry)
so I don't know what CS you need. If you
need not only __KERNEL_CS, you probably
want the (regs->xcs & 4) check to see if
the CS is not from LDT at least. Does this
make sense?
Anyway, would be nice to get this fixed.
This can cause Oopses because you deref
the completely bogus pointer later in the
code.
Writing a test-case for this problem is
not a several-minutes work, but if you
really need one, I may try to hack it out.

Thanks.


[-- Attachment #2: kprb.diff --]
[-- Type: text/x-patch, Size: 684 bytes --]

--- linux/arch/i386/kernel/kprobes.c.old	2004-11-18 16:22:46.000000000 +0300
+++ linux/arch/i386/kernel/kprobes.c	2004-12-02 22:01:05.000000000 +0300
@@ -92,6 +92,11 @@
 	int ret = 0;
 	u8 *addr = (u8 *) (regs->eip - 1);
 
+	/* If we are in v86 mode or CS is not ours, get out */
+	if ((regs->eflags & VM_MASK) || regs->xcs != __KERNEL_CS) {
+		return 0;
+	}
+
 	/* We're in an interrupt, but this is clear and BUG()-safe. */
 	preempt_disable();
 
@@ -117,10 +122,6 @@
 	p = get_kprobe(addr);
 	if (!p) {
 		unlock_kprobes();
-		if (regs->eflags & VM_MASK) {
-			/* We are in virtual-8086 mode. Return 0 */
-			goto no_kprobe;
-		}
 
 		if (*addr != BREAKPOINT_INSTRUCTION) {
 			/*