* [PATCH] f2fs: fix 32-bit linking
@ 2019-06-28 10:39 Arnd Bergmann
2019-06-28 12:44 ` Christoph Hellwig
2019-06-28 15:59 ` Jaegeuk Kim
0 siblings, 2 replies; 10+ messages in thread
From: Arnd Bergmann @ 2019-06-28 10:39 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu
Cc: Arnd Bergmann, Qiuyang Sun, Sahitya Tummala, Eric Biggers,
Wang Shilong, linux-f2fs-devel, linux-kernel
Not all architectures support get_user() with a 64-bit argument:
ERROR: "__get_user_bad" [fs/f2fs/f2fs.ko] undefined!
Use copy_from_user() here, this will always work.
Fixes: d2ae7494d043 ("f2fs: ioctl for removing a range from F2FS")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
fs/f2fs/file.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 998affe31419..465853029b8e 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3066,7 +3066,8 @@ static int f2fs_ioc_resize_fs(struct file *filp, unsigned long arg)
if (f2fs_readonly(sbi->sb))
return -EROFS;
- if (get_user(block_count, (__u64 __user *)arg))
+ if (copy_from_user(&block_count, (void __user *)arg,
+ sizeof(block_count)))
return -EFAULT;
ret = f2fs_resize_fs(sbi, block_count);
--
2.20.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] f2fs: fix 32-bit linking
2019-06-28 10:39 [PATCH] f2fs: fix 32-bit linking Arnd Bergmann
@ 2019-06-28 12:44 ` Christoph Hellwig
2019-06-28 13:09 ` Arnd Bergmann
2019-06-28 23:25 ` Randy Dunlap
2019-06-28 15:59 ` Jaegeuk Kim
1 sibling, 2 replies; 10+ messages in thread
From: Christoph Hellwig @ 2019-06-28 12:44 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jaegeuk Kim, Chao Yu, Qiuyang Sun, Sahitya Tummala, Eric Biggers,
Wang Shilong, linux-f2fs-devel, linux-kernel
On Fri, Jun 28, 2019 at 12:39:52PM +0200, Arnd Bergmann wrote:
> Not all architectures support get_user() with a 64-bit argument:
Which architectures are still missing? I think we finally need to
get everyone in line instead of repeatedly working around the lack
of minor arch support.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] f2fs: fix 32-bit linking
2019-06-28 12:44 ` Christoph Hellwig
@ 2019-06-28 13:09 ` Arnd Bergmann
2019-06-28 13:17 ` Christoph Hellwig
2019-06-28 23:25 ` Randy Dunlap
1 sibling, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2019-06-28 13:09 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jaegeuk Kim, Chao Yu, Qiuyang Sun, Sahitya Tummala, Eric Biggers,
Wang Shilong, Linux F2FS DEV, Mailing List,
Linux Kernel Mailing List, Russell King - ARM Linux
On Fri, Jun 28, 2019 at 2:44 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Fri, Jun 28, 2019 at 12:39:52PM +0200, Arnd Bergmann wrote:
> > Not all architectures support get_user() with a 64-bit argument:
>
> Which architectures are still missing? I think we finally need to
> get everyone in line instead of repeatedly working around the lack
> of minor arch support.
I came across this on arm-nommu (which disables
CONFIG_CPU_SPECTRE) during randconfig testing.
I don't see an easy way to add this in there, short of rewriting the
whole __get_user_err() function. Any suggestions?
Arnd
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] f2fs: fix 32-bit linking
2019-06-28 13:09 ` Arnd Bergmann
@ 2019-06-28 13:17 ` Christoph Hellwig
2019-06-28 14:46 ` Arnd Bergmann
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2019-06-28 13:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Christoph Hellwig, Jaegeuk Kim, Chao Yu, Qiuyang Sun,
Sahitya Tummala, Eric Biggers, Wang Shilong, Linux F2FS DEV,
Mailing List, Linux Kernel Mailing List,
Russell King - ARM Linux
On Fri, Jun 28, 2019 at 03:09:47PM +0200, Arnd Bergmann wrote:
> I came across this on arm-nommu (which disables
> CONFIG_CPU_SPECTRE) during randconfig testing.
>
> I don't see an easy way to add this in there, short of rewriting the
> whole __get_user_err() function. Any suggestions?
Can't we just fall back to using copy_from_user with a little wrapper
that switches based on sizeof()?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] f2fs: fix 32-bit linking
2019-06-28 13:17 ` Christoph Hellwig
@ 2019-06-28 14:46 ` Arnd Bergmann
2019-06-28 17:58 ` Russell King - ARM Linux admin
0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2019-06-28 14:46 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jaegeuk Kim, Chao Yu, Qiuyang Sun, Sahitya Tummala, Eric Biggers,
Wang Shilong, Linux F2FS DEV, Mailing List,
Linux Kernel Mailing List, Russell King - ARM Linux
On Fri, Jun 28, 2019 at 3:17 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Fri, Jun 28, 2019 at 03:09:47PM +0200, Arnd Bergmann wrote:
> > I came across this on arm-nommu (which disables
> > CONFIG_CPU_SPECTRE) during randconfig testing.
> >
> > I don't see an easy way to add this in there, short of rewriting the
> > whole __get_user_err() function. Any suggestions?
>
> Can't we just fall back to using copy_from_user with a little wrapper
> that switches based on sizeof()?
I came up with something now. It's not pretty, but seems to satisfy the
compiler. Not a proper patch yet, but let me know if you find a bug.
This might contain a double uaccess_save_and_enable/uaccess_restore,
not sure how much we care about that.
Arnd
index 7e0d2727c6b5..c21cdecadf26 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -307,6 +307,7 @@ static inline void set_fs(mm_segment_t fs)
do { \
unsigned long __gu_addr = (unsigned long)(ptr); \
unsigned long __gu_val; \
+ unsigned long long __gu_val8; \
unsigned int __ua_flags; \
__chk_user_ptr(ptr); \
might_fault(); \
@@ -315,10 +316,13 @@ do {
\
case 1: __get_user_asm_byte(__gu_val, __gu_addr, err); break; \
case 2: __get_user_asm_half(__gu_val, __gu_addr, err); break; \
case 4: __get_user_asm_word(__gu_val, __gu_addr, err); break; \
+ case 8: __get_user_asm_dword(__gu_val8, __gu_addr, err);break; \
default: (__gu_val) = __get_user_bad(); \
} \
uaccess_restore(__ua_flags); \
- (x) = (__typeof__(*(ptr)))__gu_val; \
+ (x) = __builtin_choose_expr(sizeof(*(ptr)) == 8, \
+ (__typeof__(*(ptr)))__gu_val8, \
+ (__typeof__(*(ptr)))__gu_val); \
} while (0)
#define __get_user_asm(x, addr, err, instr) \
@@ -373,6 +377,8 @@ do {
\
__get_user_asm(x, addr, err, ldr)
#endif
+#define __get_user_asm_dword(x, addr, err) \
+ do { err = raw_copy_from_user(&x, (void __user *)addr, 8) ?
-EFAULT : 0; } while (0)
#define __put_user_switch(x, ptr, __err, __fn) \
do { \
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] f2fs: fix 32-bit linking
2019-06-28 10:39 [PATCH] f2fs: fix 32-bit linking Arnd Bergmann
2019-06-28 12:44 ` Christoph Hellwig
@ 2019-06-28 15:59 ` Jaegeuk Kim
2019-07-01 14:54 ` Arnd Bergmann
1 sibling, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2019-06-28 15:59 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Chao Yu, Qiuyang Sun, Sahitya Tummala, Eric Biggers,
Wang Shilong, linux-f2fs-devel, linux-kernel
Hi Arnd,
If you don't mind, can I integrate this into the original patch in the queue?
Thanks,
On 06/28, Arnd Bergmann wrote:
> Not all architectures support get_user() with a 64-bit argument:
>
> ERROR: "__get_user_bad" [fs/f2fs/f2fs.ko] undefined!
>
> Use copy_from_user() here, this will always work.
>
> Fixes: d2ae7494d043 ("f2fs: ioctl for removing a range from F2FS")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> fs/f2fs/file.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 998affe31419..465853029b8e 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -3066,7 +3066,8 @@ static int f2fs_ioc_resize_fs(struct file *filp, unsigned long arg)
> if (f2fs_readonly(sbi->sb))
> return -EROFS;
>
> - if (get_user(block_count, (__u64 __user *)arg))
> + if (copy_from_user(&block_count, (void __user *)arg,
> + sizeof(block_count)))
> return -EFAULT;
>
> ret = f2fs_resize_fs(sbi, block_count);
> --
> 2.20.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] f2fs: fix 32-bit linking
2019-06-28 14:46 ` Arnd Bergmann
@ 2019-06-28 17:58 ` Russell King - ARM Linux admin
2019-07-01 14:58 ` Arnd Bergmann
0 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux admin @ 2019-06-28 17:58 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Christoph Hellwig, Jaegeuk Kim, Chao Yu, Qiuyang Sun,
Sahitya Tummala, Eric Biggers, Wang Shilong, Linux F2FS DEV,
Mailing List, Linux Kernel Mailing List
On Fri, Jun 28, 2019 at 04:46:14PM +0200, Arnd Bergmann wrote:
> On Fri, Jun 28, 2019 at 3:17 PM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Fri, Jun 28, 2019 at 03:09:47PM +0200, Arnd Bergmann wrote:
> > > I came across this on arm-nommu (which disables
> > > CONFIG_CPU_SPECTRE) during randconfig testing.
> > >
> > > I don't see an easy way to add this in there, short of rewriting the
> > > whole __get_user_err() function. Any suggestions?
> >
> > Can't we just fall back to using copy_from_user with a little wrapper
> > that switches based on sizeof()?
>
> I came up with something now. It's not pretty, but seems to satisfy the
> compiler. Not a proper patch yet, but let me know if you find a bug.
Have you checked what the behaviour is when "ptr" is a pointer to a
pointer? I think you'll end up with a compiler warning for every
case, complaining about casting an unsigned long long to a pointer.
>
> This might contain a double uaccess_save_and_enable/uaccess_restore,
> not sure how much we care about that.
>
> Arnd
>
> index 7e0d2727c6b5..c21cdecadf26 100644
> --- a/arch/arm/include/asm/uaccess.h
> +++ b/arch/arm/include/asm/uaccess.h
> @@ -307,6 +307,7 @@ static inline void set_fs(mm_segment_t fs)
> do { \
> unsigned long __gu_addr = (unsigned long)(ptr); \
> unsigned long __gu_val; \
> + unsigned long long __gu_val8; \
> unsigned int __ua_flags; \
> __chk_user_ptr(ptr); \
> might_fault(); \
> @@ -315,10 +316,13 @@ do {
> \
> case 1: __get_user_asm_byte(__gu_val, __gu_addr, err); break; \
> case 2: __get_user_asm_half(__gu_val, __gu_addr, err); break; \
> case 4: __get_user_asm_word(__gu_val, __gu_addr, err); break; \
> + case 8: __get_user_asm_dword(__gu_val8, __gu_addr, err);break; \
> default: (__gu_val) = __get_user_bad(); \
> } \
> uaccess_restore(__ua_flags); \
> - (x) = (__typeof__(*(ptr)))__gu_val; \
> + (x) = __builtin_choose_expr(sizeof(*(ptr)) == 8, \
> + (__typeof__(*(ptr)))__gu_val8, \
> + (__typeof__(*(ptr)))__gu_val); \
> } while (0)
>
> #define __get_user_asm(x, addr, err, instr) \
> @@ -373,6 +377,8 @@ do {
> \
> __get_user_asm(x, addr, err, ldr)
> #endif
>
> +#define __get_user_asm_dword(x, addr, err) \
> + do { err = raw_copy_from_user(&x, (void __user *)addr, 8) ?
> -EFAULT : 0; } while (0)
>
> #define __put_user_switch(x, ptr, __err, __fn) \
> do { \
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] f2fs: fix 32-bit linking
2019-06-28 12:44 ` Christoph Hellwig
2019-06-28 13:09 ` Arnd Bergmann
@ 2019-06-28 23:25 ` Randy Dunlap
1 sibling, 0 replies; 10+ messages in thread
From: Randy Dunlap @ 2019-06-28 23:25 UTC (permalink / raw)
To: Christoph Hellwig, Arnd Bergmann
Cc: Jaegeuk Kim, Chao Yu, Qiuyang Sun, Sahitya Tummala, Eric Biggers,
Wang Shilong, linux-f2fs-devel, linux-kernel
On 6/28/19 5:44 AM, Christoph Hellwig wrote:
> On Fri, Jun 28, 2019 at 12:39:52PM +0200, Arnd Bergmann wrote:
>> Not all architectures support get_user() with a 64-bit argument:
>
> Which architectures are still missing? I think we finally need to
> get everyone in line instead of repeatedly working around the lack
> of minor arch support.
>
arch/microblaze/include/asm/uaccess.c does not support get_user()
with a size of 8.
--
~Randy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] f2fs: fix 32-bit linking
2019-06-28 15:59 ` Jaegeuk Kim
@ 2019-07-01 14:54 ` Arnd Bergmann
0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2019-07-01 14:54 UTC (permalink / raw)
To: Jaegeuk Kim
Cc: Chao Yu, Qiuyang Sun, Sahitya Tummala, Eric Biggers,
Wang Shilong, Linux F2FS DEV, Mailing List,
Linux Kernel Mailing List
On Fri, Jun 28, 2019 at 5:59 PM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>
> Hi Arnd,
>
> If you don't mind, can I integrate this into the original patch in the queue?
Yes, I think that would be good anyway, it may take a little longer to fix all
the architectures.
Arnd
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] f2fs: fix 32-bit linking
2019-06-28 17:58 ` Russell King - ARM Linux admin
@ 2019-07-01 14:58 ` Arnd Bergmann
0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2019-07-01 14:58 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Christoph Hellwig, Jaegeuk Kim, Chao Yu, Qiuyang Sun,
Sahitya Tummala, Eric Biggers, Wang Shilong, Linux F2FS DEV,
Mailing List, Linux Kernel Mailing List
On Fri, Jun 28, 2019 at 7:58 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Fri, Jun 28, 2019 at 04:46:14PM +0200, Arnd Bergmann wrote:
> > On Fri, Jun 28, 2019 at 3:17 PM Christoph Hellwig <hch@infradead.org> wrote:
> > >
> > > On Fri, Jun 28, 2019 at 03:09:47PM +0200, Arnd Bergmann wrote:
> > > > I came across this on arm-nommu (which disables
> > > > CONFIG_CPU_SPECTRE) during randconfig testing.
> > > >
> > > > I don't see an easy way to add this in there, short of rewriting the
> > > > whole __get_user_err() function. Any suggestions?
> > >
> > > Can't we just fall back to using copy_from_user with a little wrapper
> > > that switches based on sizeof()?
> >
> > I came up with something now. It's not pretty, but seems to satisfy the
> > compiler. Not a proper patch yet, but let me know if you find a bug.
>
> Have you checked what the behaviour is when "ptr" is a pointer to a
> pointer? I think you'll end up with a compiler warning for every
> case, complaining about casting an unsigned long long to a pointer.
I have built lots of kernels using this patch as a test, though my autobuilder
is currently configured to use clang-8, and other compilers or versions
might show more warnings.
> > uaccess_restore(__ua_flags); \
> > - (x) = (__typeof__(*(ptr)))__gu_val; \
> > + (x) = __builtin_choose_expr(sizeof(*(ptr)) == 8, \
> > + (__typeof__(*(ptr)))__gu_val8, \
> > + (__typeof__(*(ptr)))__gu_val); \
> > } while (0)
The __builtin_choose_expr() here is supposed to take care of the case
of a pointer type, gcc and clang should both ignore the non-taken
branch and only produce warnings for the case they actually use.
Arnd
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-07-01 14:58 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-28 10:39 [PATCH] f2fs: fix 32-bit linking Arnd Bergmann
2019-06-28 12:44 ` Christoph Hellwig
2019-06-28 13:09 ` Arnd Bergmann
2019-06-28 13:17 ` Christoph Hellwig
2019-06-28 14:46 ` Arnd Bergmann
2019-06-28 17:58 ` Russell King - ARM Linux admin
2019-07-01 14:58 ` Arnd Bergmann
2019-06-28 23:25 ` Randy Dunlap
2019-06-28 15:59 ` Jaegeuk Kim
2019-07-01 14:54 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).