xtensa: align: validate user access in fast_load_store under MMU

[PATCH] xtensa: align: validate user access in fast_load_store under MMU

Posted by Ricky Ringler 1 month, 3 weeks ago

fast_load_store aligns faulting address then reads two words
with l32i. With CONFIG_MMU, bad user access can fault
in this path.

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@gmail.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

Signed-off-by: Ricky Ringler <richard.rringler@gmail.com>
---
 arch/xtensa/include/asm/asm-uaccess.h | 2 +-
 arch/xtensa/kernel/align.S            | 9 ++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/xtensa/include/asm/asm-uaccess.h b/arch/xtensa/include/asm/asm-uaccess.h
index 7cec869136e3..c5baa134d3d8 100644
--- a/arch/xtensa/include/asm/asm-uaccess.h
+++ b/arch/xtensa/include/asm/asm-uaccess.h
@@ -68,7 +68,7 @@
  * 	<aa>	register containing memory address
  * 	<as>	register containing memory size
  * 	<at>	temp register
- * 	<sp>
+ * 	<sp>	unused; ignored
  * 	<error>	label to branch to on error; implies fall-through
  * 		macro on success
  * On Exit:
diff --git a/arch/xtensa/kernel/align.S b/arch/xtensa/kernel/align.S
index ee97edce2300..808f9f843d33 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,15 +181,15 @@ ENTRY(fast_load_store)
 	bbsi.l	a4, OP1_SI_BIT + INSN_OP1, .Linvalid_instruction
 
 1:
-	movi	a3, ~3
+	movi    a3, ~3
 	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
 #endif
 	l32i	a5, a3, 0
 	l32i	a6, a3, 4
-- 
2.43.0

[PATCH] xtensa: mmu: validate user access in fast_load_store with MMU

Posted by Ricky Ringler 1 month, 3 weeks ago

fast_load_store aligns faulting address then reads two words
with l32i. With CONFIG_MMU, bad user access can fault
in this path.

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@gmail.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

Signed-off-by: Ricky Ringler <richard.rringler@gmail.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
 	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
 #endif
+
 	l32i	a5, a3, 0
 	l32i	a6, a3, 4
 	__src_b	a3, a5, a6		# a3 has the data word
-- 
2.43.0

Re: [PATCH] xtensa: mmu: validate user access in fast_load_store with MMU

Posted by Max Filippov 1 month, 3 weeks ago

Hi Ricky,

On Fri, Dec 12, 2025 at 6:53 PM Ricky Ringler
<richard.rringler@gmail.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@gmail.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@gmail.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

Re: [PATCH] xtensa: mmu: validate user access in fast_load_store with MMU

Posted by Max Filippov 1 month, 3 weeks ago

On Sat, Dec 13, 2025 at 11:14 PM Max Filippov <jcmvbkbc@gmail.com> wrote:
> >  #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
>
> New code should use tabs, not spaces, just like the existing code.

Disregard that, the other version of the patch is formatted with spaces,
this one is good.

Please add patch version if you send multiple versions of the patch.

-- 
Thanks.
-- Max

[PATCH v2] xtensa: align: validate access in fast_load_store

Posted by Ricky Ringler 1 month, 3 weeks ago

access_ok() is used only in user mode and
branches to .Linvalid_instruction on fault.
Kernel mode behavior unchanged.

v2:
- Address Max's feedback
- Fixed formatting
- Added user mode branch

Tested-by: Ricky Ringler <richard.rringler@gmail.com>

Testing:
- Built with CONFIG_MMU enabled
- objdump before/after comparison and validated code path

Signed-off-by: Ricky Ringler <richard.rringler@gmail.com>
---
 arch/xtensa/kernel/align.S | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/xtensa/kernel/align.S b/arch/xtensa/kernel/align.S
index ee97edce2300..64e775933e9b 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
@@ -185,8 +188,12 @@ ENTRY(fast_load_store)
 
 #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
