Re: [PATCH 5/7 v6] hw-breakpoints: Rewrite the hw-breakpoints layer on top of perf events

["K.Prasad" <prasad@linux.vnet.ibm.com>]

On Wed, Nov 11, 2009 at 06:32:07PM +0530, K.Prasad wrote:
> On Sun, Nov 08, 2009 at 04:28:59PM +0100, Frederic Weisbecker wrote:
> 
> There were a few comments that I posted against version 6 of your
> patchset (which happened to cross your version 7 posting...) regarding
> the breakpoint interfaces, reservation of register for unpinned events
> and such...
> 
> By the way, I'm looking at refs/heads/perfevents/hw-breakpoint branch in
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> and hope that's correct/latest?
> 
> Some more comments about the ptrace implementation here...
> 
>

I forgot to mention another potential bug here...

static int ptrace_write_dr7(struct task_struct *tsk, unsigned long data)
{
..
...
restore:
	/*
	 * Loop through all the hardware breakpoints, making the
	 * appropriate changes to each.
	 */
	for (i = 0; i < HBP_NUM; i++) {
		enabled = decode_dr7(data, i, &len, &type);
		bp = thread->ptrace_bps[i];

		if (!enabled) {
			if (bp) {
				/*
				 * Don't unregister the breakpoints right-away,
				 * unless all register_user_hw_breakpoint()
				 * requests have succeeded. This prevents
				 * any window of opportunity for debug
				 * register grabbing by other users.
				 */
				if (!second_pass)
					continue;
				thread->ptrace_bps[i] = NULL;
				unregister_hw_breakpoint(bp);
			}
			continue;
		}

So, the breakpoint is unregistered whenever bits corresponding to
DR0-DR3 are set to a disabled state in DR7.

		/*
		 * We shoud have at least an inactive breakpoint at this
		 * slot. It means the user is writing dr7 without having
		 * written the address register first
		 */
		if (!bp) {
			rc = -EINVAL;
			break;
		}
..
...
}

Now think of the following sequence of write operations through ptrace:
1. Populate address in DRn (where 0 <= n <= 3) (breakpoint registration)
2. Enable corresponding bits in DR7 (modify breakpoint to active state)
3. Disable bits in DR7 (unregister breakpoint)
4. Enable bits in DR7 (returns with failure)

The assumption that every 'enable' operation in DR7 is preceded by a
write operation on DR0-DR3 need not be always true.

Thanks,
K.Prasad