Received-SPF: pass (google.com: domain of linux-kernel+bounces-212102-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Wed, 12 Jun 2024 19:41:32 +0100
From: Mark Rutland <mark.rutland@arm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Anvin <hpa@zytor.com>, Ingo Molnar <mingo@kernel.org>,
	Borislav Petkov <bp@alien8.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Josh Poimboeuf <jpoimboe@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	the arch/x86 maintainers <x86@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-arch <linux-arch@vger.kernel.org>
Subject: Re: [PATCH 0/7] arm64 / x86-64: low-level code generation issues
Message-ID: <Zmnr3BjBkV4JxsIj@J2N7QTR9R3.cambridge.arm.com>
References: <20240610204821.230388-1-torvalds@linux-foundation.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240610204821.230388-1-torvalds@linux-foundation.org>

On Mon, Jun 10, 2024 at 01:48:14PM -0700, Linus Torvalds wrote:
> The last three patches are purely arm64-specific, and just fix up some
> nasty code generation in the user access functions.  I just noticed that
> I will need to implement 'user_access_save()' for KCSAN now that I do
> the unsafe user access functions. 
> 
> Anyway, that 'user_access_save/restore()' issue only shows up with
> KCSAN.  And it would be a no-op thanks to arm64 doing SMAP the right way
> (pet peeve: arm64 did what I told the x86 designers to do originally,
> but they claimed was too hard, so we ended up with that CLAC/STAC
> instead)... 
> 
> Sadly that "no-op for KCSAN" would is except for the horrid
> CONFIG_ARM64_SW_TTBR0_PAN case, which is why I'm not touching it.  I'm
> hoping some hapless^Whelpful arm64 person is willing to tackle this (or
> maybe make KCSAN and ARM64_SW_TTBR0_PAN incompatible in the Kconfig). 

Given how badly things go when we get this wrong (e.g. TLB corruption), I'd
like to say "just mark it incompatible", applying to all instrumentation, not
just KCSAN.

Any new microarchitecture since ~2014 has real HW PAN, and IIUC it's really
only Cortex-{A53,A57,A72} that don't have it, so I think it'd be fair to say
don't use sanitizers with SW PAN on those CPUs.

Otherwise, I came up with the below (untested). It's a bit horrid because we
could have instrumentation in the middle of __uaccess_ttbr0_{enable,disable}(),
and so we aways need the ISB just in case, and TBH I'm not sure that we use
user_access_{save,restore}() in all the places we should.

Mark.

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 30e2f8fa87a4..83301400ec4c 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -92,6 +92,38 @@ static inline void __uaccess_ttbr0_enable(void)
        local_irq_restore(flags);
 }
 
+static inline unsigned long user_access_ttbr0_save(void)
+{
+       if (!system_uses_ttbr0_pan())
+               return 0;
+
+       /*
+        * If TTBR1 has an ASID other than the reserved ASID, then we have an
+        * active user TTBR0 or are part-way through enabling/disabling TTBR0
+        * access.
+        */
+       if (read_sysreg(ttbr1_el1) & TTBR_ASID_MASK) {
+               __uaccess_ttbr0_disable();
+               return 1;
+       }
+
+       /*
+        * Instrumentation could fire during __uaccess_ttbr0_disable() between
+        * the final write to TTBR1 and before the trailing ISB. We need an ISB
+        * to ensure that we don't continue to use the old ASID.
+        */
+       isb();
+       return 0;
+}
+
+static inline void user_access_ttbr0_restore(unsigned long enabled)
+{
+       if (!system_uses_ttbr0_pan() || !enabled)
+               return;
+
+       __uaccess_ttbr0_enable();
+}
+
 static inline bool uaccess_ttbr0_disable(void)
 {
        if (!system_uses_ttbr0_pan())
@@ -117,8 +149,20 @@ static inline bool uaccess_ttbr0_enable(void)
 {
        return false;
 }
+
+static inline unsigned long user_access_ttbr0_save(void)
+{
+       return 0;
+}
+
+static inline void user_access_ttbr0_restore(unsigned long)
+{
+}
 #endif
 
+#define user_access_save       user_access_ttbr0_save
+#define user_access_restore    user_access_ttbr0_restore
+
 static inline void __uaccess_disable_hw_pan(void)
 {
        asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN,