All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sh: uaccess: Require offsettable operands for 64-bit user access
@ 2026-07-16 16:53 Florian Fuchs
  2026-07-16 17:05 ` John Paul Adrian Glaubitz
  0 siblings, 1 reply; 3+ messages in thread
From: Florian Fuchs @ 2026-07-16 16:53 UTC (permalink / raw)
  To: Rich Felker, John Paul Adrian Glaubitz, linux-sh
  Cc: Florian Fuchs, linux-kernel

The 64-bit __get_user_u64()/__put_user_u64() paths address the memory
operand as %T2 (base+4). This base+4 requires an offsettable address,
but "m" also allows the indexed mode @(R0,Rn) (e.g. for a variable-
indexed array element), which has no displacement field for the +4.
Depending on optimization this build fails with "invalid 'asm'" or an
ICE in GCC's change_address_1.

Require an offsettable memory operand, allowing GCC to reload an
unsuitable indexed address into an appropriate form.

Fixes: 2d2b308a8b7d ("sh: Implement __get_user_u64() required for 64-bit get_user()")
Signed-off-by: Florian Fuchs <fuchsfl@gmail.com>
---
The patch was verified on the J2 core on the Mimas v2 board. And booted
on the Dreamcast SH4. checkpatch doesn't like the missing whitespaces
around : I preferred not to touch the other unrelated lines, so I kept
the local style.

The original GCC ICE, without this patch can be seen here:
https://lore.kernel.org/linux-sh/ef22a5de-d945-4080-9f4a-f5a1ab9de1dc@landley.net/

 arch/sh/include/asm/uaccess_32.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/sh/include/asm/uaccess_32.h b/arch/sh/include/asm/uaccess_32.h
index 5d7ddc092afd..cef40414bfb1 100644
--- a/arch/sh/include/asm/uaccess_32.h
+++ b/arch/sh/include/asm/uaccess_32.h
@@ -92,7 +92,7 @@ __asm__ __volatile__( \
 	".long	1b + 2, 3b\n\t" \
 	".previous" \
 	:"=&r" (err), "=&r" (x) \
-	:"m" (__m(addr)), "i" (-EFAULT), "0" (err)); })
+	:"o" (__m(addr)), "i" (-EFAULT), "0" (err)); })
 #else
 #define __get_user_u64(x, addr, err) \
 ({ \
@@ -116,7 +116,7 @@ __asm__ __volatile__( \
 	".long	1b + 2, 3b\n\t" \
 	".previous" \
 	:"=&r" (err), "=&r" (x) \
-	:"m" (__m(addr)), "i" (-EFAULT), "0" (err)); })
+	:"o" (__m(addr)), "i" (-EFAULT), "0" (err)); })
 #endif
 
 #define __put_user_size(x,ptr,size,retval)		\
@@ -196,7 +196,7 @@ __asm__ __volatile__( \
 	".long	1b, 3b\n\t" \
 	".previous" \
 	: "=r" (retval) \
-	: "r" (val), "m" (__m(addr)), "i" (-EFAULT), "0" (retval) \
+	: "r" (val), "o" (__m(addr)), "i" (-EFAULT), "0" (retval) \
         : "memory"); })
 #else
 #define __put_user_u64(val,addr,retval) \
@@ -218,7 +218,7 @@ __asm__ __volatile__( \
 	".long	1b, 3b\n\t" \
 	".previous" \
 	: "=r" (retval) \
-	: "r" (val), "m" (__m(addr)), "i" (-EFAULT), "0" (retval) \
+	: "r" (val), "o" (__m(addr)), "i" (-EFAULT), "0" (retval) \
         : "memory"); })
 #endif
 
-- 
2.43.0


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

* Re: [PATCH] sh: uaccess: Require offsettable operands for 64-bit user access
  2026-07-16 16:53 [PATCH] sh: uaccess: Require offsettable operands for 64-bit user access Florian Fuchs
@ 2026-07-16 17:05 ` John Paul Adrian Glaubitz
  2026-07-16 17:52   ` David Laight
  0 siblings, 1 reply; 3+ messages in thread
From: John Paul Adrian Glaubitz @ 2026-07-16 17:05 UTC (permalink / raw)
  To: Florian Fuchs, Rich Felker, linux-sh; +Cc: linux-kernel

Hi Florian,

On Thu, 2026-07-16 at 18:53 +0200, Florian Fuchs wrote:
> The 64-bit __get_user_u64()/__put_user_u64() paths address the memory
> operand as %T2 (base+4). This base+4 requires an offsettable address,
> but "m" also allows the indexed mode @(R0,Rn) (e.g. for a variable-
> indexed array element), which has no displacement field for the +4.
> Depending on optimization this build fails with "invalid 'asm'" or an
> ICE in GCC's change_address_1.
> 
> Require an offsettable memory operand, allowing GCC to reload an
> unsuitable indexed address into an appropriate form.
> 
> Fixes: 2d2b308a8b7d ("sh: Implement __get_user_u64() required for 64-bit get_user()")
> Signed-off-by: Florian Fuchs <fuchsfl@gmail.com>
> ---
> The patch was verified on the J2 core on the Mimas v2 board. And booted
> on the Dreamcast SH4. checkpatch doesn't like the missing whitespaces
> around : I preferred not to touch the other unrelated lines, so I kept
> the local style.
> 
> The original GCC ICE, without this patch can be seen here:
> https://lore.kernel.org/linux-sh/ef22a5de-d945-4080-9f4a-f5a1ab9de1dc@landley.net/
> 
>  arch/sh/include/asm/uaccess_32.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/sh/include/asm/uaccess_32.h b/arch/sh/include/asm/uaccess_32.h
> index 5d7ddc092afd..cef40414bfb1 100644
> --- a/arch/sh/include/asm/uaccess_32.h
> +++ b/arch/sh/include/asm/uaccess_32.h
> @@ -92,7 +92,7 @@ __asm__ __volatile__( \
>  	".long	1b + 2, 3b\n\t" \
>  	".previous" \
>  	:"=&r" (err), "=&r" (x) \
> -	:"m" (__m(addr)), "i" (-EFAULT), "0" (err)); })
> +	:"o" (__m(addr)), "i" (-EFAULT), "0" (err)); })
>  #else
>  #define __get_user_u64(x, addr, err) \
>  ({ \
> @@ -116,7 +116,7 @@ __asm__ __volatile__( \
>  	".long	1b + 2, 3b\n\t" \
>  	".previous" \
>  	:"=&r" (err), "=&r" (x) \
> -	:"m" (__m(addr)), "i" (-EFAULT), "0" (err)); })
> +	:"o" (__m(addr)), "i" (-EFAULT), "0" (err)); })
>  #endif
>  
>  #define __put_user_size(x,ptr,size,retval)		\
> @@ -196,7 +196,7 @@ __asm__ __volatile__( \
>  	".long	1b, 3b\n\t" \
>  	".previous" \
>  	: "=r" (retval) \
> -	: "r" (val), "m" (__m(addr)), "i" (-EFAULT), "0" (retval) \
> +	: "r" (val), "o" (__m(addr)), "i" (-EFAULT), "0" (retval) \
>          : "memory"); })
>  #else
>  #define __put_user_u64(val,addr,retval) \
> @@ -218,7 +218,7 @@ __asm__ __volatile__( \
>  	".long	1b, 3b\n\t" \
>  	".previous" \
>  	: "=r" (retval) \
> -	: "r" (val), "m" (__m(addr)), "i" (-EFAULT), "0" (retval) \
> +	: "r" (val), "o" (__m(addr)), "i" (-EFAULT), "0" (retval) \
>          : "memory"); })
>  #endif
>  

Nice catch, thanks a lot for fixing this!

Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

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

* Re: [PATCH] sh: uaccess: Require offsettable operands for 64-bit user access
  2026-07-16 17:05 ` John Paul Adrian Glaubitz
