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

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Arnd Bergmann <arnd@arndb.de>, Stefan Wahren <wahrenst@gmx.net>,
	Kees Cook <keescook@chromium.org>,
	linux-arm-kernel@lists.infradead.org,
	Catalin Marinas <catalin.marinas@arm.com>
Subject: Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
Date: Wed, 15 May 2024 17:18:11 +0100	[thread overview]
Message-ID: <ZkTgQ8EZG3E7o4lR@shell.armlinux.org.uk> (raw)
In-Reply-To: <CAMj1kXEPUKx-6QqfpjvgrL-YFbaz=akh=gEG0QakFJUDo-1C0w@mail.gmail.com>

On Wed, May 15, 2024 at 05:41:45PM +0200, Ard Biesheuvel wrote:
> This definitely looks like some pathological compiler behavior:
> 
> The faulting instruction in question is in busybox
> 
>    15f450:       eef11a10        vmrs    r1, fpscr
> 
> which [as expected] triggers an UNDEF exception as FP is disabled
> after a context switch.
> 
> The get_user() call in do_undefinstr() [arch/arm/kernel/traps.c:482]
> 
>     if (get_user(instr, (u32 __user *)pc))
> 
> gets compiled to the below, where the thing to note is that the
> out-of-line version of uaccess_save_and_enable() returns the old ttbcr
> value in R0. This value gets recorded in R3, but it also gets happily
> passed on to __get_user_4(), which expects the user address in R0, not
> the old value of TTBCR.
> 
> │    0xc040a5e4 <do_undefinstr+228>  bl      0xc0409fe4
> <uaccess_save_and_enable>
> │    0xc040a5e8 <do_undefinstr+232>  mov     r3, r0
> │  > 0xc040a5ec <do_undefinstr+236>  bl      0xc10443a8 <__get_user_4>
> │    0xc040a5f0 <do_undefinstr+240>  mcr     15, 0, r3, cr2, cr0, {2}
> │    0xc040a5f4 <do_undefinstr+244>  isb     sy
> 
> With __always_inline, this part is emitted as
> 
>  5fc:   ee124f50        mrc     15, 0, r4, cr2, cr0, {2}
>  600:   e0033004        and     r3, r3, r4
>  604:   ee023f50        mcr     15, 0, r3, cr2, cr0, {2}
>  608:   f57ff06f        isb     sy
>  60c:   ebfffffe        bl      0 <__get_user_4>
>                         60c: R_ARM_CALL __get_user_4
>  610:   ee024f50        mcr     15, 0, r4, cr2, cr0, {2}
>  614:   f57ff06f        isb     sy
> 
> and so R0 is preserved, and the issue does not happen.
> 
> Not sure how to reduce this to a reproducer that can be used to report
> the issue to the GCC folks, but it is most definitely a compiler
> problem, as far as I can tell.

Well this has come up before...

commit 851140ab0d083c78e5723a8b1cbd258f567a7aff
Author: Masahiro Yamada <yamada.masahiro@socionext.com>
Date:   Wed Oct 2 11:28:02 2019 +0100

    ARM: 8908/1: add __always_inline to functions called from __get_user_check()

I assume it's a wontfix on the GCC side.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

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