linux-kernel - Re: [PATCH] xtensa: mmu: validate user access in fast_load

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMo8Bf+aaBMnSdxQGgfwKOGLhWGW=u2pmTB_X996C3aHO9g16g@mail.gmail.com>
Date: Sat, 13 Dec 2025 23:14:04 -0800
From: Max Filippov <jcmvbkbc@...il.com>
To: Ricky Ringler <richard.rringler@...il.com>
Cc: Chris Zankel <chris@...kel.net>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] xtensa: mmu: validate user access in fast_load_store with MMU

Hi Ricky,

On Fri, Dec 12, 2025 at 6:53 PM Ricky Ringler
<richard.rringler@...il.com> wrote:
>
> fast_load_store aligns faulting address then reads two words
> with l32i. With CONFIG_MMU, bad user access can fault
> in this path.

This fault can also happen when accessing memory from
the kernel mode.

> Replace arbitrary jump to .Linvalid_instruction with
> access_ok() to validate aligned address before loads
> and branch to .Linvalid_instruction on failure.
>
> a0: scratch. a2: sp. Matches existing usage in
> Xtensa entry.S.
>
> Tested-by: Ricky Ringler <richard.rringler@...il.com>
>
> Testing:
> - Built Xtensa with CONFIG_MMU enabled
> - objdump before/after comparison and validated code path
> - Ran emulated Xtensa device with QEMU and manually
> triggered unaligned and aligned loads

fast_load_store is not called for the unaligned access, it's called
for the load/store exception. On the real hardware it happens e.g.
when l8ui is used to load from the address range mapped to the
instruction fetch bus. This doesn't happen on QEMU.

> Signed-off-by: Ricky Ringler <richard.rringler@...il.com>
> ---
>  arch/xtensa/kernel/align.S | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/xtensa/kernel/align.S b/arch/xtensa/kernel/align.S
> index ee97edce2300..5205c6ebb586 100644
> --- a/arch/xtensa/kernel/align.S
> +++ b/arch/xtensa/kernel/align.S
> @@ -21,6 +21,9 @@
>  #include <asm/asm-offsets.h>
>  #include <asm/asmmacro.h>
>  #include <asm/processor.h>
> +#ifdef CONFIG_MMU
> +#include <asm/asm-uaccess.h>
> +#endif
>
>  #if XCHAL_UNALIGNED_LOAD_EXCEPTION || defined CONFIG_XTENSA_LOAD_STORE
>  #define LOAD_EXCEPTION_HANDLER
> @@ -178,16 +181,17 @@ ENTRY(fast_load_store)
>         bbsi.l  a4, OP1_SI_BIT + INSN_OP1, .Linvalid_instruction
>
>  1:
> -       movi    a3, ~3
> +       movi    a3, ~3

There's a formatting change here, but the original formatting is fine.

>         and     a3, a3, a8              # align memory address
>
>         __ssa8  a8
>
>  #ifdef CONFIG_MMU
>         /* l32e can't be used here even when it's available. */
> -       /* TODO access_ok(a3) could be used here */
> -       j       .Linvalid_instruction
> +       movi    a5, 8
> +       access_ok a3, a5, a0, a2, .Linvalid_instruction

a0 cannot be used here, it is clobbered by the macro, but the value
loaded into it by the code above is used in the code below.
a6 would work though.

access_ok only checks address validity for the user mode, but
it is also possible to get here from the kernel mode.

New code should use tabs, not spaces, just like the existing code.

>  #endif
> +
>         l32i    a5, a3, 0
>         l32i    a6, a3, 4
>         __src_b a3, a5, a6              # a3 has the data word
> --
> 2.43.0
>

-- 
Thanks.
-- Max