@ 2026-07-16 17:52   ` David Laight
  0 siblings, 0 replies; 3+ messages in thread
From: David Laight @ 2026-07-16 17:52 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz
  Cc: Florian Fuchs, Rich Felker, linux-sh, linux-kernel

On Thu, 16 Jul 2026 19:05:03 +0200
John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> wrote:

> Hi Florian,
> 
> On Thu, 2026-07-16 at 18:53 +0200, Florian Fuchs wrote:
> > The 64-bit __get_user_u64()/__put_user_u64() paths address the memory
> > operand as %T2 (base+4). This base+4 requires an offsettable address,
> > but "m" also allows the indexed mode @(R0,Rn) (e.g. for a variable-
> > indexed array element), which has no displacement field for the +4.
> > Depending on optimization this build fails with "invalid 'asm'" or an
> > ICE in GCC's change_address_1.
> > 
> > Require an offsettable memory operand, allowing GCC to reload an
> > unsuitable indexed address into an appropriate form.

An alternative would be to pass 'addr' and 'addr + 4' separately so that
the C compilation adds in the offset.
With a bit more effort the define wouldn't need repeating for LE and BE.

There is a bug in the __put_user_u64 code - it is missing the exception
table entry for the second word.
(It is present in __get_user_u64, but a separate label would be better.)

I also wonder whether there should be non-mmu copies?

	David

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

end of thread, other threads:[~2026-07-16 17:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16 16:53 [PATCH] sh: uaccess: Require offsettable operands for 64-bit user access Florian Fuchs
2026-07-16 17:05 ` John Paul Adrian Glaubitz
2026-07-16 17:52   ` David Laight

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.