| Message ID | 20170628180456.30cb9242@canb.auug.org.au (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 06/28/2017 02:04 AM, Stephen Rothwell wrote: > Hi Jens, > > After merging the block tree, today's linux-next build (powerpc > allnoconfig) failed like this: > > fs/fcntl.o: In function `do_fcntl': > fcntl.c:(.text+0x6d4): undefined reference to `__get_user_bad' > fcntl.c:(.text+0x730): undefined reference to `__get_user_bad' > > Probably caused by commit > > c75b1d9421f8 ("fs: add fcntl() interface for setting/getting write life time hints") > > On powerpc (at least) you cannot use get_user() to fetch anything larger > than "unsigned long" i.e. 32 bits on 32 bit powerpc. > > This has been discussed before (and, I think, a fix attempted). Gah, thanks for letting me know. I'll test your patch and queue it up to fix this issue.
On 06/28/2017 06:43 AM, Jens Axboe wrote: > On 06/28/2017 02:04 AM, Stephen Rothwell wrote: >> Hi Jens, >> >> After merging the block tree, today's linux-next build (powerpc >> allnoconfig) failed like this: >> >> fs/fcntl.o: In function `do_fcntl': >> fcntl.c:(.text+0x6d4): undefined reference to `__get_user_bad' >> fcntl.c:(.text+0x730): undefined reference to `__get_user_bad' >> >> Probably caused by commit >> >> c75b1d9421f8 ("fs: add fcntl() interface for setting/getting write life time hints") >> >> On powerpc (at least) you cannot use get_user() to fetch anything larger >> than "unsigned long" i.e. 32 bits on 32 bit powerpc. >> >> This has been discussed before (and, I think, a fix attempted). > > Gah, thanks for letting me know. I'll test your patch and queue it > up to fix this issue. But put_user() is fine? Just checking here, since the change adds both a u64 put and get user.
On 06/28/2017 08:01 AM, Jens Axboe wrote: > On 06/28/2017 06:43 AM, Jens Axboe wrote: >> On 06/28/2017 02:04 AM, Stephen Rothwell wrote: >>> Hi Jens, >>> >>> After merging the block tree, today's linux-next build (powerpc >>> allnoconfig) failed like this: >>> >>> fs/fcntl.o: In function `do_fcntl': >>> fcntl.c:(.text+0x6d4): undefined reference to `__get_user_bad' >>> fcntl.c:(.text+0x730): undefined reference to `__get_user_bad' >>> >>> Probably caused by commit >>> >>> c75b1d9421f8 ("fs: add fcntl() interface for setting/getting write life time hints") >>> >>> On powerpc (at least) you cannot use get_user() to fetch anything larger >>> than "unsigned long" i.e. 32 bits on 32 bit powerpc. >>> >>> This has been discussed before (and, I think, a fix attempted). >> >> Gah, thanks for letting me know. I'll test your patch and queue it >> up to fix this issue. > > But put_user() is fine? Just checking here, since the change adds > both a u64 put and get user. I just changed all 4, at least that provides some symmetry in how we copy things in and out for that set of fcntls.
Hi Jens, On Wed, 28 Jun 2017 09:11:32 -0600 Jens Axboe <axboe@kernel.dk> wrote: > > On 06/28/2017 08:01 AM, Jens Axboe wrote: > > But put_user() is fine? Just checking here, since the change adds > > both a u64 put and get user. Yes, put_user is fine (it does 2 4 byte moves. The asm is there to do the 8 byte get_user, but the surrounding C code uses an unsigned long for the destination in all cases (some other arches do the same). I don't remember why it is like that. > I just changed all 4, at least that provides some symmetry in how > we copy things in and out for that set of fcntls. OK, thanks.
diff --git a/fs/fcntl.c b/fs/fcntl.c index 24e233c75a33..19825eb7c40d 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -277,6 +277,7 @@ static long fcntl_rw_hint(struct file *file, unsigned int cmd, { struct inode *inode = file_inode(file); u64 *argp = (u64 __user *)arg; + u64 h; enum rw_hint hint; switch (cmd) { @@ -285,8 +286,9 @@ static long fcntl_rw_hint(struct file *file, unsigned int cmd, return -EFAULT; return 0; case F_SET_FILE_RW_HINT: - if (get_user(hint, argp)) + if (copy_from_user(&h, argp, sizeof(h))) return -EFAULT; + hint = (enum rw_hint)h; if (!rw_hint_valid(hint)) return -EINVAL; @@ -299,8 +301,9 @@ static long fcntl_rw_hint(struct file *file, unsigned int cmd, return -EFAULT; return 0; case F_SET_RW_HINT: - if (get_user(hint, argp)) + if (copy_from_user(&h, argp, sizeof(h))) return -EFAULT; + hint = (enum rw_hint)h; if (!rw_hint_valid(hint)) return -EINVAL;
Hi Jens, After merging the block tree, today's linux-next build (powerpc allnoconfig) failed like this: fs/fcntl.o: In function `do_fcntl': fcntl.c:(.text+0x6d4): undefined reference to `__get_user_bad' fcntl.c:(.text+0x730): undefined reference to `__get_user_bad' Probably caused by commit c75b1d9421f8 ("fs: add fcntl() interface for setting/getting write life time hints") On powerpc (at least) you cannot use get_user() to fetch anything larger than "unsigned long" i.e. 32 bits on 32 bit powerpc. This has been discussed before (and, I think, a fix attempted). For now I have applied the following patch: From: Stephen Rothwell <sfr@canb.auug.org.au> Date: Wed, 28 Jun 2017 17:50:31 +1000 Subject: [PATCH] fs: don't read an enum directly from a u64 __user * Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> --- fs/fcntl.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)