linux-kernel - Re: [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4)

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:   Wed, 12 Jul 2023 16:23:51 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Christophe Leroy <christophe.leroy@...roup.eu>
Cc:     Michael Ellerman <mpe@...erman.id.au>,
        Nicholas Piggin <npiggin@...il.com>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        Sathvika Vasireddy <sv@...ux.ibm.com>,
        Naveen N Rao <naveen@...nel.org>,
        linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32
 (v4)

On Tue, Jul 11, 2023 at 06:08:26PM +0200, Christophe Leroy wrote:
> This series adds UACCESS validation for PPC32. It includes
> a dozen of changes to objtool core.
> 
> It applies on top of series "Cleanup/Optimise KUAP (v3)"
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=363368&state=*

That contains:

+static __always_inline void uaccess_begin_32s(unsigned long addr)
+{
+	unsigned long tmp;
+
+	asm volatile(ASM_MMU_FTR_IFSET(
+		"mfsrin %0, %1;"
+		"rlwinm %0, %0, 0, %2;"
+		"mtsrin %0, %1;"
+		"isync", "", %3)
+		: "=&r"(tmp)
+		: "r"(addr), "i"(~SR_KS), "i"(MMU_FTR_KUAP)
+		: "memory");
+}
+
+static __always_inline void uaccess_end_32s(unsigned long addr)
+{
+	unsigned long tmp;
+
+	asm volatile(ASM_MMU_FTR_IFSET(
+		"mfsrin %0, %1;"
+		"oris %0, %0, %2;"
+		"mtsrin %0, %1;"
+		"isync", "", %3)
+		: "=&r"(tmp)
+		: "r"(addr), "i"(SR_KS >> 16), "i"(MMU_FTR_KUAP)
+		: "memory");
+}

And I am a bit puzzled by the isync placement of uaccess_end, should
that not start with the isync, to ensure completion of the uaccess
region before disabling it?

Or is that not the purpose of the isync?

> It is almost mature, performs code analysis for all PPC32.
> 
> In this version objtool switch table lookup has been enhanced to
> handle nested switch tables.
> 
> Most object files are correctly decoded, only a few
> 'unreachable instruction' warnings remain due to more complex
> fonctions which include back and forth jumps or branches.
> 
> It allowed to detect some UACCESS mess in a few files. They've been
> fixed through other patches.
> 
> Changes in v4:
> - Split series in two parts, the powerpc uaccess rework is submitted
> separately, see https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=363368&state=*
> - Support of UACCESS on all PPC32 including book3s/32 which was missing in v3.
> - More elaborated switch tables lookup.
> - Patches 2, 7, 8, 9, 10, 11 are new
> - Patch 11 in series v3 is now removed.

The patches look eminently reasonable to me; Josh, could you please have
a look?