+	rsr	a6, ps
+	bbsi.l	a6, PS_UM_BIT, 1f	# user mode
+	j	.Linvalid_instruction	# kernel mode
+1:
+	movi	a5, 8
+	access_ok a3, a5, a6, a2, .Linvalid_instruction
 #endif
 	l32i	a5, a3, 0
 	l32i	a6, a3, 4
-- 
2.43.0

Re: [PATCH v2] xtensa: align: validate access in fast_load_store

Posted by Max Filippov 1 month, 3 weeks ago

On Sun, Dec 14, 2025 at 2:30 PM Ricky Ringler
<richard.rringler@gmail.com> wrote:
>
> access_ok() is used only in user mode and
> branches to .Linvalid_instruction on fault.
> Kernel mode behavior unchanged.
>
> v2:
> - Address Max's feedback
> - Fixed formatting
> - Added user mode branch

Please move the patch change log past the `---` marker so that
it is dropped when the patch is committed.

> Tested-by: Ricky Ringler <richard.rringler@gmail.com>
>
> Testing:
> - Built with CONFIG_MMU enabled
> - objdump before/after comparison and validated code path
>
> Signed-off-by: Ricky Ringler <richard.rringler@gmail.com>
> ---
>  arch/xtensa/kernel/align.S | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/xtensa/kernel/align.S b/arch/xtensa/kernel/align.S
> index ee97edce2300..64e775933e9b 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
> @@ -185,8 +188,12 @@ ENTRY(fast_load_store)
>
>  #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
> +       rsr     a6, ps
> +       bbsi.l  a6, PS_UM_BIT, 1f       # user mode
> +       j       .Linvalid_instruction   # kernel mode

It should be the other way around: kernel mode is allowed to access
anything, user mode needs address checking.

> +1:
> +       movi    a5, 8
> +       access_ok a3, a5, a6, a2, .Linvalid_instruction
>  #endif
>         l32i    a5, a3, 0
>         l32i    a6, a3, 4
> --
> 2.43.0
>


-- 
Thanks.
-- Max

[PATCH v3] xtensa: align: validate access in fast_load_store

Posted by Ricky Ringler 1 month, 3 weeks ago

access_ok() is used only in user mode and
branches to .Linvalid_instruction on fault.
Kernel mode skips access_ok().

Tested-by: Ricky Ringler <richard.rringler@gmail.com>

Signed-off-by: Ricky Ringler <richard.rringler@gmail.com>
---
v3:
- Changed kernel mode behavior to skip access_ok()

v2:
- Address Max's feedback
- Fixed formatting
- Added user mode branch

Testing:
- Built with CONFIG_MMU enabled
- objdump before/after comparison and validated code path

 arch/xtensa/kernel/align.S | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/xtensa/kernel/align.S b/arch/xtensa/kernel/align.S
index ee97edce2300..f2d0d831ce43 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
@@ -185,8 +188,11 @@ ENTRY(fast_load_store)
 
 #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
+	rsr	a6, ps
+	bbci.l	a6, PS_UM_BIT, 1f # kernel mode
+	movi	a5, 8
+	access_ok a3, a5, a6, a2, .Linvalid_instruction
+1:
 #endif
 	l32i	a5, a3, 0
 	l32i	a6, a3, 4
-- 
2.43.0

Re: [PATCH v3] xtensa: align: validate access in fast_load_store

Posted by Max Filippov 1 month, 3 weeks ago

On Mon, Dec 15, 2025 at 6:33 AM Ricky Ringler
<richard.rringler@gmail.com> wrote:
>
> access_ok() is used only in user mode and
> branches to .Linvalid_instruction on fault.
> Kernel mode skips access_ok().
>
> Tested-by: Ricky Ringler <richard.rringler@gmail.com>
>
> Signed-off-by: Ricky Ringler <richard.rringler@gmail.com>
> ---
> v3:
> - Changed kernel mode behavior to skip access_ok()
>
> v2:
> - Address Max's feedback
> - Fixed formatting
> - Added user mode branch
>
> Testing:
> - Built with CONFIG_MMU enabled
> - objdump before/after comparison and validated code path
>
>  arch/xtensa/kernel/align.S | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

Thanks. Applied to my xtensa tree.
-- Max