lkml.org 
[lkml]   [2000]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH] 2.4.0 i386 watchpoint problems [NEW PATCH]
Date
From
Here is a patch to arch/i386/traps.c and arch/i386/signal.c which does
what you are suggesting, I believe.

I have tested this and it works fine for me. (Though I do also need
the patch which stores dr6 back into current->thread.debugreg[6]. That
is not included here since I submitted it separately and assume it is
uncontentious). All user generated watchpoints are noted, as are ones
triggered from the kernel by system calls overwriting the watched
data.

A couple more points :-

1) I restore the debug control register at the point where a signal is
about to be delivered, rather than at the top of the loop as you
suggested. I think that's safe (and potentially less work if we go
around the loop more than once).

2) Rather than zapping the eip in the watchpoint trap to -1 if the
trap occurs in the kernel, I make it the user eip from the thread.
That makes a debugger point at the right place, i.e. the system
call inside which the watchpoint triggered.

-- Jim

James Cownie <jcownie@etnus.com>
Etnus, LLC. +44 117 9071438
http://www.etnus.com

jcownie@pc2: diff -u signal.c-test7 signal.c
--- signal.c-test7 Mon Sep 25 11:52:35 2000
+++ signal.c Mon Sep 25 11:52:38 2000
@@ -600,6 +600,7 @@

for (;;) {
unsigned long signr;
+ unsigned long thread_dr7;

spin_lock_irq(&current->sigmask_lock);
signr = dequeue_signal(&current->blocked, &info);
@@ -689,6 +690,16 @@
/* NOTREACHED */
}
}
+
+ /* Reenable any watchpoints before delivering the
+ * signal to user space. The processor register will
+ * have been cleared if the watchpoint triggered
+ * inside the kernel.
+ */
+ thread_dr7 = current->thread.debugreg[7];
+ __asm__("movl %0,%%db7"
+ : /* no output */
+ : "r" (thread_dr7));

/* Whee! Actually deliver the signal. */
handle_signal(signr, ka, &info, oldset, regs);
jcownie@pc2: diff -c traps.c-2.4.0-test7 traps.c
*** traps.c-2.4.0-test7 Sat Aug 5 00:15:38 2000
--- traps.c Mon Sep 25 13:42:43 2000
***************
*** 491,507 ****
}
/*
! * Careful - we must not do a lock-kernel until we have checked that the
! * debug fault happened in user mode. Getting debug exceptions while
! * in the kernel has to be handled without locking, to avoid deadlocks..
*
* Being careful here means that we don't have to be as careful in a
* lot of more complicated places (task switching can be a bit lazy
* about restoring all the debug state, and ptrace doesn't have to
* find every occurrence of the TF bit that could be saved away even
! * by user code - and we don't have to be careful about what values
! * can be written to the debug registers because there are no really
! * bad cases).
*/
asmlinkage void do_debug(struct pt_regs * regs, long error_code)
{
--- 491,516 ----
}
/*
! * Our handling of the processor debug registers is non-trivial.
! * We do not clear them on entry and exit from the kernel. Therefore
! * it is possible to get a watchpoint trap here from inside the kernel.
! * However, the code in ./ptrace.c has ensured that the user can
! * only set watchpoints on userspace addresses. Therefore the in-kernel
! * watchpoint trap can only occur in code which is reading/writing
! * from user space. Such code must not hold kernel locks (since it
! * can equally take a page fault), therefore it is safe to call
! * force_sig_info even though that claims and releases locks.
! *
! * Code in ./signal.c ensures that the debug control register
! * is restored before we deliver any signal, and therefore that
! * user code runs with the correct debug control register even though
! * we clear it here.
*
* Being careful here means that we don't have to be as careful in a
* lot of more complicated places (task switching can be a bit lazy
* about restoring all the debug state, and ptrace doesn't have to
* find every occurrence of the TF bit that could be saved away even
! * by user code)
*/
asmlinkage void do_debug(struct pt_regs * regs, long error_code)
{
***************
*** 535,562 ****
goto clear_TF;
}
- /* If this is a kernel mode trap, we need to reset db7 to allow us to continue sanely */
- if ((regs->xcs & 3) == 0)
- goto clear_dr7;
/* Ok, finally something we can handle */
tsk->thread.trap_no = 1;
tsk->thread.error_code = error_code;
info.si_signo = SIGTRAP;
info.si_errno = 0;
info.si_code = TRAP_BRKPT;
! info.si_addr = (void *)regs->eip;
force_sig_info(SIGTRAP, &info, tsk);
- return;
- debug_vm86:
- handle_vm86_trap((struct kernel_vm86_regs *) regs, error_code, 1);
- return;

clear_dr7:
__asm__("movl %0,%%db7"
: /* no output */
: "r" (0));
return;
clear_TF:
--- 544,574 ----
goto clear_TF;
}
/* Ok, finally something we can handle */
tsk->thread.trap_no = 1;
tsk->thread.error_code = error_code;
info.si_signo = SIGTRAP;
info.si_errno = 0;
info.si_code = TRAP_BRKPT;
!
! /* If this is a kernel mode trap, save the user PC on entry to
! * the kernel, that's what the debugger can make sense of.
! */
! info.si_addr = ((regs->xcs & 3) == 0) ? (void *)tsk->thread.eip :
! (void *)regs->eip;
force_sig_info(SIGTRAP, &info, tsk);
+ /* Disable additional traps. They'll be re-enabled when
+ * the signal is delivered.
+ */
clear_dr7:
__asm__("movl %0,%%db7"
: /* no output */
: "r" (0));
+ return;
+
+ debug_vm86:
+ handle_vm86_trap((struct kernel_vm86_regs *) regs, error_code, 1);
return;
clear_TF:
jcownie@pc2:
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 12:38    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site