Re: [PATCH v3 0/10] Uprobes v3

[Ananth N Mavinakayanahalli]

On Wed, May 12, 2010 at 02:13:05PM +0200, Peter Zijlstra wrote:
> On Wed, 2010-05-12 at 15:55 +0530, Srikar Dronamraju wrote:
> > * Peter Zijlstra <peterz@xxxxxxxxxxxxx> [2010-05-11 22:59:45]:
> > 
> > > On Thu, 2010-05-06 at 23:31 +0530, Srikar Dronamraju wrote:
> > > >   - Addressed comments from Oleg, including removal of interrupt context
> > > >     handlers, reverting background page replacement in favour of
> > > >     access_process_vm(). 
> > > 
> > > 
> > > > +static int write_opcode(struct task_struct *tsk, unsigned long vaddr,
> > > > +                                               user_bkpt_opcode_t opcode)
> > > > +{
> > > > +       int ret;
> > > > +
> > > > +       if (!tsk)
> > > > +               return -EINVAL;
> > > > +
> > > > +       ret = access_process_vm(tsk, vaddr, &opcode, user_bkpt_opcode_sz, 1);
> > > > +       return (ret == user_bkpt_opcode_sz ? 0 : -EFAULT);
> > > > +}
> > > 
> > > Why!
> > > 
> > > That's not not the atomic sequence outlined.
> > > 
> > 
> > 
> > Yes, we had moved away from access_process_vm to background page
> > replacement in Version 1 and Version 2. 
> > 
> > One of the reasons being Mathieu suggesting to Jim in LFCS that 
> > for almost all architectures insertion of a breakpoint instruction on a
> > user page is an atomic operation, as far as the CPU is concerned.
> 
> That is true, however when restoring the old instruction you do need to
> take care, so your usage from set_orig_insn() is dubious.
> 
> > Can you and other VM experts tell me if access_process_vm isnt going to
> > be atomic with respect to inserting/deleting a breakpoint instruction? 
> 
> Well, clearly not, since it simply does a gup(.force=1), if whatever
> page is there is writable it will simply poke at it.
> 
> Now writing the INT3 is special, but restoring the old insn is not and
> might confuse another CPU that is currently trying to execute code near
> there.

Yes, this helps for breakpoint insertion, but...

The question is whether only INT3 special or single-byte changes are
also guaranteed to be atomic. In http://lkml.org/lkml/2010/1/27/275
Peter Anvin states 'specific case of a more generic rule'.

For restoring the old instruction, we technically need to put back just
one byte, irrespective of the actual length of the underlying
instruction. Now, as long as we have the housekeeping code to handle the
possibility of a thread hitting the said breakpoint when its being
removed, is it safe to assume atomicity for replacing one byte of
possibly a longer instruction?

Ananth
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/