mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] xtensa: align: validate user access in fast_load_store under MMU
@ 2025-12-13  2:52 Ricky Ringler
  2025-12-13  2:52 ` [PATCH] xtensa: mmu: validate user access in fast_load_store with MMU Ricky Ringler
  0 siblings, 1 reply; 8+ messages in thread
From: Ricky Ringler @ 2025-12-13  2:52 UTC (permalink / raw)
  To: Chris Zankel, Max Filippov; +Cc: linux-kernel, Ricky Ringler

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] xtensa: mmu: validate user access in fast_load_store with MMU
  2025-12-13  2:52 [PATCH] xtensa: align: validate user access in fast_load_store under MMU Ricky Ringler
@ 2025-12-13  2:52 ` Ricky Ringler
  2025-12-14  7:14   ` Max Filippov
  2025-12-14 22:28   ` [PATCH v2] xtensa: align: validate access in fast_load_store Ricky Ringler
  0 siblings, 2 replies; 8+ messages in thread
From: Ricky Ringler @ 2025-12-13  2:52 UTC (permalink / raw)
  To: Chris Zankel, Max Filippov; +Cc: linux-kernel, Ricky Ringler

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xtensa: mmu: validate user access in fast_load_store with MMU
  2025-12-13  2:52 ` [PATCH] xtensa: mmu: validate user access in fast_load_store with MMU Ricky Ringler
@ 2025-12-14  7:14   ` Max Filippov
  2025-12-14  7:24     ` Max Filippov
  2025-12-14 22:28   ` [PATCH v2] xtensa: align: validate access in fast_load_store Ricky Ringler
  1 sibling, 1 reply; 8+ messages in thread
From: Max Filippov @ 2025-12-14  7:14 UTC (permalink / raw)
  To: Ricky Ringler; +Cc: Chris Zankel, linux-kernel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xtensa: mmu: validate user access in fast_load_store with MMU
  2025-12-14  7:14   ` Max Filippov
@ 2025-12-14  7:24     ` Max Filippov
  0 siblings, 0 replies; 8+ messages in thread
From: Max Filippov @ 2025-12-14  7:24 UTC (permalink / raw)
  To: Ricky Ringler; +Cc: Chris Zankel, linux-kernel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] xtensa: align: validate access in fast_load_store
  2025-12-13  2:52 ` [PATCH] xtensa: mmu: validate user access in fast_load_store with MMU Ricky Ringler
  2025-12-14  7:14   ` Max Filippov
@ 2025-12-14 22:28   ` Ricky Ringler
  2025-12-15  3:06     ` Max Filippov
  1 sibling, 1 reply; 8+ messages in thread
From: Ricky Ringler @ 2025-12-14 22:28 UTC (permalink / raw)
  To: Max Filippov, Chris Zankel; +Cc: linux-kernel, Ricky Ringler

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] xtensa: align: validate access in fast_load_store
  2025-12-14 22:28   ` [PATCH v2] xtensa: align: validate access in fast_load_store Ricky Ringler
@ 2025-12-15  3:06     ` Max Filippov
  2025-12-15 14:33       ` [PATCH v3] " Ricky Ringler
  0 siblings, 1 reply; 8+ messages in thread
From: Max Filippov @ 2025-12-15  3:06 UTC (permalink / raw)
  To: Ricky Ringler; +Cc: Chris Zankel, linux-kernel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3] xtensa: align: validate access in fast_load_store
  2025-12-15  3:06     ` Max Filippov
@ 2025-12-15 14:33       ` Ricky Ringler
  2025-12-15 15:11         ` Max Filippov
  0 siblings, 1 reply; 8+ messages in thread
From: Ricky Ringler @ 2025-12-15 14:33 UTC (permalink / raw)
  To: Max Filippov, Chris Zankel; +Cc: linux-kernel, Ricky Ringler

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] xtensa: align: validate access in fast_load_store
  2025-12-15 14:33       ` [PATCH v3] " Ricky Ringler
@ 2025-12-15 15:11         ` Max Filippov
  0 siblings, 0 replies; 8+ messages in thread
From: Max Filippov @ 2025-12-15 15:11 UTC (permalink / raw)
  To: Ricky Ringler; +Cc: Chris Zankel, linux-kernel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-12-15 15:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-13  2:52 [PATCH] xtensa: align: validate user access in fast_load_store under MMU Ricky Ringler
2025-12-13  2:52 ` [PATCH] xtensa: mmu: validate user access in fast_load_store with MMU Ricky Ringler
2025-12-14  7:14   ` Max Filippov
2025-12-14  7:24     ` Max Filippov
2025-12-14 22:28   ` [PATCH v2] xtensa: align: validate access in fast_load_store Ricky Ringler
2025-12-15  3:06     ` Max Filippov
2025-12-15 14:33       ` [PATCH v3] " Ricky Ringler
2025-12-15 15:11         ` Max Filippov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®