Re: [Patch 0/5] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XXII

[Paul Mackerras <paulus@samba.org>]

On Mon, Jun 07, 2010 at 12:33:51PM +0530, K.Prasad wrote:

> Given that 'ptrace_bps' is used only for ptrace originated breakpoints
> and that we return early i.e. before detecting extraneous interrupts
> in hw_breakpoint_handler() (as shown above) they shouldn't overlap each
> other. The following comment in hw_breakpoint_handler() explains the
> same.
> 		/*
> 		 * To prevent invocation of perf_event_bp(), we shall overload
> 		 * thread.ptrace_bps[] pointer (unused for non-ptrace
> 		 * exceptions) to flag an extraneous interrupt which must be
> 		 * skipped.
> 		 */

My point is that while we are using ptrace_bps[0] to mark a non-ptrace
breakpoint that we're single-stepping, some other process could be
ptracing this process and could get into ptrace_set_debugreg() and
would think that the process already has a ptrace breakpoint and call
modify_user_hw_breakpoint() when it should be calling
register_user_hw_breakpoint().  Or this process could die and so we
call flush_ptrace_hw_breakpoint() and it incorrectly thinks we have a
ptrace breakpoint.

If there is a reason why we can be quite sure that while we are using
current->thread.ptrace_bps[0] in this way, ptrace_set_debugreg() can
never get called with this task as the ptracee, and nor can
flush_ptrace_hw_breakpoint() get called on this task, then maybe it's
safe.  But it's not at all obviously safe.  So I'd very much rather we
just use an extra flag somewhere, that isn't used elsewhere for
anything else, so we can convince ourselves that it is all correct
without having to look at lots of different pieces of code.

There are 3 bytes of padding in struct arch_hw_breakpoint; couldn't we
use one of them as a "not really hit" flag?

Paul.