linux-kernel - Re: [PATCH v4 2/2] powerpc: Uprobes port to powerpc

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]

Message-ID: <20120823090209.GA4630@redhat.com>
Date:	Thu, 23 Aug 2012 11:02:09 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
Cc:	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
	Michael Ellerman <michael@...erman.id.au>, ananth@...ibm.com,
	ppcdev <linuxppc-dev@...ts.ozlabs.org>,
	lkml <linux-kernel@...r.kernel.org>,
	Paul Mackerras <paulus@...ba.org>,
	Anton Blanchard <anton@...ba.org>, peterz@...radead.org,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH v4 2/2] powerpc: Uprobes port to powerpc

On 08/23, Benjamin Herrenschmidt wrote:
>
> On Thu, 2012-08-23 at 11:02 +0530, Srikar Dronamraju wrote:
> > >
> >
> > insn is updated/accessed in the arch independent code. Size of
> > uprobe_opcode_t could be different for different archs.
> > uprobe_opcode_t
> > represents the size of the smallest breakpoint instruction for an
> > arch.
> >
> > Hence u8 works out the best. I know we could still use uprobe_opcode_t
> > and achieve the same. In which case, we would have to interpret
> > MAX_UINSN_BYTES differently. Do you see any advantages of using
> > uprobe_opcode_t instead of u8 across archs?
>
> But don't you actively rely on the fact that on powerpc, unlike x86, you
> -can- atomically replace an instruction with a single 32-bit store ?

I must have missed something...

But powerpc does not replace an instruction, the arch independent code
does this and it assumes that uprobe->arch.insn is u8[MAX_UINSN_BYTES].

Perhaps you meant that on powerpc it is "safe" to replace the insn
even if this can race with some CPU executing this code? But uprobes
has to replace the original page anyway, we should not write to
->vm_file.

I agree that memcpy() in arch_uprobe_analyze_insn() and
arch_uprobe_skip_sstep() looks a bit strange. May be powerpc can do

	struct arch_uprobe {
		union {
			u8		insn[MAX_UINSN_BYTES];
			u32		ainsn;
		};
	};

and use auprobe->ainsn directly, I dunno.

But probably I misunderstood you.

Oleg.

--
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/