linux-kernel - Re: [PATCH 3/5] uprobes: Fix UPROBE_SKIP_SSTEP checks in handle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 17 Sep 2012 22:50:55 +0530
From:	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <peterz@...radead.org>,
	Anton Arapov <anton@...hat.com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/5] uprobes: Fix UPROBE_SKIP_SSTEP checks in
 handle_swbp()

* Oleg Nesterov <oleg@...hat.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

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