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

[Ard Biesheuvel]

On Tue, 14 May 2024 at 22:34, Florian Fainelli <f.fainelli at gmail.com> wrote:
>
> On 5/14/24 13:33, Linus Walleij wrote:
> > On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli at gmail.com> wrote:
> >> On 5/14/24 10:03, Russell King (Oracle) wrote:
> >
> >>> I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> >>> a "memory" clobber to the asm() instruction in there.
> >>>
> >>
> >> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
> >>
> >> diff --git a/arch/arm/include/asm/proc-fns.h
> >> b/arch/arm/include/asm/proc-fns.h
> >> index 9b3105a2a5e0..1087bd2af433 100644
> >> --- a/arch/arm/include/asm/proc-fns.h
> >> +++ b/arch/arm/include/asm/proc-fns.h
> >> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
> >>
> >>    static inline void cpu_set_ttbcr(unsigned int ttbcr)
> >>    {
> >> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> >> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
> >>    }
> >>
> >>    #else  /*!CONFIG_MMU */
> >>
> >> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
> >>
> >> Thanks a lot Russell!
> >
> > Second that, very nicely pinpointed Russell!
> >
> > Florian, do you want to send a patch or should I?
>
> I was wondering if Russell was able to fold this directly into patch #2
> where cpu_set_ttbr() is added, so as to not break functionality across
> bisection.

Sadly, I can still reproduce this with the above fix.

I included TTBCR in the DEBUG_USER output, and (as expected), it has
A1, EPD0 and T0SZ set to the 'uaccess disabled' values.

Using __always_inline on uaccess_save_and_enable() and
uaccess_restore() (as the CONFIG_CPU_SW_DOMAIN_PAN does) seems to work
around it. The "memory" clobber seems unnecessary in my case, but it
is needed for correctness in any case.

It is unclear to me why placing these helpers out of line should make
any difference, and I am not convinced it is a problem in the code
(IIRC we've had other issues with -Os in the past)






Run /init as init process
init (1): undefined instruction: pc=0015f450 ttbcr=b5403587
CPU: 0 PID: 1 Comm: init Not tainted 6.9.0-rc1-00037-gb7e329ac0464-dirty #8
Hardware name: Generic DT based system
PC is at 0x15f450
LR is at 0x1548b0
pc : [<0015f450>]    lr : [<001548b0>]    psr: 20000010
sp : bec97da0  ip : bec97ee8  fp : 00000000
r10: 00000000  r9 : 00000000  r8 : 00000000
r7 : 00000000  r6 : bec97f0c  r5 : 00000000  r4 : 00000040
r3 : 00000000  r2 : 00000040  r1 : 00000040  r0 : 00000000
Flags: nzCv  IRQs on  FIQs on  Mode USER_32  ISA ARM  Segment user
Control: 30c5383d  Table: 427122c0  DAC: fffffffd
Code: bad PC value
Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004
CPU: 0 PID: 1 Comm: init Not tainted 6.9.0-rc1-00037-gb7e329ac0464-dirty #8
Hardware name: Generic DT based system
Call trace:
 unwind_backtrace from show_stack+0x10/0x14
 show_stack from dump_stack_lvl+0x54/0x68
 dump_stack_lvl from panic+0xf8/0x34c
 panic from do_exit+0x1dc/0x880
 do_exit from sys_exit_group+0x0/0x10