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

[Ard Biesheuvel]

On Wed, 15 May 2024 at 18:18, Russell King (Oracle)
<linux at armlinux.org.uk> wrote:
>
> 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 at 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.
>

Yes, that is the exact same issue.

Not sure whether it has been reported - the GCC side might not even be aware.

I managed to reproduce this in godbolt - it happens even with -O2 not
just with -Os

https://godbolt.org/z/do56voKsa

As far as I can tell, functions that use asm("r#") cannot safely call
other functions at all unless those are __always_inline. The fact that
it triggers with -Os first is just because it inlines much less
aggressively than other optimization levels.