Re: [PATCH 3/5] uprobes: Fix UPROBE_SKIP_SSTEP checks in handle_swbp()

[Srikar Dronamraju <srikar@linux.vnet.ibm.com>]

* Oleg Nesterov <oleg@redhat.com> [2012-09-15 17:01:20]:

> On 09/15, Ananth N Mavinakayanahalli wrote:
> >
> > On Fri, Sep 14, 2012 at 07:15:57PM +0200, Oleg Nesterov wrote:
> > >
> > > Note: probably we should rename "skip" to "emulate" and I think
> > > that "clear UPROBE_SKIP_SSTEP" should be moved to arch_can_skip.
> >
> > Agree. emulate is more accurate in this situation since, especially on
> > powerpc, we do emulate most instructions.
> 
> Yes. And even on x86, perhaps we should emulate at least pushf to
> not expose TF set by uprobes.
> 

Good idea. 

> Off-topic question... I am trying to understand if arch_uprobe_skip_sstep()
> is correct on x86.
> 
> It doesn't update regs->ip. 

Right. we need to adjust for the size of the instruction.

> Probably this is fine, at least this is
> fine if it finds "nop" eventually. But I can't undestand what
> "0x66* { 0x90 | 0x0f 0x1f | 0x0f 0x19 | 0x87 0xc0 }" means.
> OK, 0x66 and 0x90 are clear. But, say, 0x0f 0x1f ?

we skip is 0x66 ..0x66 0x0f 0x1f

So we have a check
if (i == (MAX_UINSN_BYTES - 1)) 

so this ensures that we are consider 0x0f 0x1f as nop if and only if
they are at the end and preceeded by 0x66. This is not an exhaustive
list of nops.

So are you suggesting extending the list of nops or is it that we are
considering non nop instructions as nops?

Extending the list, we certainly should not just for nops.

> 
> I compiled this program
> 
> 	int main(void)
> 	{
> 		asm volatile (".word 0x1f0f");
> 		return 0;
> 	}
> 
> and objdump reports:
> 
> 	000000000040047c <main>:
> 	  40047c:       0f 1f 31                nopl   (%rcx)

Current uprobes code wouldnt skip the above insn because it has 31
following it.

> 	  40047f:       c0 c3 90                rol    $0x90,%bl

we dont skip this too.

-- 
Thanks and Regards
Srikar