Messages in this thread Patch in this message | | | Date | Wed, 20 May 2009 10:04:09 -0400 (EDT) | From | Alan Stern <> | Subject | Re: [Patch 1/2] Improvements and minor fixes to HW Breakpoint interface |
| |
On Wed, 20 May 2009, K.Prasad wrote:
> --- linux-2.6-tip.hbkpt.orig/arch/x86/kernel/hw_breakpoint.c > +++ linux-2.6-tip.hbkpt/arch/x86/kernel/hw_breakpoint.c > @@ -296,6 +296,19 @@ void arch_flush_thread_hw_breakpoint(str > > /* > * Handle debug exception notifications. > + * > + * Return value is either NOTIFY_STOP or NOTIFY_DONE as explained below. > + * > + * NOTIFY_DONE returned if one of the following conditions is true. > + * i) When the causative address is from user-space and the exception > + * is a valid one i.e. not triggered a result of lazy debug register --------------------^----^ missing ',' characters ---------------------------------------^ missing "as"
> + * switching > + * ii) When there are more bits than trap<n> set in DR6 register (such > + * as BD, BS or BT) indicating that more than one debug condition is > + * met and requires some more action in do_debug(). > + * > + * NOTIFY_STOP returned for all other cases > + * > */ > int __kprobes hw_breakpoint_handler(struct die_args *args) > { > @@ -346,8 +359,10 @@ int __kprobes hw_breakpoint_handler(stru > * or due to the delay between updates of hbp_kernel_pos > * and this_hbp_kernel. > */ > - if (!bp) > + if (!bp) { > + rc = NOTIFY_STOP; > continue; > + } > > (bp->triggered)(bp, args->regs);
This is the same mistake as before. If rc had previously been set to NOTIFY_DONE then you will unconditionally change it back to NOTIFY_STOP. You should make the code do what the comment says, set rc to NOTIFY_DONE when a non-NULL user breakpoint occurs:
if (i >= hbp_kernel_pos) { bp = per_cpu(this_hbp_kernel[i], cpu); } else { bp = current->thread.hbp[i]; if (bp) rc = NOTIFY_DONE; } /* * bp can be NULL due to lazy debug register switching * or due to the delay between updates of hbp_kernel_pos * and this_hbp_kernel. */ if (!bp) continue;
(bp->triggered)(bp, args->regs);
The rest of the changes are okay.
Alan Stern
| |