Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement

From: Catalin Marinas <catalin.marinas@arm.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>,
	Russell King <linux@armlinux.org.uk>,
	Ard Biesheuvel <ardb@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Stefan Wahren <wahrenst@gmx.net>,
	Kees Cook <keescook@chromium.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
Date: Tue, 14 May 2024 15:39:01 +0100	[thread overview]
Message-ID: <ZkN3hTZ22A_4bl9_@arm.com> (raw)
In-Reply-To: <CACRpkdakWixq5kfW48zUU0gcjVYNTKf+DR9cg9LixrY_SwsS9w@mail.gmail.com>

On Tue, May 14, 2024 at 09:37:21AM +0200, Linus Walleij wrote:
> On Mon, May 13, 2024 at 10:29 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > I guess I should bring out the STM32MP157 board again and retest
> > to verify that that one hardware works with this. Any other ideas?
> 
> I got this Cortex-A7 board out and booted with LPAE and PAN
> using TTBR0 and it boots fine (some kind of OpenEmbedded/YOCTO
> system is in it with systemd and all.
> 
> Tested with LKDTM provoked crashes and it behaves as expected.
> 
> Given Florians report it clearly works on some LPAE:s and not on some
> others for some reason! I suppose we need to figure it out.
> 
> Catalin do you have some ideas?

Since EPD0 is allowed to be cached in the TLB, we need to change the
ASID as well (assuming that it's at least tagged by ASID). But, to keep
the uaccess enable/disable code simple, patch 4 short-circuits this by
toggling the TTBCR.A1 bit. At the time (~2015) this was fine on the
32-bit CPUs. Since the A1 bit is theoretically allowed to be cached in
the TLB, toggling it may not have any effect without a TLB invalidation.
I recall some old documentation stating that the EPD0 value is only
cached when 0, not 1 (IOW, toggling it to 1 would not prevent existing
TLB entries from being hit). This wouldn't have been a significant
issue, most likely the PAN protection not always working but here it's
the other way around, it looks like a value of 1 persisting even after
being toggled.

I'd say the weird behaviour is caused by different microarchitectures,
especially if you run this on ARMv8 hardware (the original patch was
never intended to).

It would be worth trying to do a full TLBI invalidation after uaccess
enable/disable just to check this theory.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel