From 80b79dd0e2f29f06a6a54a5755c718f1c7ebb136 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Sat, 27 May 2017 06:07:18 -0400 Subject: [PATCH 01/13] fs: locks: Fix some troubles at kernel-doc comments There are a few syntax violations that cause outputs of a few comments to not be properly parsed in ReST format. No functional changes. Signed-off-by: Mauro Carvalho Chehab Signed-off-by: Jeff Layton --- fs/locks.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/fs/locks.c b/fs/locks.c index af2031a1fcff..4a4543a7f9c1 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -1858,8 +1858,8 @@ EXPORT_SYMBOL(generic_setlease); * * Call this to establish a lease on the file. The "lease" argument is not * used for F_UNLCK requests and may be NULL. For commands that set or alter - * an existing lease, the (*lease)->fl_lmops->lm_break operation must be set; - * if not, this function will return -ENOLCK (and generate a scary-looking + * an existing lease, the ``(*lease)->fl_lmops->lm_break`` operation must be + * set; if not, this function will return -ENOLCK (and generate a scary-looking * stack trace). * * The "priv" pointer is passed directly to the lm_setup function as-is. It @@ -1972,15 +1972,13 @@ EXPORT_SYMBOL(locks_lock_inode_wait); * @cmd: the type of lock to apply. * * Apply a %FL_FLOCK style lock to an open file descriptor. - * The @cmd can be one of + * The @cmd can be one of: * - * %LOCK_SH -- a shared lock. - * - * %LOCK_EX -- an exclusive lock. - * - * %LOCK_UN -- remove an existing lock. - * - * %LOCK_MAND -- a `mandatory' flock. This exists to emulate Windows Share Modes. + * - %LOCK_SH -- a shared lock. + * - %LOCK_EX -- an exclusive lock. + * - %LOCK_UN -- remove an existing lock. + * - %LOCK_MAND -- a 'mandatory' flock. + * This exists to emulate Windows Share Modes. * * %LOCK_MAND can be combined with %LOCK_READ or %LOCK_WRITE to allow other * processes read and write access respectively. From a75d30c772078546ac00399a94ecdc82df1a4d72 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sat, 27 May 2017 06:07:19 -0400 Subject: [PATCH 02/13] fs/locks: pass kernel struct flock to fcntl_getlk/setlk This will make it easier to implement a sane compat fcntl syscall. [ jlayton: fix undeclared identifiers in 32-bit fcntl64 syscall handler ] Signed-off-by: Christoph Hellwig Reviewed-by: Jeff Layton Signed-off-by: Jeff Layton --- fs/fcntl.c | 27 +++++++++++++--- fs/locks.c | 79 +++++++++++++--------------------------------- include/linux/fs.h | 8 ++--- 3 files changed, 48 insertions(+), 66 deletions(-) diff --git a/fs/fcntl.c b/fs/fcntl.c index f4e7267d117f..ed4283d500a3 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -246,6 +246,8 @@ static int f_getowner_uids(struct file *filp, unsigned long arg) static long do_fcntl(int fd, unsigned int cmd, unsigned long arg, struct file *filp) { + void __user *argp = (void __user *)arg; + struct flock flock; long err = -EINVAL; switch (cmd) { @@ -273,7 +275,11 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg, case F_OFD_GETLK: #endif case F_GETLK: - err = fcntl_getlk(filp, cmd, (struct flock __user *) arg); + if (copy_from_user(&flock, argp, sizeof(flock))) + return -EFAULT; + err = fcntl_getlk(filp, cmd, &flock); + if (!err && copy_to_user(argp, &flock, sizeof(flock))) + return -EFAULT; break; #if BITS_PER_LONG != 32 /* 32-bit arches must use fcntl64() */ @@ -283,7 +289,9 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg, /* Fallthrough */ case F_SETLK: case F_SETLKW: - err = fcntl_setlk(fd, filp, cmd, (struct flock __user *) arg); + if (copy_from_user(&flock, argp, sizeof(flock))) + return -EFAULT; + err = fcntl_setlk(fd, filp, cmd, &flock); break; case F_GETOWN: /* @@ -383,7 +391,9 @@ out: SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd, unsigned long, arg) { + void __user *argp = (void __user *)arg; struct fd f = fdget_raw(fd); + struct flock64 flock; long err = -EBADF; if (!f.file) @@ -401,14 +411,21 @@ SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd, switch (cmd) { case F_GETLK64: case F_OFD_GETLK: - err = fcntl_getlk64(f.file, cmd, (struct flock64 __user *) arg); + err = -EFAULT; + if (copy_from_user(&flock, argp, sizeof(flock))) + break; + err = fcntl_getlk64(f.file, cmd, &flock); + if (!err && copy_to_user(argp, &flock, sizeof(flock))) + err = -EFAULT; break; case F_SETLK64: case F_SETLKW64: case F_OFD_SETLK: case F_OFD_SETLKW: - err = fcntl_setlk64(fd, f.file, cmd, - (struct flock64 __user *) arg); + err = -EFAULT; + if (copy_from_user(&flock, argp, sizeof(flock))) + break; + err = fcntl_setlk64(fd, f.file, cmd, &flock); break; default: err = do_fcntl(fd, cmd, arg, f.file); diff --git a/fs/locks.c b/fs/locks.c index 4a4543a7f9c1..afefeb4ad6de 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -2084,26 +2084,22 @@ static void posix_lock_to_flock64(struct flock64 *flock, struct file_lock *fl) /* Report the first existing lock that would conflict with l. * This implements the F_GETLK command of fcntl(). */ -int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock __user *l) +int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock) { struct file_lock file_lock; - struct flock flock; int error; - error = -EFAULT; - if (copy_from_user(&flock, l, sizeof(flock))) - goto out; error = -EINVAL; - if ((flock.l_type != F_RDLCK) && (flock.l_type != F_WRLCK)) + if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK) goto out; - error = flock_to_posix_lock(filp, &file_lock, &flock); + error = flock_to_posix_lock(filp, &file_lock, flock); if (error) goto out; if (cmd == F_OFD_GETLK) { error = -EINVAL; - if (flock.l_pid != 0) + if (flock->l_pid != 0) goto out; cmd = F_GETLK; @@ -2115,15 +2111,12 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock __user *l) if (error) goto out; - flock.l_type = file_lock.fl_type; + flock->l_type = file_lock.fl_type; if (file_lock.fl_type != F_UNLCK) { - error = posix_lock_to_flock(&flock, &file_lock); + error = posix_lock_to_flock(flock, &file_lock); if (error) goto rel_priv; } - error = -EFAULT; - if (!copy_to_user(l, &flock, sizeof(flock))) - error = 0; rel_priv: locks_release_private(&file_lock); out: @@ -2216,26 +2209,16 @@ check_fmode_for_setlk(struct file_lock *fl) * This implements both the F_SETLK and F_SETLKW commands of fcntl(). */ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd, - struct flock __user *l) + struct flock *flock) { struct file_lock *file_lock = locks_alloc_lock(); - struct flock flock; - struct inode *inode; + struct inode *inode = locks_inode(filp); struct file *f; int error; if (file_lock == NULL) return -ENOLCK; - inode = locks_inode(filp); - - /* - * This might block, so we do it before checking the inode. - */ - error = -EFAULT; - if (copy_from_user(&flock, l, sizeof(flock))) - goto out; - /* Don't allow mandatory locks on files that may be memory mapped * and shared. */ @@ -2244,7 +2227,7 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd, goto out; } - error = flock_to_posix_lock(filp, file_lock, &flock); + error = flock_to_posix_lock(filp, file_lock, flock); if (error) goto out; @@ -2259,7 +2242,7 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd, switch (cmd) { case F_OFD_SETLK: error = -EINVAL; - if (flock.l_pid != 0) + if (flock->l_pid != 0) goto out; cmd = F_SETLK; @@ -2268,7 +2251,7 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd, break; case F_OFD_SETLKW: error = -EINVAL; - if (flock.l_pid != 0) + if (flock->l_pid != 0) goto out; cmd = F_SETLKW; @@ -2313,26 +2296,22 @@ out: /* Report the first existing lock that would conflict with l. * This implements the F_GETLK command of fcntl(). */ -int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 __user *l) +int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock) { struct file_lock file_lock; - struct flock64 flock; int error; - error = -EFAULT; - if (copy_from_user(&flock, l, sizeof(flock))) - goto out; error = -EINVAL; - if ((flock.l_type != F_RDLCK) && (flock.l_type != F_WRLCK)) + if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK) goto out; - error = flock64_to_posix_lock(filp, &file_lock, &flock); + error = flock64_to_posix_lock(filp, &file_lock, flock); if (error) goto out; if (cmd == F_OFD_GETLK) { error = -EINVAL; - if (flock.l_pid != 0) + if (flock->l_pid != 0) goto out; cmd = F_GETLK64; @@ -2344,13 +2323,9 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 __user *l) if (error) goto out; - flock.l_type = file_lock.fl_type; + flock->l_type = file_lock.fl_type; if (file_lock.fl_type != F_UNLCK) - posix_lock_to_flock64(&flock, &file_lock); - - error = -EFAULT; - if (!copy_to_user(l, &flock, sizeof(flock))) - error = 0; + posix_lock_to_flock64(flock, &file_lock); locks_release_private(&file_lock); out: @@ -2361,26 +2336,16 @@ out: * This implements both the F_SETLK and F_SETLKW commands of fcntl(). */ int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd, - struct flock64 __user *l) + struct flock64 *flock) { struct file_lock *file_lock = locks_alloc_lock(); - struct flock64 flock; - struct inode *inode; + struct inode *inode = locks_inode(filp); struct file *f; int error; if (file_lock == NULL) return -ENOLCK; - /* - * This might block, so we do it before checking the inode. - */ - error = -EFAULT; - if (copy_from_user(&flock, l, sizeof(flock))) - goto out; - - inode = locks_inode(filp); - /* Don't allow mandatory locks on files that may be memory mapped * and shared. */ @@ -2389,7 +2354,7 @@ int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd, goto out; } - error = flock64_to_posix_lock(filp, file_lock, &flock); + error = flock64_to_posix_lock(filp, file_lock, flock); if (error) goto out; @@ -2404,7 +2369,7 @@ int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd, switch (cmd) { case F_OFD_SETLK: error = -EINVAL; - if (flock.l_pid != 0) + if (flock->l_pid != 0) goto out; cmd = F_SETLK64; @@ -2413,7 +2378,7 @@ int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd, break; case F_OFD_SETLKW: error = -EINVAL; - if (flock.l_pid != 0) + if (flock->l_pid != 0) goto out; cmd = F_SETLKW64; diff --git a/include/linux/fs.h b/include/linux/fs.h index 803e5a9b2654..aa4affb38c39 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1038,14 +1038,14 @@ static inline struct inode *locks_inode(const struct file *f) } #ifdef CONFIG_FILE_LOCKING -extern int fcntl_getlk(struct file *, unsigned int, struct flock __user *); +extern int fcntl_getlk(struct file *, unsigned int, struct flock *); extern int fcntl_setlk(unsigned int, struct file *, unsigned int, - struct flock __user *); + struct flock *); #if BITS_PER_LONG == 32 -extern int fcntl_getlk64(struct file *, unsigned int, struct flock64 __user *); +extern int fcntl_getlk64(struct file *, unsigned int, struct flock64 *); extern int fcntl_setlk64(unsigned int, struct file *, unsigned int, - struct flock64 __user *); + struct flock64 *); #endif extern int fcntl_setlease(unsigned int fd, struct file *filp, long arg); From 104289576b33403a2c01097202a07dab74a5c231 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 27 May 2017 00:32:51 -0400 Subject: [PATCH 03/13] drbd: ->sendpage() never needed set_fs() Signed-off-by: Al Viro --- drivers/block/drbd/drbd_main.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c index 84455c365f57..da4944ea974e 100644 --- a/drivers/block/drbd/drbd_main.c +++ b/drivers/block/drbd/drbd_main.c @@ -1550,7 +1550,6 @@ static int _drbd_send_page(struct drbd_peer_device *peer_device, struct page *pa int offset, size_t size, unsigned msg_flags) { struct socket *socket = peer_device->connection->data.socket; - mm_segment_t oldfs = get_fs(); int len = size; int err = -EIO; @@ -1565,7 +1564,6 @@ static int _drbd_send_page(struct drbd_peer_device *peer_device, struct page *pa msg_flags |= MSG_NOSIGNAL; drbd_update_congested(peer_device->connection); - set_fs(KERNEL_DS); do { int sent; @@ -1585,7 +1583,6 @@ static int _drbd_send_page(struct drbd_peer_device *peer_device, struct page *pa len -= sent; offset += sent; } while (len > 0 /* THINK && device->cstate >= C_CONNECTED*/); - set_fs(oldfs); clear_bit(NET_CONGESTED, &peer_device->connection->flags); if (len == 0) { From 4d7edbc34cccfc5a20c9c429c7757c34444a5fe2 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 27 May 2017 16:11:23 -0400 Subject: [PATCH 04/13] nfsd_readlink(): switch to vfs_get_link() Signed-off-by: Al Viro --- fs/nfsd/vfs.c | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 2be32955d7f2..6eef95c585e3 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1464,41 +1464,34 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp, __be32 nfsd_readlink(struct svc_rqst *rqstp, struct svc_fh *fhp, char *buf, int *lenp) { - mm_segment_t oldfs; __be32 err; - int host_err; + const char *link; struct path path; + DEFINE_DELAYED_CALL(done); + int len; err = fh_verify(rqstp, fhp, S_IFLNK, NFSD_MAY_NOP); - if (err) - goto out; + if (unlikely(err)) + return err; path.mnt = fhp->fh_export->ex_path.mnt; path.dentry = fhp->fh_dentry; - err = nfserr_inval; - if (!d_is_symlink(path.dentry)) - goto out; + if (unlikely(!d_is_symlink(path.dentry))) + return nfserr_inval; touch_atime(&path); - /* N.B. Why does this call need a get_fs()?? - * Remove the set_fs and watch the fireworks:-) --okir - */ - oldfs = get_fs(); set_fs(KERNEL_DS); - host_err = vfs_readlink(path.dentry, (char __user *)buf, *lenp); - set_fs(oldfs); + link = vfs_get_link(path.dentry, &done); + if (IS_ERR(link)) + return nfserrno(PTR_ERR(link)); - if (host_err < 0) - goto out_nfserr; - *lenp = host_err; - err = 0; -out: - return err; - -out_nfserr: - err = nfserrno(host_err); - goto out; + len = strlen(link); + if (len < *lenp) + *lenp = len; + memcpy(buf, link, *lenp); + do_delayed_call(&done); + return 0; } /* From 94073ad77fff221b5e66b8b9863a546ba212d6a3 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sat, 27 May 2017 06:07:20 -0400 Subject: [PATCH 05/13] fs/locks: don't mess with the address limit in compat_fcntl64 Instead write a proper compat syscall that calls common helpers. [ jlayton: fix pointer dereferencing in fixup_compat_flock ] Signed-off-by: Christoph Hellwig Reviewed-by: Jeff Layton Signed-off-by: Jeff Layton --- fs/fcntl.c | 118 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 67 insertions(+), 51 deletions(-) diff --git a/fs/fcntl.c b/fs/fcntl.c index ed4283d500a3..bbf80344c125 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -506,76 +506,92 @@ convert_fcntl_cmd(unsigned int cmd) return cmd; } +/* + * GETLK was successful and we need to return the data, but it needs to fit in + * the compat structure. + * l_start shouldn't be too big, unless the original start + end is greater than + * COMPAT_OFF_T_MAX, in which case the app was asking for trouble, so we return + * -EOVERFLOW in that case. l_len could be too big, in which case we just + * truncate it, and only allow the app to see that part of the conflicting lock + * that might make sense to it anyway + */ +static int fixup_compat_flock(struct flock *flock) +{ + if (flock->l_start > COMPAT_OFF_T_MAX) + return -EOVERFLOW; + if (flock->l_len > COMPAT_OFF_T_MAX) + flock->l_len = COMPAT_OFF_T_MAX; + return 0; +} + COMPAT_SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd, compat_ulong_t, arg) { - mm_segment_t old_fs; - struct flock f; - long ret; - unsigned int conv_cmd; + struct fd f = fdget_raw(fd); + struct flock flock; + long err = -EBADF; + + if (!f.file) + return err; + + if (unlikely(f.file->f_mode & FMODE_PATH)) { + if (!check_fcntl_cmd(cmd)) + goto out_put; + } + + err = security_file_fcntl(f.file, cmd, arg); + if (err) + goto out_put; switch (cmd) { case F_GETLK: + err = get_compat_flock(&flock, compat_ptr(arg)); + if (err) + break; + err = fcntl_getlk(f.file, convert_fcntl_cmd(cmd), &flock); + if (err) + break; + err = fixup_compat_flock(&flock); + if (err) + return err; + err = put_compat_flock(&flock, compat_ptr(arg)); + break; + case F_GETLK64: + case F_OFD_GETLK: + err = get_compat_flock64(&flock, compat_ptr(arg)); + if (err) + break; + err = fcntl_getlk(f.file, convert_fcntl_cmd(cmd), &flock); + if (err) + break; + err = fixup_compat_flock(&flock); + if (err) + return err; + err = put_compat_flock64(&flock, compat_ptr(arg)); + break; case F_SETLK: case F_SETLKW: - ret = get_compat_flock(&f, compat_ptr(arg)); - if (ret != 0) + err = get_compat_flock(&flock, compat_ptr(arg)); + if (err) break; - old_fs = get_fs(); - set_fs(KERNEL_DS); - ret = sys_fcntl(fd, cmd, (unsigned long)&f); - set_fs(old_fs); - if (cmd == F_GETLK && ret == 0) { - /* GETLK was successful and we need to return the data... - * but it needs to fit in the compat structure. - * l_start shouldn't be too big, unless the original - * start + end is greater than COMPAT_OFF_T_MAX, in which - * case the app was asking for trouble, so we return - * -EOVERFLOW in that case. - * l_len could be too big, in which case we just truncate it, - * and only allow the app to see that part of the conflicting - * lock that might make sense to it anyway - */ - - if (f.l_start > COMPAT_OFF_T_MAX) - ret = -EOVERFLOW; - if (f.l_len > COMPAT_OFF_T_MAX) - f.l_len = COMPAT_OFF_T_MAX; - if (ret == 0) - ret = put_compat_flock(&f, compat_ptr(arg)); - } + err = fcntl_setlk(fd, f.file, convert_fcntl_cmd(cmd), &flock); break; - - case F_GETLK64: case F_SETLK64: case F_SETLKW64: - case F_OFD_GETLK: case F_OFD_SETLK: case F_OFD_SETLKW: - ret = get_compat_flock64(&f, compat_ptr(arg)); - if (ret != 0) + err = get_compat_flock64(&flock, compat_ptr(arg)); + if (err) break; - old_fs = get_fs(); - set_fs(KERNEL_DS); - conv_cmd = convert_fcntl_cmd(cmd); - ret = sys_fcntl(fd, conv_cmd, (unsigned long)&f); - set_fs(old_fs); - if ((conv_cmd == F_GETLK || conv_cmd == F_OFD_GETLK) && ret == 0) { - /* need to return lock information - see above for commentary */ - if (f.l_start > COMPAT_LOFF_T_MAX) - ret = -EOVERFLOW; - if (f.l_len > COMPAT_LOFF_T_MAX) - f.l_len = COMPAT_LOFF_T_MAX; - if (ret == 0) - ret = put_compat_flock64(&f, compat_ptr(arg)); - } + err = fcntl_setlk(fd, f.file, convert_fcntl_cmd(cmd), &flock); break; - default: - ret = sys_fcntl(fd, cmd, arg); + err = do_fcntl(fd, cmd, arg, f.file); break; } - return ret; +out_put: + fdput(f); + return err; } COMPAT_SYSCALL_DEFINE3(fcntl, unsigned int, fd, unsigned int, cmd, From ae2a9762d6f54331f683464f07498cf66e7c125c Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 3 Jun 2017 21:41:51 -0400 Subject: [PATCH 06/13] compat statfs: switch to copy_to_user() Signed-off-by: Al Viro --- fs/statfs.c | 58 +++++++++++++++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/fs/statfs.c b/fs/statfs.c index 4e4623c7a126..41a6a82da5e2 100644 --- a/fs/statfs.c +++ b/fs/statfs.c @@ -244,6 +244,7 @@ SYSCALL_DEFINE2(ustat, unsigned, dev, struct ustat __user *, ubuf) #ifdef CONFIG_COMPAT static int put_compat_statfs(struct compat_statfs __user *ubuf, struct kstatfs *kbuf) { + struct compat_statfs buf; if (sizeof ubuf->f_blocks == 4) { if ((kbuf->f_blocks | kbuf->f_bfree | kbuf->f_bavail | kbuf->f_bsize | kbuf->f_frsize) & 0xffffffff00000000ULL) @@ -257,20 +258,20 @@ static int put_compat_statfs(struct compat_statfs __user *ubuf, struct kstatfs * && (kbuf->f_ffree & 0xffffffff00000000ULL)) return -EOVERFLOW; } - if (!access_ok(VERIFY_WRITE, ubuf, sizeof(*ubuf)) || - __put_user(kbuf->f_type, &ubuf->f_type) || - __put_user(kbuf->f_bsize, &ubuf->f_bsize) || - __put_user(kbuf->f_blocks, &ubuf->f_blocks) || - __put_user(kbuf->f_bfree, &ubuf->f_bfree) || - __put_user(kbuf->f_bavail, &ubuf->f_bavail) || - __put_user(kbuf->f_files, &ubuf->f_files) || - __put_user(kbuf->f_ffree, &ubuf->f_ffree) || - __put_user(kbuf->f_namelen, &ubuf->f_namelen) || - __put_user(kbuf->f_fsid.val[0], &ubuf->f_fsid.val[0]) || - __put_user(kbuf->f_fsid.val[1], &ubuf->f_fsid.val[1]) || - __put_user(kbuf->f_frsize, &ubuf->f_frsize) || - __put_user(kbuf->f_flags, &ubuf->f_flags) || - __clear_user(ubuf->f_spare, sizeof(ubuf->f_spare))) + memset(&buf, 0, sizeof(struct compat_statfs)); + buf.f_type = kbuf->f_type; + buf.f_bsize = kbuf->f_bsize; + buf.f_blocks = kbuf->f_blocks; + buf.f_bfree = kbuf->f_bfree; + buf.f_bavail = kbuf->f_bavail; + buf.f_files = kbuf->f_files; + buf.f_ffree = kbuf->f_ffree; + buf.f_namelen = kbuf->f_namelen; + buf.f_fsid.val[0] = kbuf->f_fsid.val[0]; + buf.f_fsid.val[1] = kbuf->f_fsid.val[1]; + buf.f_frsize = kbuf->f_frsize; + buf.f_flags = kbuf->f_flags; + if (copy_to_user(ubuf, &buf, sizeof(struct compat_statfs))) return -EFAULT; return 0; } @@ -299,6 +300,7 @@ COMPAT_SYSCALL_DEFINE2(fstatfs, unsigned int, fd, struct compat_statfs __user *, static int put_compat_statfs64(struct compat_statfs64 __user *ubuf, struct kstatfs *kbuf) { + struct compat_statfs64 buf; if (sizeof(ubuf->f_bsize) == 4) { if ((kbuf->f_type | kbuf->f_bsize | kbuf->f_namelen | kbuf->f_frsize | kbuf->f_flags) & 0xffffffff00000000ULL) @@ -312,20 +314,20 @@ static int put_compat_statfs64(struct compat_statfs64 __user *ubuf, struct kstat && (kbuf->f_ffree & 0xffffffff00000000ULL)) return -EOVERFLOW; } - if (!access_ok(VERIFY_WRITE, ubuf, sizeof(*ubuf)) || - __put_user(kbuf->f_type, &ubuf->f_type) || - __put_user(kbuf->f_bsize, &ubuf->f_bsize) || - __put_user(kbuf->f_blocks, &ubuf->f_blocks) || - __put_user(kbuf->f_bfree, &ubuf->f_bfree) || - __put_user(kbuf->f_bavail, &ubuf->f_bavail) || - __put_user(kbuf->f_files, &ubuf->f_files) || - __put_user(kbuf->f_ffree, &ubuf->f_ffree) || - __put_user(kbuf->f_namelen, &ubuf->f_namelen) || - __put_user(kbuf->f_fsid.val[0], &ubuf->f_fsid.val[0]) || - __put_user(kbuf->f_fsid.val[1], &ubuf->f_fsid.val[1]) || - __put_user(kbuf->f_frsize, &ubuf->f_frsize) || - __put_user(kbuf->f_flags, &ubuf->f_flags) || - __clear_user(ubuf->f_spare, sizeof(ubuf->f_spare))) + memset(&buf, 0, sizeof(struct compat_statfs64)); + buf.f_type = kbuf->f_type; + buf.f_bsize = kbuf->f_bsize; + buf.f_blocks = kbuf->f_blocks; + buf.f_bfree = kbuf->f_bfree; + buf.f_bavail = kbuf->f_bavail; + buf.f_files = kbuf->f_files; + buf.f_ffree = kbuf->f_ffree; + buf.f_namelen = kbuf->f_namelen; + buf.f_fsid.val[0] = kbuf->f_fsid.val[0]; + buf.f_fsid.val[1] = kbuf->f_fsid.val[1]; + buf.f_frsize = kbuf->f_frsize; + buf.f_flags = kbuf->f_flags; + if (copy_to_user(ubuf, &buf, sizeof(struct compat_statfs64))) return -EFAULT; return 0; } From 31acd2665b8f45548e1acfb9131657646333f91c Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 3 Jun 2017 21:44:11 -0400 Subject: [PATCH 07/13] isdn: get rid of pointless access_ok() copy_to_user()/copy_from_user()/get_user() check themselves Signed-off-by: Al Viro --- drivers/isdn/i4l/isdn_common.c | 18 ------------------ drivers/isdn/i4l/isdn_ppp.c | 6 ------ drivers/isdn/isdnloop/isdnloop.c | 2 -- 3 files changed, 26 deletions(-) diff --git a/drivers/isdn/i4l/isdn_common.c b/drivers/isdn/i4l/isdn_common.c index 9b856e1890d1..89b09c51ab7c 100644 --- a/drivers/isdn/i4l/isdn_common.c +++ b/drivers/isdn/i4l/isdn_common.c @@ -1304,9 +1304,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg) if (arg) { ulong __user *p = argp; int i; - if (!access_ok(VERIFY_WRITE, p, - sizeof(ulong) * ISDN_MAX_CHANNELS * 2)) - return -EFAULT; for (i = 0; i < ISDN_MAX_CHANNELS; i++) { put_user(dev->ibytes[i], p++); put_user(dev->obytes[i], p++); @@ -1540,11 +1537,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg) char __user *p = argp; int i; - if (!access_ok(VERIFY_WRITE, argp, - (ISDN_MODEM_NUMREG + ISDN_MSNLEN + ISDN_LMSNLEN) - * ISDN_MAX_CHANNELS)) - return -EFAULT; - for (i = 0; i < ISDN_MAX_CHANNELS; i++) { if (copy_to_user(p, dev->mdm.info[i].emu.profile, ISDN_MODEM_NUMREG)) @@ -1567,11 +1559,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg) char __user *p = argp; int i; - if (!access_ok(VERIFY_READ, argp, - (ISDN_MODEM_NUMREG + ISDN_MSNLEN + ISDN_LMSNLEN) - * ISDN_MAX_CHANNELS)) - return -EFAULT; - for (i = 0; i < ISDN_MAX_CHANNELS; i++) { if (copy_from_user(dev->mdm.info[i].emu.profile, p, ISDN_MODEM_NUMREG)) @@ -1617,8 +1604,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg) int j = 0; while (1) { - if (!access_ok(VERIFY_READ, p, 1)) - return -EFAULT; get_user(bname[j], p++); switch (bname[j]) { case '\0': @@ -1685,9 +1670,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg) drvidx = 0; if (drvidx == -1) return -ENODEV; - if (!access_ok(VERIFY_WRITE, argp, - sizeof(isdn_ioctl_struct))) - return -EFAULT; c.driver = drvidx; c.command = ISDN_CMD_IOCTL; c.arg = cmd; diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c index d07dd5196ffc..487478bafaf5 100644 --- a/drivers/isdn/i4l/isdn_ppp.c +++ b/drivers/isdn/i4l/isdn_ppp.c @@ -795,9 +795,6 @@ isdn_ppp_read(int min, struct file *file, char __user *buf, int count) if (!(is->state & IPPP_OPEN)) return 0; - if (!access_ok(VERIFY_WRITE, buf, count)) - return -EFAULT; - spin_lock_irqsave(&is->buflock, flags); b = is->first->next; save_buf = b->buf; @@ -2014,9 +2011,6 @@ isdn_ppp_dev_ioctl_stats(int slot, struct ifreq *ifr, struct net_device *dev) struct ppp_stats t; isdn_net_local *lp = netdev_priv(dev); - if (!access_ok(VERIFY_WRITE, res, sizeof(struct ppp_stats))) - return -EFAULT; - /* build a temporary stat struct and copy it to user space */ memset(&t, 0, sizeof(struct ppp_stats)); diff --git a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c index ef9c8e4f1fa2..32cb0cbd7217 100644 --- a/drivers/isdn/isdnloop/isdnloop.c +++ b/drivers/isdn/isdnloop/isdnloop.c @@ -1142,8 +1142,6 @@ isdnloop_command(isdn_ctrl *c, isdnloop_card *card) case ISDNLOOP_IOCTL_DEBUGVAR: return (ulong) card; case ISDNLOOP_IOCTL_STARTUP: - if (!access_ok(VERIFY_READ, (void *) a, sizeof(isdnloop_sdef))) - return -EFAULT; return isdnloop_start(card, (isdnloop_sdef *) a); break; case ISDNLOOP_IOCTL_ADDCARD: From 5a5011936ec0ef22e9433cf2ee3520db4993b445 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 3 Jun 2017 21:50:46 -0400 Subject: [PATCH 08/13] adb: get rid of pointless access_ok() Signed-off-by: Al Viro --- drivers/macintosh/adb.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/macintosh/adb.c b/drivers/macintosh/adb.c index fee939efc4fc..039dc8285fc5 100644 --- a/drivers/macintosh/adb.c +++ b/drivers/macintosh/adb.c @@ -723,8 +723,6 @@ static ssize_t adb_read(struct file *file, char __user *buf, return -EINVAL; if (count > sizeof(req->reply)) count = sizeof(req->reply); - if (!access_ok(VERIFY_WRITE, buf, count)) - return -EFAULT; req = NULL; spin_lock_irqsave(&state->lock, flags); @@ -781,8 +779,6 @@ static ssize_t adb_write(struct file *file, const char __user *buf, return -EINVAL; if (adb_controller == NULL) return -ENXIO; - if (!access_ok(VERIFY_READ, buf, count)) - return -EFAULT; req = kmalloc(sizeof(struct adb_request), GFP_KERNEL); From 20dcf8e244b963a5c64cdda336d00d5169d17985 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 3 Jun 2017 22:00:37 -0400 Subject: [PATCH 09/13] lpfc debugfs: get rid of pointless access_ok() copy_from_user() needs no protection - it does the checks itself Signed-off-by: Al Viro --- drivers/scsi/lpfc/lpfc_debugfs.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c index fce549a91911..ca08d3730d7b 100644 --- a/drivers/scsi/lpfc/lpfc_debugfs.c +++ b/drivers/scsi/lpfc/lpfc_debugfs.c @@ -1942,10 +1942,6 @@ lpfc_debugfs_nvmestat_write(struct file *file, const char __user *buf, if (nbytes > 64) nbytes = 64; - /* Protect copy from user */ - if (!access_ok(VERIFY_READ, buf, nbytes)) - return -EFAULT; - memset(mybuf, 0, sizeof(mybuf)); if (copy_from_user(mybuf, buf, nbytes)) @@ -2026,10 +2022,6 @@ lpfc_debugfs_nvmektime_write(struct file *file, const char __user *buf, if (nbytes > 64) nbytes = 64; - /* Protect copy from user */ - if (!access_ok(VERIFY_READ, buf, nbytes)) - return -EFAULT; - memset(mybuf, 0, sizeof(mybuf)); if (copy_from_user(mybuf, buf, nbytes)) @@ -2158,10 +2150,6 @@ lpfc_debugfs_nvmeio_trc_write(struct file *file, const char __user *buf, if (nbytes > 64) nbytes = 64; - /* Protect copy from user */ - if (!access_ok(VERIFY_READ, buf, nbytes)) - return -EFAULT; - memset(mybuf, 0, sizeof(mybuf)); if (copy_from_user(mybuf, buf, nbytes)) @@ -2269,10 +2257,6 @@ lpfc_debugfs_cpucheck_write(struct file *file, const char __user *buf, if (nbytes > 64) nbytes = 64; - /* Protect copy from user */ - if (!access_ok(VERIFY_READ, buf, nbytes)) - return -EFAULT; - memset(mybuf, 0, sizeof(mybuf)); if (copy_from_user(mybuf, buf, nbytes)) @@ -2343,10 +2327,6 @@ static int lpfc_idiag_cmd_get(const char __user *buf, size_t nbytes, int i; size_t bsize; - /* Protect copy from user */ - if (!access_ok(VERIFY_READ, buf, nbytes)) - return -EFAULT; - memset(mybuf, 0, sizeof(mybuf)); memset(idiag_cmd, 0, sizeof(*idiag_cmd)); bsize = min(nbytes, (sizeof(mybuf)-1)); From 393cc3f51135ea2520521f776ef3afdf3395c797 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Tue, 13 Jun 2017 13:35:50 +0200 Subject: [PATCH 10/13] fs/fcntl: f_setown, allow returning error Allow f_setown to return an error value. We will fail in the next patch with EINVAL for bad input to f_setown, so tile the path for the later patch. Signed-off-by: Jiri Slaby Reviewed-by: Jeff Layton Cc: Jeff Layton Cc: "J. Bruce Fields" Cc: Alexander Viro Cc: linux-fsdevel@vger.kernel.org Signed-off-by: Jeff Layton --- fs/fcntl.c | 7 ++++--- include/linux/fs.h | 2 +- net/socket.c | 3 +-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/fcntl.c b/fs/fcntl.c index bbf80344c125..313eba860346 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -109,7 +109,7 @@ void __f_setown(struct file *filp, struct pid *pid, enum pid_type type, } EXPORT_SYMBOL(__f_setown); -void f_setown(struct file *filp, unsigned long arg, int force) +int f_setown(struct file *filp, unsigned long arg, int force) { enum pid_type type; struct pid *pid; @@ -123,6 +123,8 @@ void f_setown(struct file *filp, unsigned long arg, int force) pid = find_vpid(who); __f_setown(filp, pid, type, force); rcu_read_unlock(); + + return 0; } EXPORT_SYMBOL(f_setown); @@ -305,8 +307,7 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg, force_successful_syscall_return(); break; case F_SETOWN: - f_setown(filp, arg, 1); - err = 0; + err = f_setown(filp, arg, 1); break; case F_GETOWN_EX: err = f_getown_ex(filp, arg); diff --git a/include/linux/fs.h b/include/linux/fs.h index aa4affb38c39..25ee1ff6d45b 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1249,7 +1249,7 @@ extern void fasync_free(struct fasync_struct *); extern void kill_fasync(struct fasync_struct **, int, int); extern void __f_setown(struct file *filp, struct pid *, enum pid_type, int force); -extern void f_setown(struct file *filp, unsigned long arg, int force); +extern int f_setown(struct file *filp, unsigned long arg, int force); extern void f_delown(struct file *filp); extern pid_t f_getown(struct file *filp); extern int send_sigurg(struct fown_struct *fown); diff --git a/net/socket.c b/net/socket.c index c2564eb25c6b..a30a1e324390 100644 --- a/net/socket.c +++ b/net/socket.c @@ -950,8 +950,7 @@ static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg) err = -EFAULT; if (get_user(pid, (int __user *)argp)) break; - f_setown(sock->file, pid, 1); - err = 0; + err = f_setown(sock->file, pid, 1); break; case FIOGETOWN: case SIOCGPGRP: From fc3dc67471461c0efcb1ed22fb7595121d65fad9 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Tue, 13 Jun 2017 13:35:51 +0200 Subject: [PATCH 11/13] fs/fcntl: f_setown, avoid undefined behaviour fcntl(0, F_SETOWN, 0x80000000) triggers: UBSAN: Undefined behaviour in fs/fcntl.c:118:7 negation of -2147483648 cannot be represented in type 'int': CPU: 1 PID: 18261 Comm: syz-executor Not tainted 4.8.1-0-syzkaller #1 ... Call Trace: ... [] ? f_setown+0x1d8/0x200 [] ? SyS_fcntl+0x999/0xf30 [] ? entry_SYSCALL_64_fastpath+0x23/0xc1 Fix that by checking the arg parameter properly (against INT_MAX) before "who = -who". And return immediatelly with -EINVAL in case it is wrong. Note that according to POSIX we can return EINVAL: http://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html [EINVAL] The cmd argument is F_SETOWN and the value of the argument is not valid as a process or process group identifier. [v2] returns an error, v1 used to fail silently [v3] implement proper check for the bad value INT_MIN Signed-off-by: Jiri Slaby Cc: Jeff Layton Cc: "J. Bruce Fields" Cc: Alexander Viro Cc: linux-fsdevel@vger.kernel.org Signed-off-by: Jeff Layton --- fs/fcntl.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/fcntl.c b/fs/fcntl.c index 313eba860346..693322e28751 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -116,6 +116,10 @@ int f_setown(struct file *filp, unsigned long arg, int force) int who = arg; type = PIDTYPE_PID; if (who < 0) { + /* avoid overflow below */ + if (who == INT_MIN) + return -EINVAL; + type = PIDTYPE_PGID; who = -who; } From f73127356f344483c82632accda2e72b7e0e5f25 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 14 Jun 2017 09:11:54 -0400 Subject: [PATCH 12/13] fs/fcntl: return -ESRCH in f_setown when pid/pgid can't be found The current implementation of F_SETOWN doesn't properly vet the argument passed in and only returns an error if INT_MIN is passed in. If the argument doesn't specify a valid pid/pgid, then we just end up cleaning out the file->f_owner structure. What we really want is to only clean that out only in the case where userland passed in an argument of 0. For anything else, we want to return ESRCH if it doesn't refer to a valid pid. The relevant POSIX spec page is here: http://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html Cc: Jiri Slaby Cc: zhong jiang Signed-off-by: Jeff Layton --- fs/fcntl.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/fs/fcntl.c b/fs/fcntl.c index 693322e28751..afed3b364979 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -112,8 +112,9 @@ EXPORT_SYMBOL(__f_setown); int f_setown(struct file *filp, unsigned long arg, int force) { enum pid_type type; - struct pid *pid; - int who = arg; + struct pid *pid = NULL; + int who = arg, ret = 0; + type = PIDTYPE_PID; if (who < 0) { /* avoid overflow below */ @@ -123,12 +124,19 @@ int f_setown(struct file *filp, unsigned long arg, int force) type = PIDTYPE_PGID; who = -who; } + rcu_read_lock(); - pid = find_vpid(who); - __f_setown(filp, pid, type, force); + if (who) { + pid = find_vpid(who); + if (!pid) + ret = -ESRCH; + } + + if (!ret) + __f_setown(filp, pid, type, force); rcu_read_unlock(); - return 0; + return ret; } EXPORT_SYMBOL(f_setown); From 8c6657cb50cb037ff58b3f6a547c6569568f3527 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 26 Jun 2017 23:51:31 -0400 Subject: [PATCH 13/13] Switch flock copyin/copyout primitives to copy_{from,to}_user() ... and lose HAVE_ARCH_...; if copy_{to,from}_user() on an architecture sucks badly enough to make it a problem, we have a worse problem. Signed-off-by: Al Viro --- fs/fcntl.c | 67 +++++++++++++++++++++++++++--------------------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/fs/fcntl.c b/fs/fcntl.c index afed3b364979..f525bc75c07d 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -452,57 +452,56 @@ out: #endif #ifdef CONFIG_COMPAT +/* careful - don't use anywhere else */ +#define copy_flock_fields(from, to) \ + (to).l_type = (from).l_type; \ + (to).l_whence = (from).l_whence; \ + (to).l_start = (from).l_start; \ + (to).l_len = (from).l_len; \ + (to).l_pid = (from).l_pid; + static int get_compat_flock(struct flock *kfl, struct compat_flock __user *ufl) { - if (!access_ok(VERIFY_READ, ufl, sizeof(*ufl)) || - __get_user(kfl->l_type, &ufl->l_type) || - __get_user(kfl->l_whence, &ufl->l_whence) || - __get_user(kfl->l_start, &ufl->l_start) || - __get_user(kfl->l_len, &ufl->l_len) || - __get_user(kfl->l_pid, &ufl->l_pid)) + struct compat_flock fl; + + if (copy_from_user(&fl, ufl, sizeof(struct compat_flock))) return -EFAULT; + copy_flock_fields(*kfl, fl); + return 0; +} + +static int get_compat_flock64(struct flock *kfl, struct compat_flock64 __user *ufl) +{ + struct compat_flock64 fl; + + if (copy_from_user(&fl, ufl, sizeof(struct compat_flock64))) + return -EFAULT; + copy_flock_fields(*kfl, fl); return 0; } static int put_compat_flock(struct flock *kfl, struct compat_flock __user *ufl) { - if (!access_ok(VERIFY_WRITE, ufl, sizeof(*ufl)) || - __put_user(kfl->l_type, &ufl->l_type) || - __put_user(kfl->l_whence, &ufl->l_whence) || - __put_user(kfl->l_start, &ufl->l_start) || - __put_user(kfl->l_len, &ufl->l_len) || - __put_user(kfl->l_pid, &ufl->l_pid)) + struct compat_flock fl; + + memset(&fl, 0, sizeof(struct compat_flock)); + copy_flock_fields(fl, *kfl); + if (copy_to_user(ufl, &fl, sizeof(struct compat_flock))) return -EFAULT; return 0; } -#ifndef HAVE_ARCH_GET_COMPAT_FLOCK64 -static int get_compat_flock64(struct flock *kfl, struct compat_flock64 __user *ufl) -{ - if (!access_ok(VERIFY_READ, ufl, sizeof(*ufl)) || - __get_user(kfl->l_type, &ufl->l_type) || - __get_user(kfl->l_whence, &ufl->l_whence) || - __get_user(kfl->l_start, &ufl->l_start) || - __get_user(kfl->l_len, &ufl->l_len) || - __get_user(kfl->l_pid, &ufl->l_pid)) - return -EFAULT; - return 0; -} -#endif - -#ifndef HAVE_ARCH_PUT_COMPAT_FLOCK64 static int put_compat_flock64(struct flock *kfl, struct compat_flock64 __user *ufl) { - if (!access_ok(VERIFY_WRITE, ufl, sizeof(*ufl)) || - __put_user(kfl->l_type, &ufl->l_type) || - __put_user(kfl->l_whence, &ufl->l_whence) || - __put_user(kfl->l_start, &ufl->l_start) || - __put_user(kfl->l_len, &ufl->l_len) || - __put_user(kfl->l_pid, &ufl->l_pid)) + struct compat_flock64 fl; + + memset(&fl, 0, sizeof(struct compat_flock64)); + copy_flock_fields(fl, *kfl); + if (copy_to_user(ufl, &fl, sizeof(struct compat_flock64))) return -EFAULT; return 0; } -#endif +#undef copy_flock_fields static unsigned int convert_fcntl_cmd(unsigned int cmd)