| Message ID | 20220216131332.1489939-3-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | clean up asm/uaccess.h, kill set_fs for good | expand |
From: Arnd Bergmann > Sent: 16 February 2022 13:13 > > These two architectures implement 8-byte get_user() through > a memcpy() into a four-byte variable, which won't fit. > > Use a temporary 64-bit variable instead here, and use a double > cast the way that risc-v and openrisc do to avoid compile-time > warnings. > ... > case 4: \ > - __get_user_asm("lw", (ptr), __gu_val, __gu_err); \ > + __get_user_asm("lw", (ptr), x, __gu_err); \ > break; \ > - case 8: \ > - __gu_err = __copy_from_user(&__gu_val, ptr, 8); \ > - if (__gu_err) \ > - __gu_err = -EFAULT; \ > + case 8: { \ > + __u64 __x = 0; \ > + __gu_err = raw_copy_from_user(&__x, ptr, 8) ? \ > + -EFAULT : 0; \ > + (x) = (typeof(x))(typeof((x) - (x)))__x; \ > break; \ Wouldn't it be better to just fetch two 32bit values: Something like (for LE - nios2 is definitely LE: __u32 val_lo, val_hi; __get_user_asm("lw", (ptr), val_lo, __gu_err); __get_user_asm("lw", (ptr) + 4, val_hi, __gu_err); x = val_lo | val_hi << 32; break; David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On 2/16/22 07:13, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > These two architectures implement 8-byte get_user() through > a memcpy() into a four-byte variable, which won't fit. > > Use a temporary 64-bit variable instead here, and use a double > cast the way that risc-v and openrisc do to avoid compile-time > warnings. > > Fixes: 6a090e97972d ("arch/microblaze: support get_user() of size 8 bytes") > Fixes: 5ccc6af5e88e ("nios2: Memory management") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > arch/microblaze/include/asm/uaccess.h | 18 +++++++++--------- > arch/nios2/include/asm/uaccess.h | 26 ++++++++++++++++---------- > 2 files changed, 25 insertions(+), 19 deletions(-) > > diff --git a/arch/microblaze/include/asm/uaccess.h b/arch/microblaze/include/asm/uaccess.h > index 5b6e0e7788f4..3fe96979d2c6 100644 > --- a/arch/microblaze/include/asm/uaccess.h > +++ b/arch/microblaze/include/asm/uaccess.h > @@ -130,27 +130,27 @@ extern long __user_bad(void); > > #define __get_user(x, ptr) \ > ({ \ > - unsigned long __gu_val = 0; \ > long __gu_err; \ > switch (sizeof(*(ptr))) { \ > case 1: \ > - __get_user_asm("lbu", (ptr), __gu_val, __gu_err); \ > + __get_user_asm("lbu", (ptr), x, __gu_err); \ > break; \ > case 2: \ > - __get_user_asm("lhu", (ptr), __gu_val, __gu_err); \ > + __get_user_asm("lhu", (ptr), x, __gu_err); \ > break; \ > case 4: \ > - __get_user_asm("lw", (ptr), __gu_val, __gu_err); \ > + __get_user_asm("lw", (ptr), x, __gu_err); \ > break; \ > - case 8: \ > - __gu_err = __copy_from_user(&__gu_val, ptr, 8); \ > - if (__gu_err) \ > - __gu_err = -EFAULT; \ > + case 8: { \ > + __u64 __x = 0; \ > + __gu_err = raw_copy_from_user(&__x, ptr, 8) ? \ > + -EFAULT : 0; \ > + (x) = (typeof(x))(typeof((x) - (x)))__x; \ > break; \ > + } \ > default: \ > /* __gu_val = 0; __gu_err = -EINVAL;*/ __gu_err = __user_bad();\ > } \ > - x = (__force __typeof__(*(ptr))) __gu_val; \ > __gu_err; \ > }) > > diff --git a/arch/nios2/include/asm/uaccess.h b/arch/nios2/include/asm/uaccess.h > index ba9340e96fd4..ca9285a915ef 100644 > --- a/arch/nios2/include/asm/uaccess.h > +++ b/arch/nios2/include/asm/uaccess.h > @@ -88,6 +88,7 @@ extern __must_check long strnlen_user(const char __user *s, long n); > /* Optimized macros */ > #define __get_user_asm(val, insn, addr, err) \ > { \ > + unsigned long __gu_val; \ > __asm__ __volatile__( \ > " movi %0, %3\n" \ > "1: " insn " %1, 0(%2)\n" \ > @@ -96,14 +97,20 @@ extern __must_check long strnlen_user(const char __user *s, long n); > " .section __ex_table,\"a\"\n" \ > " .word 1b, 2b\n" \ > " .previous" \ > - : "=&r" (err), "=r" (val) \ > + : "=&r" (err), "=r" (__gu_val) \ > : "r" (addr), "i" (-EFAULT)); \ > + val = (__force __typeof__(*(addr)))__gu_val; \ > } > > -#define __get_user_unknown(val, size, ptr, err) do { \ > +extern void __get_user_unknown(void); > + > +#define __get_user_8(val, ptr, err) do { \ > + u64 __val = 0; \ > err = 0; \ > - if (__copy_from_user(&(val), ptr, size)) { \ > + if (raw_copy_from_user(&(__val), ptr, sizeof(val))) { \ > err = -EFAULT; \ > + } else { \ > + val = (typeof(val))(typeof((val) - (val)))__val; \ > } \ > } while (0) > > @@ -119,8 +126,11 @@ do { \ > case 4: \ > __get_user_asm(val, "ldw", ptr, err); \ > break; \ > + case 8: \ > + __get_user_8(val, ptr, err); \ > + break; \ > default: \ > - __get_user_unknown(val, size, ptr, err); \ > + __get_user_unknown(); \ > break; \ > } \ > } while (0) > @@ -129,9 +139,7 @@ do { \ > ({ \ > long __gu_err = -EFAULT; \ > const __typeof__(*(ptr)) __user *__gu_ptr = (ptr); \ > - unsigned long __gu_val = 0; \ > - __get_user_common(__gu_val, sizeof(*(ptr)), __gu_ptr, __gu_err);\ > - (x) = (__force __typeof__(x))__gu_val; \ > + __get_user_common(x, sizeof(*(ptr)), __gu_ptr, __gu_err); \ > __gu_err; \ > }) > > @@ -139,11 +147,9 @@ do { \ > ({ \ > long __gu_err = -EFAULT; \ > const __typeof__(*(ptr)) __user *__gu_ptr = (ptr); \ > - unsigned long __gu_val = 0; \ > if (access_ok( __gu_ptr, sizeof(*__gu_ptr))) \ > - __get_user_common(__gu_val, sizeof(*__gu_ptr), \ > + __get_user_common(x, sizeof(*__gu_ptr), \ > __gu_ptr, __gu_err); \ > - (x) = (__force __typeof__(x))__gu_val; \ > __gu_err; \ > }) > Acked-by: Dinh Nguyen <dinguyen@kernel.org>
diff --git a/arch/microblaze/include/asm/uaccess.h b/arch/microblaze/include/asm/uaccess.h index 5b6e0e7788f4..3fe96979d2c6 100644 --- a/arch/microblaze/include/asm/uaccess.h +++ b/arch/microblaze/include/asm/uaccess.h @@ -130,27 +130,27 @@ extern long __user_bad(void); #define __get_user(x, ptr) \ ({ \ - unsigned long __gu_val = 0; \ long __gu_err; \ switch (sizeof(*(ptr))) { \ case 1: \ - __get_user_asm("lbu", (ptr), __gu_val, __gu_err); \ + __get_user_asm("lbu", (ptr), x, __gu_err); \ break; \ case 2: \ - __get_user_asm("lhu", (ptr), __gu_val, __gu_err); \ + __get_user_asm("lhu", (ptr), x, __gu_err); \ break; \ case 4: \ - __get_user_asm("lw", (ptr), __gu_val, __gu_err); \ + __get_user_asm("lw", (ptr), x, __gu_err); \ break; \ - case 8: \ - __gu_err = __copy_from_user(&__gu_val, ptr, 8); \ - if (__gu_err) \ - __gu_err = -EFAULT; \ + case 8: { \ + __u64 __x = 0; \ + __gu_err = raw_copy_from_user(&__x, ptr, 8) ? \ + -EFAULT : 0; \ + (x) = (typeof(x))(typeof((x) - (x)))__x; \ break; \ + } \ default: \ /* __gu_val = 0; __gu_err = -EINVAL;*/ __gu_err = __user_bad();\ } \ - x = (__force __typeof__(*(ptr))) __gu_val; \ __gu_err; \ }) diff --git a/arch/nios2/include/asm/uaccess.h b/arch/nios2/include/asm/uaccess.h index ba9340e96fd4..ca9285a915ef 100644 --- a/arch/nios2/include/asm/uaccess.h +++ b/arch/nios2/include/asm/uaccess.h @@ -88,6 +88,7 @@ extern __must_check long strnlen_user(const char __user *s, long n); /* Optimized macros */ #define __get_user_asm(val, insn, addr, err) \ { \ + unsigned long __gu_val; \ __asm__ __volatile__( \ " movi %0, %3\n" \ "1: " insn " %1, 0(%2)\n" \ @@ -96,14 +97,20 @@ extern __must_check long strnlen_user(const char __user *s, long n); " .section __ex_table,\"a\"\n" \ " .word 1b, 2b\n" \ " .previous" \ - : "=&r" (err), "=r" (val) \ + : "=&r" (err), "=r" (__gu_val) \ : "r" (addr), "i" (-EFAULT)); \ + val = (__force __typeof__(*(addr)))__gu_val; \ } -#define __get_user_unknown(val, size, ptr, err) do { \ +extern void __get_user_unknown(void); + +#define __get_user_8(val, ptr, err) do { \ + u64 __val = 0; \ err = 0; \ - if (__copy_from_user(&(val), ptr, size)) { \ + if (raw_copy_from_user(&(__val), ptr, sizeof(val))) { \ err = -EFAULT; \ + } else { \ + val = (typeof(val))(typeof((val) - (val)))__val; \ } \ } while (0) @@ -119,8 +126,11 @@ do { \ case 4: \ __get_user_asm(val, "ldw", ptr, err); \ break; \ + case 8: \ + __get_user_8(val, ptr, err); \ + break; \ default: \ - __get_user_unknown(val, size, ptr, err); \ + __get_user_unknown(); \ break; \ } \ } while (0) @@ -129,9 +139,7 @@ do { \ ({ \ long __gu_err = -EFAULT; \ const __typeof__(*(ptr)) __user *__gu_ptr = (ptr); \ - unsigned long __gu_val = 0; \ - __get_user_common(__gu_val, sizeof(*(ptr)), __gu_ptr, __gu_err);\ - (x) = (__force __typeof__(x))__gu_val; \ + __get_user_common(x, sizeof(*(ptr)), __gu_ptr, __gu_err); \ __gu_err; \ }) @@ -139,11 +147,9 @@ do { \ ({ \ long __gu_err = -EFAULT; \ const __typeof__(*(ptr)) __user *__gu_ptr = (ptr); \ - unsigned long __gu_val = 0; \ if (access_ok( __gu_ptr, sizeof(*__gu_ptr))) \ - __get_user_common(__gu_val, sizeof(*__gu_ptr), \ + __get_user_common(x, sizeof(*__gu_ptr), \ __gu_ptr, __gu_err); \ - (x) = (__force __typeof__(x))__gu_val; \ __gu_err; \ })