* [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() @ 2018-09-12 15:01 Arnd Bergmann 2018-09-12 15:01 ` [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg Arnd Bergmann ` (5 more replies) 0 siblings, 6 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:01 UTC (permalink / raw) To: viro; +Cc: linux-fsdevel, Arnd Bergmann, linux-kernel Many drivers have ioctl() handlers that are completely compatible between 32-bit and 64-bit architectures, except for the argument that is passed down from user space and may have to be passed through compat_ptr() in order to become a valid 64-bit pointer. Using ".compat_ptr=generic_compat_ioctl_ptrarg" in file operations should let us simplify a lot of those drivers to avoid #ifdef checks, and convert additional drivers that don't have proper compat handling yet. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/compat_ioctl.c | 10 ++++++++++ include/linux/fs.h | 7 +++++++ 2 files changed, 17 insertions(+) diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index a9b00942e87d..2d7c7e149083 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -122,6 +122,16 @@ get_user(val, srcptr) || put_user(val, dstptr); \ }) +/* helper function to avoid trivial compat_ioctl() implementations */ +long generic_compat_ioctl_ptrarg(struct file *file, unsigned int cmd, unsigned long arg) +{ + if (!file->f_op->unlocked_ioctl) + return -ENOIOCTLCMD; + + return file->f_op->unlocked_ioctl(file, cmd, (unsigned long)compat_ptr(arg)); +} +EXPORT_SYMBOL_GPL(generic_compat_ioctl_ptrarg); + static int do_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { int err; diff --git a/include/linux/fs.h b/include/linux/fs.h index 33322702c910..18a90aa2dc93 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1643,6 +1643,13 @@ int vfs_mkobj(struct dentry *, umode_t, extern long vfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg); +#ifdef CONFIG_COMPAT +extern long generic_compat_ioctl_ptrarg(struct file *file, unsigned int cmd, + unsigned long arg); +#else +#define generic_compat_ioctl_ptrarg NULL +#endif + /* * VFS file helper functions. */ -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:01 [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() Arnd Bergmann @ 2018-09-12 15:01 ` Arnd Bergmann 2018-09-12 15:33 ` Jason Gunthorpe ` (2 more replies) 2018-09-12 15:01 ` [PATCH v2 03/17] compat_ioctl: use correct compat_ptr() translation in drivers Arnd Bergmann ` (4 subsequent siblings) 5 siblings, 3 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:01 UTC (permalink / raw) To: viro Cc: linux-fsdevel, Arnd Bergmann, Sudip Mukherjee, Greg Kroah-Hartman, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, Stefan Richter, Jiri Kosina, Benjamin Tissoires, Alexander Shishkin, Tomas Winkler, Artem Bityutskiy, Marek Vasut, David S. Miller, Alex Williamson, OGAWA Hirofumi, linux-kernel, linux-integrity, linux1394-devel, linux-usb, linux-input, linux-mtd, netdev, devel, kvm, virtualization Each of these drivers has a copy of the same trivial helper function to convert the pointer argument and then call the native ioctl handler. We now have a generic implementation of that, so use it. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/char/ppdev.c | 12 +--------- drivers/char/tpm/tpm_vtpm_proxy.c | 12 +--------- drivers/firewire/core-cdev.c | 12 +--------- drivers/hid/usbhid/hiddev.c | 11 +-------- drivers/hwtracing/stm/core.c | 12 +--------- drivers/misc/mei/main.c | 22 +---------------- drivers/mtd/ubi/cdev.c | 36 +++------------------------- drivers/net/tap.c | 12 +--------- drivers/staging/pi433/pi433_if.c | 12 +--------- drivers/usb/core/devio.c | 16 +------------ drivers/vfio/vfio.c | 39 +++---------------------------- drivers/vhost/net.c | 12 +--------- drivers/vhost/scsi.c | 12 +--------- drivers/vhost/test.c | 12 +--------- drivers/vhost/vsock.c | 12 +--------- fs/fat/file.c | 13 +---------- 16 files changed, 20 insertions(+), 237 deletions(-) diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c index 1ae77b41050a..c38a62457cf0 100644 --- a/drivers/char/ppdev.c +++ b/drivers/char/ppdev.c @@ -674,14 +674,6 @@ static long pp_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return ret; } -#ifdef CONFIG_COMPAT -static long pp_compat_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) -{ - return pp_ioctl(file, cmd, (unsigned long)compat_ptr(arg)); -} -#endif - static int pp_open(struct inode *inode, struct file *file) { unsigned int minor = iminor(inode); @@ -790,9 +782,7 @@ static const struct file_operations pp_fops = { .write = pp_write, .poll = pp_poll, .unlocked_ioctl = pp_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = pp_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = pp_open, .release = pp_release, }; diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c index 87a0ce47f201..a170f5ca7416 100644 --- a/drivers/char/tpm/tpm_vtpm_proxy.c +++ b/drivers/char/tpm/tpm_vtpm_proxy.c @@ -678,20 +678,10 @@ static long vtpmx_fops_ioctl(struct file *f, unsigned int ioctl, } } -#ifdef CONFIG_COMPAT -static long vtpmx_fops_compat_ioctl(struct file *f, unsigned int ioctl, - unsigned long arg) -{ - return vtpmx_fops_ioctl(f, ioctl, (unsigned long)compat_ptr(arg)); -} -#endif - static const struct file_operations vtpmx_fops = { .owner = THIS_MODULE, .unlocked_ioctl = vtpmx_fops_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = vtpmx_fops_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .llseek = noop_llseek, }; diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index d8e185582642..2acc0c9ddf94 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -1659,14 +1659,6 @@ static long fw_device_op_ioctl(struct file *file, return dispatch_ioctl(file->private_data, cmd, (void __user *)arg); } -#ifdef CONFIG_COMPAT -static long fw_device_op_compat_ioctl(struct file *file, - unsigned int cmd, unsigned long arg) -{ - return dispatch_ioctl(file->private_data, cmd, compat_ptr(arg)); -} -#endif - static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma) { struct client *client = file->private_data; @@ -1808,7 +1800,5 @@ const struct file_operations fw_device_ops = { .mmap = fw_device_op_mmap, .release = fw_device_op_release, .poll = fw_device_op_poll, -#ifdef CONFIG_COMPAT - .compat_ioctl = fw_device_op_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, }; diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c index 23872d08308c..73a168f97024 100644 --- a/drivers/hid/usbhid/hiddev.c +++ b/drivers/hid/usbhid/hiddev.c @@ -845,13 +845,6 @@ static long hiddev_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return r; } -#ifdef CONFIG_COMPAT -static long hiddev_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) -{ - return hiddev_ioctl(file, cmd, (unsigned long)compat_ptr(arg)); -} -#endif - static const struct file_operations hiddev_fops = { .owner = THIS_MODULE, .read = hiddev_read, @@ -861,9 +854,7 @@ static const struct file_operations hiddev_fops = { .release = hiddev_release, .unlocked_ioctl = hiddev_ioctl, .fasync = hiddev_fasync, -#ifdef CONFIG_COMPAT - .compat_ioctl = hiddev_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .llseek = noop_llseek, }; diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c index 10bcb5d73f90..3f5cbb948781 100644 --- a/drivers/hwtracing/stm/core.c +++ b/drivers/hwtracing/stm/core.c @@ -651,23 +651,13 @@ stm_char_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return err; } -#ifdef CONFIG_COMPAT -static long -stm_char_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) -{ - return stm_char_ioctl(file, cmd, (unsigned long)compat_ptr(arg)); -} -#else -#define stm_char_compat_ioctl NULL -#endif - static const struct file_operations stm_fops = { .open = stm_char_open, .release = stm_char_release, .write = stm_char_write, .mmap = stm_char_mmap, .unlocked_ioctl = stm_char_ioctl, - .compat_ioctl = stm_char_compat_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .llseek = no_llseek, }; diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c index 4d77a6ae183a..a26b645e432f 100644 --- a/drivers/misc/mei/main.c +++ b/drivers/misc/mei/main.c @@ -535,24 +535,6 @@ static long mei_ioctl(struct file *file, unsigned int cmd, unsigned long data) return rets; } -/** - * mei_compat_ioctl - the compat IOCTL function - * - * @file: pointer to file structure - * @cmd: ioctl command - * @data: pointer to mei message structure - * - * Return: 0 on success , <0 on error - */ -#ifdef CONFIG_COMPAT -static long mei_compat_ioctl(struct file *file, - unsigned int cmd, unsigned long data) -{ - return mei_ioctl(file, cmd, (unsigned long)compat_ptr(data)); -} -#endif - - /** * mei_poll - the poll function * @@ -855,9 +837,7 @@ static const struct file_operations mei_fops = { .owner = THIS_MODULE, .read = mei_read, .unlocked_ioctl = mei_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = mei_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = mei_open, .release = mei_release, .write = mei_write, diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c index 22547d7a84ea..2aad1da86acc 100644 --- a/drivers/mtd/ubi/cdev.c +++ b/drivers/mtd/ubi/cdev.c @@ -1061,36 +1061,6 @@ static long ctrl_cdev_ioctl(struct file *file, unsigned int cmd, return err; } -#ifdef CONFIG_COMPAT -static long vol_cdev_compat_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) -{ - unsigned long translated_arg = (unsigned long)compat_ptr(arg); - - return vol_cdev_ioctl(file, cmd, translated_arg); -} - -static long ubi_cdev_compat_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) -{ - unsigned long translated_arg = (unsigned long)compat_ptr(arg); - - return ubi_cdev_ioctl(file, cmd, translated_arg); -} - -static long ctrl_cdev_compat_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) -{ - unsigned long translated_arg = (unsigned long)compat_ptr(arg); - - return ctrl_cdev_ioctl(file, cmd, translated_arg); -} -#else -#define vol_cdev_compat_ioctl NULL -#define ubi_cdev_compat_ioctl NULL -#define ctrl_cdev_compat_ioctl NULL -#endif - /* UBI volume character device operations */ const struct file_operations ubi_vol_cdev_operations = { .owner = THIS_MODULE, @@ -1101,7 +1071,7 @@ const struct file_operations ubi_vol_cdev_operations = { .write = vol_cdev_write, .fsync = vol_cdev_fsync, .unlocked_ioctl = vol_cdev_ioctl, - .compat_ioctl = vol_cdev_compat_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; /* UBI character device operations */ @@ -1109,13 +1079,13 @@ const struct file_operations ubi_cdev_operations = { .owner = THIS_MODULE, .llseek = no_llseek, .unlocked_ioctl = ubi_cdev_ioctl, - .compat_ioctl = ubi_cdev_compat_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; /* UBI control character device operations */ const struct file_operations ubi_ctrl_cdev_operations = { .owner = THIS_MODULE, .unlocked_ioctl = ctrl_cdev_ioctl, - .compat_ioctl = ctrl_cdev_compat_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .llseek = no_llseek, }; diff --git a/drivers/net/tap.c b/drivers/net/tap.c index f0f7cd977667..720deb07f2b4 100644 --- a/drivers/net/tap.c +++ b/drivers/net/tap.c @@ -1124,14 +1124,6 @@ static long tap_ioctl(struct file *file, unsigned int cmd, } } -#ifdef CONFIG_COMPAT -static long tap_compat_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) -{ - return tap_ioctl(file, cmd, (unsigned long)compat_ptr(arg)); -} -#endif - static const struct file_operations tap_fops = { .owner = THIS_MODULE, .open = tap_open, @@ -1141,9 +1133,7 @@ static const struct file_operations tap_fops = { .poll = tap_poll, .llseek = no_llseek, .unlocked_ioctl = tap_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = tap_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static int tap_sendmsg(struct socket *sock, struct msghdr *m, diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index c85a805a1243..9e4caf7ad384 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -945,16 +945,6 @@ pi433_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) return retval; } -#ifdef CONFIG_COMPAT -static long -pi433_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) -{ - return pi433_ioctl(filp, cmd, (unsigned long)compat_ptr(arg)); -} -#else -#define pi433_compat_ioctl NULL -#endif /* CONFIG_COMPAT */ - /*-------------------------------------------------------------------------*/ static int pi433_open(struct inode *inode, struct file *filp) @@ -1111,7 +1101,7 @@ static const struct file_operations pi433_fops = { .write = pi433_write, .read = pi433_read, .unlocked_ioctl = pi433_ioctl, - .compat_ioctl = pi433_compat_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = pi433_open, .release = pi433_release, .llseek = no_llseek, diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 6ce77b33da61..269e0befba2d 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -2553,18 +2553,6 @@ static long usbdev_ioctl(struct file *file, unsigned int cmd, return ret; } -#ifdef CONFIG_COMPAT -static long usbdev_compat_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) -{ - int ret; - - ret = usbdev_do_ioctl(file, cmd, compat_ptr(arg)); - - return ret; -} -#endif - /* No kernel lock - fine */ static __poll_t usbdev_poll(struct file *file, struct poll_table_struct *wait) @@ -2588,9 +2576,7 @@ const struct file_operations usbdev_file_operations = { .read = usbdev_read, .poll = usbdev_poll, .unlocked_ioctl = usbdev_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = usbdev_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .mmap = usbdev_mmap, .open = usbdev_open, .release = usbdev_release, diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 64833879f75d..79f08a99602d 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -1200,15 +1200,6 @@ static long vfio_fops_unl_ioctl(struct file *filep, return ret; } -#ifdef CONFIG_COMPAT -static long vfio_fops_compat_ioctl(struct file *filep, - unsigned int cmd, unsigned long arg) -{ - arg = (unsigned long)compat_ptr(arg); - return vfio_fops_unl_ioctl(filep, cmd, arg); -} -#endif /* CONFIG_COMPAT */ - static int vfio_fops_open(struct inode *inode, struct file *filep) { struct vfio_container *container; @@ -1291,9 +1282,7 @@ static const struct file_operations vfio_fops = { .read = vfio_fops_read, .write = vfio_fops_write, .unlocked_ioctl = vfio_fops_unl_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = vfio_fops_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .mmap = vfio_fops_mmap, }; @@ -1572,15 +1561,6 @@ static long vfio_group_fops_unl_ioctl(struct file *filep, return ret; } -#ifdef CONFIG_COMPAT -static long vfio_group_fops_compat_ioctl(struct file *filep, - unsigned int cmd, unsigned long arg) -{ - arg = (unsigned long)compat_ptr(arg); - return vfio_group_fops_unl_ioctl(filep, cmd, arg); -} -#endif /* CONFIG_COMPAT */ - static int vfio_group_fops_open(struct inode *inode, struct file *filep) { struct vfio_group *group; @@ -1636,9 +1616,7 @@ static int vfio_group_fops_release(struct inode *inode, struct file *filep) static const struct file_operations vfio_group_fops = { .owner = THIS_MODULE, .unlocked_ioctl = vfio_group_fops_unl_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = vfio_group_fops_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = vfio_group_fops_open, .release = vfio_group_fops_release, }; @@ -1703,24 +1681,13 @@ static int vfio_device_fops_mmap(struct file *filep, struct vm_area_struct *vma) return device->ops->mmap(device->device_data, vma); } -#ifdef CONFIG_COMPAT -static long vfio_device_fops_compat_ioctl(struct file *filep, - unsigned int cmd, unsigned long arg) -{ - arg = (unsigned long)compat_ptr(arg); - return vfio_device_fops_unl_ioctl(filep, cmd, arg); -} -#endif /* CONFIG_COMPAT */ - static const struct file_operations vfio_device_fops = { .owner = THIS_MODULE, .release = vfio_device_fops_release, .read = vfio_device_fops_read, .write = vfio_device_fops_write, .unlocked_ioctl = vfio_device_fops_unl_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = vfio_device_fops_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .mmap = vfio_device_fops_mmap, }; diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 4e656f89cb22..e9624350f6a5 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -1535,14 +1535,6 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl, } } -#ifdef CONFIG_COMPAT -static long vhost_net_compat_ioctl(struct file *f, unsigned int ioctl, - unsigned long arg) -{ - return vhost_net_ioctl(f, ioctl, (unsigned long)compat_ptr(arg)); -} -#endif - static ssize_t vhost_net_chr_read_iter(struct kiocb *iocb, struct iov_iter *to) { struct file *file = iocb->ki_filp; @@ -1578,9 +1570,7 @@ static const struct file_operations vhost_net_fops = { .write_iter = vhost_net_chr_write_iter, .poll = vhost_net_chr_poll, .unlocked_ioctl = vhost_net_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = vhost_net_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = vhost_net_open, .llseek = noop_llseek, }; diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index c24bb690680b..9180d38de353 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c @@ -1495,21 +1495,11 @@ vhost_scsi_ioctl(struct file *f, } } -#ifdef CONFIG_COMPAT -static long vhost_scsi_compat_ioctl(struct file *f, unsigned int ioctl, - unsigned long arg) -{ - return vhost_scsi_ioctl(f, ioctl, (unsigned long)compat_ptr(arg)); -} -#endif - static const struct file_operations vhost_scsi_fops = { .owner = THIS_MODULE, .release = vhost_scsi_release, .unlocked_ioctl = vhost_scsi_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = vhost_scsi_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = vhost_scsi_open, .llseek = noop_llseek, }; diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c index 40589850eb33..0b185b4712fb 100644 --- a/drivers/vhost/test.c +++ b/drivers/vhost/test.c @@ -298,21 +298,11 @@ static long vhost_test_ioctl(struct file *f, unsigned int ioctl, } } -#ifdef CONFIG_COMPAT -static long vhost_test_compat_ioctl(struct file *f, unsigned int ioctl, - unsigned long arg) -{ - return vhost_test_ioctl(f, ioctl, (unsigned long)compat_ptr(arg)); -} -#endif - static const struct file_operations vhost_test_fops = { .owner = THIS_MODULE, .release = vhost_test_release, .unlocked_ioctl = vhost_test_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = vhost_test_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = vhost_test_open, .llseek = noop_llseek, }; diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c index 34bc3ab40c6d..83c60f3a9c09 100644 --- a/drivers/vhost/vsock.c +++ b/drivers/vhost/vsock.c @@ -699,23 +699,13 @@ static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl, } } -#ifdef CONFIG_COMPAT -static long vhost_vsock_dev_compat_ioctl(struct file *f, unsigned int ioctl, - unsigned long arg) -{ - return vhost_vsock_dev_ioctl(f, ioctl, (unsigned long)compat_ptr(arg)); -} -#endif - static const struct file_operations vhost_vsock_fops = { .owner = THIS_MODULE, .open = vhost_vsock_dev_open, .release = vhost_vsock_dev_release, .llseek = noop_llseek, .unlocked_ioctl = vhost_vsock_dev_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = vhost_vsock_dev_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static struct miscdevice vhost_vsock_misc = { diff --git a/fs/fat/file.c b/fs/fat/file.c index 4f3d72fb1e60..c52c9e9ca36b 100644 --- a/fs/fat/file.c +++ b/fs/fat/file.c @@ -171,15 +171,6 @@ long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) } } -#ifdef CONFIG_COMPAT -static long fat_generic_compat_ioctl(struct file *filp, unsigned int cmd, - unsigned long arg) - -{ - return fat_generic_ioctl(filp, cmd, (unsigned long)compat_ptr(arg)); -} -#endif - static int fat_file_release(struct inode *inode, struct file *filp) { if ((filp->f_mode & FMODE_WRITE) && @@ -209,9 +200,7 @@ const struct file_operations fat_file_operations = { .mmap = generic_file_mmap, .release = fat_file_release, .unlocked_ioctl = fat_generic_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = fat_generic_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .fsync = fat_file_fsync, .splice_read = generic_file_splice_read, .fallocate = fat_fallocate, -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:01 ` [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg Arnd Bergmann @ 2018-09-12 15:33 ` Jason Gunthorpe 2018-09-12 16:20 ` Arnd Bergmann 2018-09-12 18:13 ` Greg Kroah-Hartman 2018-09-16 19:07 ` Jarkko Sakkinen 2 siblings, 1 reply; 48+ messages in thread From: Jason Gunthorpe @ 2018-09-12 15:33 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, Sudip Mukherjee, Greg Kroah-Hartman, Peter Huewe, Jarkko Sakkinen, Stefan Richter, Jiri Kosina, Benjamin Tissoires, Alexander Shishkin, Tomas Winkler, Artem Bityutskiy, Marek Vasut, David S. Miller, Alex Williamson, OGAWA Hirofumi, linux-kernel, linux-integrity, linux1394-devel, linux-usb, linux-input, linux-mtd, netdev, devel, kvm, virtualization On Wed, Sep 12, 2018 at 05:01:03PM +0200, Arnd Bergmann wrote: > Each of these drivers has a copy of the same trivial helper function to > convert the pointer argument and then call the native ioctl handler. > > We now have a generic implementation of that, so use it. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > drivers/char/ppdev.c | 12 +--------- > drivers/char/tpm/tpm_vtpm_proxy.c | 12 +--------- > drivers/firewire/core-cdev.c | 12 +--------- > drivers/hid/usbhid/hiddev.c | 11 +-------- > drivers/hwtracing/stm/core.c | 12 +--------- > drivers/misc/mei/main.c | 22 +---------------- > drivers/mtd/ubi/cdev.c | 36 +++------------------------- > drivers/net/tap.c | 12 +--------- > drivers/staging/pi433/pi433_if.c | 12 +--------- > drivers/usb/core/devio.c | 16 +------------ > drivers/vfio/vfio.c | 39 +++---------------------------- > drivers/vhost/net.c | 12 +--------- > drivers/vhost/scsi.c | 12 +--------- > drivers/vhost/test.c | 12 +--------- > drivers/vhost/vsock.c | 12 +--------- > fs/fat/file.c | 13 +---------- > 16 files changed, 20 insertions(+), 237 deletions(-) > > diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c > index 87a0ce47f201..a170f5ca7416 100644 > +++ b/drivers/char/tpm/tpm_vtpm_proxy.c > @@ -678,20 +678,10 @@ static long vtpmx_fops_ioctl(struct file *f, unsigned int ioctl, > } > } > > -#ifdef CONFIG_COMPAT > -static long vtpmx_fops_compat_ioctl(struct file *f, unsigned int ioctl, > - unsigned long arg) > -{ > - return vtpmx_fops_ioctl(f, ioctl, (unsigned long)compat_ptr(arg)); > -} > -#endif > - > static const struct file_operations vtpmx_fops = { > .owner = THIS_MODULE, > .unlocked_ioctl = vtpmx_fops_ioctl, > -#ifdef CONFIG_COMPAT > - .compat_ioctl = vtpmx_fops_compat_ioctl, > -#endif > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .llseek = noop_llseek, > }; For vtpm: Reviewed-by: Jason Gunthorpe <jgg@mellanox.com> Arnd, would you consider including a patch as part of/after this series to make compat_ioctl in drivers/infiniband/core/uverbs_main.c use this as well? Looks like a bug too? Thanks, Jason ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:33 ` Jason Gunthorpe @ 2018-09-12 16:20 ` Arnd Bergmann 0 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 16:20 UTC (permalink / raw) To: Jason Gunthorpe Cc: Al Viro, Linux FS-devel Mailing List, Sudip Mukherjee, gregkh, Peter Huewe, Jarkko Sakkinen, Stefan Richter, Jiri Kosina, Benjamin Tissoires, Alexander Shishkin, Winkler, Tomas, Artem Bityutskiy, Marek Vasut, David Miller, Alex Williamson, OGAWA Hirofumi, Linux Kernel Mailing List, linux-integrity, linux1394-devel, USB list, open list:HID CORE LAYER, linux-mtd, Networking, driverdevel, kvm, virtualization On Wed, Sep 12, 2018 at 5:33 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > On Wed, Sep 12, 2018 at 05:01:03PM +0200, Arnd Bergmann wrote: > > Each of these drivers has a copy of the same trivial helper function to > > convert the pointer argument and then call the native ioctl handler. > > > > We now have a generic implementation of that, so use it. > > > > For vtpm: > > Reviewed-by: Jason Gunthorpe <jgg@mellanox.com> > > Arnd, would you consider including a patch as part of/after this > series to make compat_ioctl in drivers/infiniband/core/uverbs_main.c > use this as well? Looks like a bug too? That should be included in patch 5 in this series. I may have skipped some Cc there since it had too many recipients (sent only to the mailing lists instead). Arnd ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:01 ` [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg Arnd Bergmann 2018-09-12 15:33 ` Jason Gunthorpe @ 2018-09-12 18:13 ` Greg Kroah-Hartman 2018-09-16 19:07 ` Jarkko Sakkinen 2 siblings, 0 replies; 48+ messages in thread From: Greg Kroah-Hartman @ 2018-09-12 18:13 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, Sudip Mukherjee, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, Stefan Richter, Jiri Kosina, Benjamin Tissoires, Alexander Shishkin, Tomas Winkler, Artem Bityutskiy, Marek Vasut, David S. Miller, Alex Williamson, OGAWA Hirofumi, linux-kernel, linux-integrity, linux1394-devel, linux-usb, linux-input, linux-mtd, netdev, devel, kvm, virtualization On Wed, Sep 12, 2018 at 05:01:03PM +0200, Arnd Bergmann wrote: > Each of these drivers has a copy of the same trivial helper function to > convert the pointer argument and then call the native ioctl handler. > > We now have a generic implementation of that, so use it. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:01 ` [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg Arnd Bergmann 2018-09-12 15:33 ` Jason Gunthorpe 2018-09-12 18:13 ` Greg Kroah-Hartman @ 2018-09-16 19:07 ` Jarkko Sakkinen 2 siblings, 0 replies; 48+ messages in thread From: Jarkko Sakkinen @ 2018-09-16 19:07 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, Sudip Mukherjee, Greg Kroah-Hartman, Peter Huewe, Jason Gunthorpe, Stefan Richter, Jiri Kosina, Benjamin Tissoires, Alexander Shishkin, Tomas Winkler, Artem Bityutskiy, Marek Vasut, David S. Miller, Alex Williamson, OGAWA Hirofumi, linux-kernel, linux-integrity, linux1394-devel, linux-usb, linux-input, linux-mtd, netdev, devel, kvm, virtualization On Wed, Sep 12, 2018 at 05:01:03PM +0200, Arnd Bergmann wrote: > diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c > index 87a0ce47f201..a170f5ca7416 100644 > --- a/drivers/char/tpm/tpm_vtpm_proxy.c > +++ b/drivers/char/tpm/tpm_vtpm_proxy.c > @@ -678,20 +678,10 @@ static long vtpmx_fops_ioctl(struct file *f, unsigned int ioctl, > } > } > > -#ifdef CONFIG_COMPAT > -static long vtpmx_fops_compat_ioctl(struct file *f, unsigned int ioctl, > - unsigned long arg) > -{ > - return vtpmx_fops_ioctl(f, ioctl, (unsigned long)compat_ptr(arg)); > -} > -#endif > - > static const struct file_operations vtpmx_fops = { > .owner = THIS_MODULE, > .unlocked_ioctl = vtpmx_fops_ioctl, > -#ifdef CONFIG_COMPAT > - .compat_ioctl = vtpmx_fops_compat_ioctl, > -#endif > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .llseek = noop_llseek, > }; Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> /Jarkko ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 03/17] compat_ioctl: use correct compat_ptr() translation in drivers 2018-09-12 15:01 [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() Arnd Bergmann 2018-09-12 15:01 ` [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg Arnd Bergmann @ 2018-09-12 15:01 ` Arnd Bergmann 2018-09-12 18:13 ` Greg Kroah-Hartman ` (2 more replies) 2018-09-12 15:01 ` [PATCH v2 04/17] ceph: fix compat_ioctl for ceph_dir_operations Arnd Bergmann ` (3 subsequent siblings) 5 siblings, 3 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:01 UTC (permalink / raw) To: viro Cc: linux-fsdevel, Arnd Bergmann, Frederic Barrat, Andrew Donnellan, Greg Kroah-Hartman, Frank Haverkamp, Guilherme G. Piccoli, Kashyap Desai, Sumit Saxena, Shivasharan S, James E.J. Bottomley, Martin K. Petersen, Felipe Balbi, linuxppc-dev, linux-kernel, megaraidlinux.pdl, linux-scsi, linux-usb A handful of drivers all have a trivial wrapper around their ioctl handler, but don't call the compat_ptr() conversion function at the moment. In practice this does not matter, since none of them are used on the s390 architecture and for all other architectures, compat_ptr() does not do anything, but using the new generic_compat_ioctl_ptrarg helper makes it more correct in theory, and simplifies the code. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/misc/cxl/flash.c | 8 +------- drivers/misc/genwqe/card_dev.c | 23 +---------------------- drivers/scsi/megaraid/megaraid_mm.c | 28 +--------------------------- drivers/usb/gadget/function/f_fs.c | 12 +----------- 4 files changed, 4 insertions(+), 67 deletions(-) diff --git a/drivers/misc/cxl/flash.c b/drivers/misc/cxl/flash.c index 43917898fb9a..acd362498f8c 100644 --- a/drivers/misc/cxl/flash.c +++ b/drivers/misc/cxl/flash.c @@ -473,12 +473,6 @@ static long device_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return -EINVAL; } -static long device_compat_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) -{ - return device_ioctl(file, cmd, arg); -} - static int device_close(struct inode *inode, struct file *file) { struct cxl *adapter = file->private_data; @@ -514,7 +508,7 @@ static const struct file_operations fops = { .owner = THIS_MODULE, .open = device_open, .unlocked_ioctl = device_ioctl, - .compat_ioctl = device_compat_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .release = device_close, }; diff --git a/drivers/misc/genwqe/card_dev.c b/drivers/misc/genwqe/card_dev.c index f453ab82f0d7..6e73df9b3788 100644 --- a/drivers/misc/genwqe/card_dev.c +++ b/drivers/misc/genwqe/card_dev.c @@ -1220,34 +1220,13 @@ static long genwqe_ioctl(struct file *filp, unsigned int cmd, return rc; } -#if defined(CONFIG_COMPAT) -/** - * genwqe_compat_ioctl() - Compatibility ioctl - * - * Called whenever a 32-bit process running under a 64-bit kernel - * performs an ioctl on /dev/genwqe<n>_card. - * - * @filp: file pointer. - * @cmd: command. - * @arg: user argument. - * Return: zero on success or negative number on failure. - */ -static long genwqe_compat_ioctl(struct file *filp, unsigned int cmd, - unsigned long arg) -{ - return genwqe_ioctl(filp, cmd, arg); -} -#endif /* defined(CONFIG_COMPAT) */ - static const struct file_operations genwqe_fops = { .owner = THIS_MODULE, .open = genwqe_open, .fasync = genwqe_fasync, .mmap = genwqe_mmap, .unlocked_ioctl = genwqe_ioctl, -#if defined(CONFIG_COMPAT) - .compat_ioctl = genwqe_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .release = genwqe_release, }; diff --git a/drivers/scsi/megaraid/megaraid_mm.c b/drivers/scsi/megaraid/megaraid_mm.c index 8428247015db..00daa1547783 100644 --- a/drivers/scsi/megaraid/megaraid_mm.c +++ b/drivers/scsi/megaraid/megaraid_mm.c @@ -45,10 +45,6 @@ static int mraid_mm_setup_dma_pools(mraid_mmadp_t *); static void mraid_mm_free_adp_resources(mraid_mmadp_t *); static void mraid_mm_teardown_dma_pools(mraid_mmadp_t *); -#ifdef CONFIG_COMPAT -static long mraid_mm_compat_ioctl(struct file *, unsigned int, unsigned long); -#endif - MODULE_AUTHOR("LSI Logic Corporation"); MODULE_DESCRIPTION("LSI Logic Management Module"); MODULE_LICENSE("GPL"); @@ -72,9 +68,7 @@ static wait_queue_head_t wait_q; static const struct file_operations lsi_fops = { .open = mraid_mm_open, .unlocked_ioctl = mraid_mm_unlocked_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = mraid_mm_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .owner = THIS_MODULE, .llseek = noop_llseek, }; @@ -228,7 +222,6 @@ mraid_mm_unlocked_ioctl(struct file *filep, unsigned int cmd, { int err; - /* inconsistent: mraid_mm_compat_ioctl doesn't take the BKL */ mutex_lock(&mraid_mm_mutex); err = mraid_mm_ioctl(filep, cmd, arg); mutex_unlock(&mraid_mm_mutex); @@ -1233,25 +1226,6 @@ mraid_mm_init(void) } -#ifdef CONFIG_COMPAT -/** - * mraid_mm_compat_ioctl - 32bit to 64bit ioctl conversion routine - * @filep : file operations pointer (ignored) - * @cmd : ioctl command - * @arg : user ioctl packet - */ -static long -mraid_mm_compat_ioctl(struct file *filep, unsigned int cmd, - unsigned long arg) -{ - int err; - - err = mraid_mm_ioctl(filep, cmd, arg); - - return err; -} -#endif - /** * mraid_mm_exit - Module exit point */ diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 3ada83d81bda..f4af64b4cb36 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -1276,14 +1276,6 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code, return ret; } -#ifdef CONFIG_COMPAT -static long ffs_epfile_compat_ioctl(struct file *file, unsigned code, - unsigned long value) -{ - return ffs_epfile_ioctl(file, code, value); -} -#endif - static const struct file_operations ffs_epfile_operations = { .llseek = no_llseek, @@ -1292,9 +1284,7 @@ static const struct file_operations ffs_epfile_operations = { .read_iter = ffs_epfile_read_iter, .release = ffs_epfile_release, .unlocked_ioctl = ffs_epfile_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = ffs_epfile_compat_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, }; -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v2 03/17] compat_ioctl: use correct compat_ptr() translation in drivers 2018-09-12 15:01 ` [PATCH v2 03/17] compat_ioctl: use correct compat_ptr() translation in drivers Arnd Bergmann @ 2018-09-12 18:13 ` Greg Kroah-Hartman 2018-09-13 0:48 ` Andrew Donnellan 2018-09-13 11:03 ` Felipe Balbi 2 siblings, 0 replies; 48+ messages in thread From: Greg Kroah-Hartman @ 2018-09-12 18:13 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, Frederic Barrat, Andrew Donnellan, Frank Haverkamp, Guilherme G. Piccoli, Kashyap Desai, Sumit Saxena, Shivasharan S, James E.J. Bottomley, Martin K. Petersen, Felipe Balbi, linuxppc-dev, linux-kernel, megaraidlinux.pdl, linux-scsi, linux-usb On Wed, Sep 12, 2018 at 05:01:04PM +0200, Arnd Bergmann wrote: > A handful of drivers all have a trivial wrapper around their ioctl > handler, but don't call the compat_ptr() conversion function at the > moment. In practice this does not matter, since none of them are used > on the s390 architecture and for all other architectures, compat_ptr() > does not do anything, but using the new generic_compat_ioctl_ptrarg > helper makes it more correct in theory, and simplifies the code. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/misc/cxl/flash.c | 8 +------- > drivers/misc/genwqe/card_dev.c | 23 +---------------------- > drivers/scsi/megaraid/megaraid_mm.c | 28 +--------------------------- > drivers/usb/gadget/function/f_fs.c | 12 +----------- > 4 files changed, 4 insertions(+), 67 deletions(-) Nice cleanup on this series! Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 03/17] compat_ioctl: use correct compat_ptr() translation in drivers 2018-09-12 15:01 ` [PATCH v2 03/17] compat_ioctl: use correct compat_ptr() translation in drivers Arnd Bergmann 2018-09-12 18:13 ` Greg Kroah-Hartman @ 2018-09-13 0:48 ` Andrew Donnellan 2018-09-13 11:03 ` Felipe Balbi 2 siblings, 0 replies; 48+ messages in thread From: Andrew Donnellan @ 2018-09-13 0:48 UTC (permalink / raw) To: Arnd Bergmann, viro Cc: linux-fsdevel, Frederic Barrat, Greg Kroah-Hartman, Frank Haverkamp, Guilherme G. Piccoli, Kashyap Desai, Sumit Saxena, Shivasharan S, James E.J. Bottomley, Martin K. Petersen, Felipe Balbi, linuxppc-dev, linux-kernel, megaraidlinux.pdl, linux-scsi, linux-usb On 13/09/18 01:01, Arnd Bergmann wrote: > A handful of drivers all have a trivial wrapper around their ioctl > handler, but don't call the compat_ptr() conversion function at the > moment. In practice this does not matter, since none of them are used > on the s390 architecture and for all other architectures, compat_ptr() > does not do anything, but using the new generic_compat_ioctl_ptrarg > helper makes it more correct in theory, and simplifies the code. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> For cxl: Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnellan@au1.ibm.com IBM Australia Limited ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 03/17] compat_ioctl: use correct compat_ptr() translation in drivers 2018-09-12 15:01 ` [PATCH v2 03/17] compat_ioctl: use correct compat_ptr() translation in drivers Arnd Bergmann 2018-09-12 18:13 ` Greg Kroah-Hartman 2018-09-13 0:48 ` Andrew Donnellan @ 2018-09-13 11:03 ` Felipe Balbi 2 siblings, 0 replies; 48+ messages in thread From: Felipe Balbi @ 2018-09-13 11:03 UTC (permalink / raw) To: Arnd Bergmann, viro Cc: linux-fsdevel, Arnd Bergmann, Frederic Barrat, Andrew Donnellan, Greg Kroah-Hartman, Frank Haverkamp, Guilherme G. Piccoli, Kashyap Desai, Sumit Saxena, Shivasharan S, James E.J. Bottomley, Martin K. Petersen, linuxppc-dev, linux-kernel, megaraidlinux.pdl, linux-scsi, linux-usb [-- Attachment #1: Type: text/plain, Size: 590 bytes --] Arnd Bergmann <arnd@arndb.de> writes: > A handful of drivers all have a trivial wrapper around their ioctl > handler, but don't call the compat_ptr() conversion function at the > moment. In practice this does not matter, since none of them are used > on the s390 architecture and for all other architectures, compat_ptr() > does not do anything, but using the new generic_compat_ioctl_ptrarg > helper makes it more correct in theory, and simplifies the code. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com> -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 04/17] ceph: fix compat_ioctl for ceph_dir_operations 2018-09-12 15:01 [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() Arnd Bergmann 2018-09-12 15:01 ` [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg Arnd Bergmann 2018-09-12 15:01 ` [PATCH v2 03/17] compat_ioctl: use correct compat_ptr() translation in drivers Arnd Bergmann @ 2018-09-12 15:01 ` Arnd Bergmann 2018-09-12 16:12 ` David Laight 2018-09-13 0:48 ` Yan, Zheng 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann ` (2 subsequent siblings) 5 siblings, 2 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:01 UTC (permalink / raw) To: viro Cc: linux-fsdevel, Arnd Bergmann, stable, Yan, Zheng, Sage Weil, Ilya Dryomov, Chengguang Xu, ceph-devel, linux-kernel The ceph_ioctl function is used both for files and directories, but only the files support doing that in 32-bit compat mode. For consistency, add the same compat handler to the dir operations as well. Cc: stable@vger.kernel.org Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/ceph/dir.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c index 82928cea0209..da73f29d7faa 100644 --- a/fs/ceph/dir.c +++ b/fs/ceph/dir.c @@ -1489,6 +1489,7 @@ const struct file_operations ceph_dir_fops = { .open = ceph_open, .release = ceph_release, .unlocked_ioctl = ceph_ioctl, + .compat_ioctl = ceph_ioctl, .fsync = ceph_fsync, .lock = ceph_lock, .flock = ceph_flock, -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* RE: [PATCH v2 04/17] ceph: fix compat_ioctl for ceph_dir_operations 2018-09-12 15:01 ` [PATCH v2 04/17] ceph: fix compat_ioctl for ceph_dir_operations Arnd Bergmann @ 2018-09-12 16:12 ` David Laight 2018-09-12 16:25 ` Arnd Bergmann 2018-09-13 0:48 ` Yan, Zheng 1 sibling, 1 reply; 48+ messages in thread From: David Laight @ 2018-09-12 16:12 UTC (permalink / raw) To: 'Arnd Bergmann', viro Cc: linux-fsdevel, stable, Yan, Zheng, Sage Weil, Ilya Dryomov, Chengguang Xu, ceph-devel, linux-kernel From: Arnd Bergmann > Sent: 12 September 2018 16:01 > > The ceph_ioctl function is used both for files and directories, but only > the files support doing that in 32-bit compat mode. > > For consistency, add the same compat handler to the dir operations > as well. Have you verified that all the relevant ioctl buffer structures are exactly the same for 32bit and 64bit applications? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 04/17] ceph: fix compat_ioctl for ceph_dir_operations 2018-09-12 16:12 ` David Laight @ 2018-09-12 16:25 ` Arnd Bergmann 0 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 16:25 UTC (permalink / raw) To: David Laight Cc: Al Viro, Linux FS-devel Mailing List, # 3.4.x, Zheng Yan, Sage Weil, Ilya Dryomov, Chengguang Xu, ceph-devel, Linux Kernel Mailing List On Wed, Sep 12, 2018 at 6:10 PM David Laight <David.Laight@aculab.com> wrote: > > From: Arnd Bergmann > > Sent: 12 September 2018 16:01 > > > > The ceph_ioctl function is used both for files and directories, but only > > the files support doing that in 32-bit compat mode. > > > > For consistency, add the same compat handler to the dir operations > > as well. > > Have you verified that all the relevant ioctl buffer structures are > exactly the same for 32bit and 64bit applications? I checked it now, it's fine: there are only ceph_ioctl_dataloc and ceph_ioctl_layout structures passed here, both of which are compatible. I assumed that the ceph_dir_fops operations were correct here (they are), but you are right that I should have double checked for more bugs as I encountered one of them. Arnd ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 04/17] ceph: fix compat_ioctl for ceph_dir_operations 2018-09-12 15:01 ` [PATCH v2 04/17] ceph: fix compat_ioctl for ceph_dir_operations Arnd Bergmann 2018-09-12 16:12 ` David Laight @ 2018-09-13 0:48 ` Yan, Zheng 1 sibling, 0 replies; 48+ messages in thread From: Yan, Zheng @ 2018-09-13 0:48 UTC (permalink / raw) To: Arnd Bergmann Cc: Al Viro, Linux FS-devel Mailing List, stable, Zheng Yan, Sage Weil, Ilya Dryomov, Chengguang Xu, ceph-devel, Linux Kernel Mailing List On Wed, Sep 12, 2018 at 11:10 PM Arnd Bergmann <arnd@arndb.de> wrote: > > The ceph_ioctl function is used both for files and directories, but only > the files support doing that in 32-bit compat mode. > > For consistency, add the same compat handler to the dir operations > as well. > > Cc: stable@vger.kernel.org > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > fs/ceph/dir.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c > index 82928cea0209..da73f29d7faa 100644 > --- a/fs/ceph/dir.c > +++ b/fs/ceph/dir.c > @@ -1489,6 +1489,7 @@ const struct file_operations ceph_dir_fops = { > .open = ceph_open, > .release = ceph_release, > .unlocked_ioctl = ceph_ioctl, > + .compat_ioctl = ceph_ioctl, > .fsync = ceph_fsync, > .lock = ceph_lock, > .flock = ceph_flock, > -- > 2.18.0 > Reviewed-by: "Yan, Zheng" <zyan@redhat.com> ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:01 [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() Arnd Bergmann ` (2 preceding siblings ...) 2018-09-12 15:01 ` [PATCH v2 04/17] ceph: fix compat_ioctl for ceph_dir_operations Arnd Bergmann @ 2018-09-12 15:08 ` Arnd Bergmann 2018-09-12 15:08 ` [PATCH v2 06/17] compat_ioctl: move rtc handling into rtc-dev.c Arnd Bergmann ` (11 more replies) 2018-09-12 15:13 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Arnd Bergmann 2018-09-13 2:07 ` [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() Al Viro 5 siblings, 12 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:08 UTC (permalink / raw) To: viro Cc: linux-fsdevel, Arnd Bergmann, Greg Kroah-Hartman, David S. Miller, devel, linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, netdev The .ioctl and .compat_ioctl file operations have the same prototype so they can both point to the same function, which works great almost all the time when all the commands are compatible. One exception is the s390 architecture, where a compat pointer is only 31 bit wide, and converting it into a 64-bit pointer requires calling compat_ptr(). Most drivers here will ever run in s390, but since we now have a generic helper for it, it's easy enough to use it consistently. I double-checked all these drivers to ensure that all ioctl arguments are used as pointers or are ignored, but are not interpreted as integer values. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/android/binder.c | 2 +- drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +- drivers/dma-buf/dma-buf.c | 4 +--- drivers/dma-buf/sw_sync.c | 2 +- drivers/dma-buf/sync_file.c | 2 +- drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +- drivers/hid/hidraw.c | 4 +--- drivers/iio/industrialio-core.c | 2 +- drivers/infiniband/core/uverbs_main.c | 4 ++-- drivers/media/rc/lirc_dev.c | 4 +--- drivers/mfd/cros_ec_dev.c | 4 +--- drivers/misc/vmw_vmci/vmci_host.c | 2 +- drivers/nvdimm/bus.c | 4 ++-- drivers/nvme/host/core.c | 2 +- drivers/pci/switch/switchtec.c | 2 +- drivers/platform/x86/wmi.c | 2 +- drivers/rpmsg/rpmsg_char.c | 4 ++-- drivers/sbus/char/display7seg.c | 2 +- drivers/sbus/char/envctrl.c | 4 +--- drivers/scsi/3w-xxxx.c | 4 +--- drivers/scsi/cxlflash/main.c | 2 +- drivers/scsi/esas2r/esas2r_main.c | 2 +- drivers/scsi/pmcraid.c | 4 +--- drivers/staging/android/ion/ion.c | 4 +--- drivers/staging/vme/devices/vme_user.c | 2 +- drivers/tee/tee_core.c | 2 +- drivers/usb/class/cdc-wdm.c | 2 +- drivers/usb/class/usbtmc.c | 4 +--- drivers/video/fbdev/ps3fb.c | 2 +- drivers/virt/fsl_hypervisor.c | 2 +- fs/btrfs/super.c | 2 +- fs/ceph/dir.c | 2 +- fs/ceph/file.c | 2 +- fs/fuse/dev.c | 2 +- fs/notify/fanotify/fanotify_user.c | 2 +- fs/userfaultfd.c | 2 +- net/rfkill/core.c | 2 +- 37 files changed, 40 insertions(+), 58 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index d58763b6b009..d2464f5759f8 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -5576,7 +5576,7 @@ static const struct file_operations binder_fops = { .owner = THIS_MODULE, .poll = binder_poll, .unlocked_ioctl = binder_ioctl, - .compat_ioctl = binder_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .mmap = binder_mmap, .open = binder_open, .flush = binder_flush, diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c b/drivers/crypto/qat/qat_common/adf_ctl_drv.c index abc7a7f64d64..8ff77a70addc 100644 --- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c +++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c @@ -68,7 +68,7 @@ static long adf_ctl_ioctl(struct file *fp, unsigned int cmd, unsigned long arg); static const struct file_operations adf_ctl_ops = { .owner = THIS_MODULE, .unlocked_ioctl = adf_ctl_ioctl, - .compat_ioctl = adf_ctl_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; struct adf_ctl_drv_info { diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 13884474d158..a6d7dc4cf7e9 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -325,9 +325,7 @@ static const struct file_operations dma_buf_fops = { .llseek = dma_buf_llseek, .poll = dma_buf_poll, .unlocked_ioctl = dma_buf_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = dma_buf_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, }; /* diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 53c1d6d36a64..bc810506d487 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -419,5 +419,5 @@ const struct file_operations sw_sync_debugfs_fops = { .open = sw_sync_debugfs_open, .release = sw_sync_debugfs_release, .unlocked_ioctl = sw_sync_ioctl, - .compat_ioctl = sw_sync_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 35dd06479867..1c64ed60c658 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -488,5 +488,5 @@ static const struct file_operations sync_file_fops = { .release = sync_file_release, .poll = sync_file_poll, .unlocked_ioctl = sync_file_ioctl, - .compat_ioctl = sync_file_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c index 297b36c26a05..1d7b1e3c3ebe 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c @@ -47,7 +47,7 @@ static const char kfd_dev_name[] = "kfd"; static const struct file_operations kfd_fops = { .owner = THIS_MODULE, .unlocked_ioctl = kfd_ioctl, - .compat_ioctl = kfd_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = kfd_open, .mmap = kfd_mmap, }; diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c index 4a44e48e08b2..e44b64812850 100644 --- a/drivers/hid/hidraw.c +++ b/drivers/hid/hidraw.c @@ -476,9 +476,7 @@ static const struct file_operations hidraw_ops = { .release = hidraw_release, .unlocked_ioctl = hidraw_ioctl, .fasync = hidraw_fasync, -#ifdef CONFIG_COMPAT - .compat_ioctl = hidraw_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .llseek = noop_llseek, }; diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index a062cfddc5af..22844b94b0e9 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -1630,7 +1630,7 @@ static const struct file_operations iio_buffer_fileops = { .owner = THIS_MODULE, .llseek = noop_llseek, .unlocked_ioctl = iio_ioctl, - .compat_ioctl = iio_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static int iio_check_unique_scan_index(struct iio_dev *indio_dev) diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c index 823beca448e1..f4755c1c9cfa 100644 --- a/drivers/infiniband/core/uverbs_main.c +++ b/drivers/infiniband/core/uverbs_main.c @@ -930,7 +930,7 @@ static const struct file_operations uverbs_fops = { .release = ib_uverbs_close, .llseek = no_llseek, .unlocked_ioctl = ib_uverbs_ioctl, - .compat_ioctl = ib_uverbs_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static const struct file_operations uverbs_mmap_fops = { @@ -941,7 +941,7 @@ static const struct file_operations uverbs_mmap_fops = { .release = ib_uverbs_close, .llseek = no_llseek, .unlocked_ioctl = ib_uverbs_ioctl, - .compat_ioctl = ib_uverbs_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static struct ib_client uverbs_client = { diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index f862f1b7f996..077209f414ed 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -730,9 +730,7 @@ static const struct file_operations lirc_fops = { .owner = THIS_MODULE, .write = ir_lirc_transmit_ir, .unlocked_ioctl = ir_lirc_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = ir_lirc_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .read = ir_lirc_read, .poll = ir_lirc_poll, .open = ir_lirc_open, diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c index 999dac752bcc..35a04bcf55da 100644 --- a/drivers/mfd/cros_ec_dev.c +++ b/drivers/mfd/cros_ec_dev.c @@ -258,9 +258,7 @@ static const struct file_operations fops = { .release = ec_device_release, .read = ec_device_read, .unlocked_ioctl = ec_device_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = ec_device_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static void cros_ec_sensors_register(struct cros_ec_dev *ec) diff --git a/drivers/misc/vmw_vmci/vmci_host.c b/drivers/misc/vmw_vmci/vmci_host.c index 83e0c95d20a4..1308f889e53b 100644 --- a/drivers/misc/vmw_vmci/vmci_host.c +++ b/drivers/misc/vmw_vmci/vmci_host.c @@ -983,7 +983,7 @@ static const struct file_operations vmuser_fops = { .release = vmci_host_close, .poll = vmci_host_poll, .unlocked_ioctl = vmci_host_unlocked_ioctl, - .compat_ioctl = vmci_host_unlocked_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static struct miscdevice vmci_host_miscdev = { diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c index 8aae6dcc839f..7449cbc55df7 100644 --- a/drivers/nvdimm/bus.c +++ b/drivers/nvdimm/bus.c @@ -1133,7 +1133,7 @@ static const struct file_operations nvdimm_bus_fops = { .owner = THIS_MODULE, .open = nd_open, .unlocked_ioctl = nd_ioctl, - .compat_ioctl = nd_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .llseek = noop_llseek, }; @@ -1141,7 +1141,7 @@ static const struct file_operations nvdimm_fops = { .owner = THIS_MODULE, .open = nd_open, .unlocked_ioctl = nvdimm_ioctl, - .compat_ioctl = nvdimm_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .llseek = noop_llseek, }; diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index dd8ec1dd9219..2d986f573a29 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2579,7 +2579,7 @@ static const struct file_operations nvme_dev_fops = { .owner = THIS_MODULE, .open = nvme_dev_open, .unlocked_ioctl = nvme_dev_ioctl, - .compat_ioctl = nvme_dev_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static ssize_t nvme_sysfs_reset(struct device *dev, diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c index 9940cc70f38b..4296919c784e 100644 --- a/drivers/pci/switch/switchtec.c +++ b/drivers/pci/switch/switchtec.c @@ -967,7 +967,7 @@ static const struct file_operations switchtec_fops = { .read = switchtec_dev_read, .poll = switchtec_dev_poll, .unlocked_ioctl = switchtec_dev_ioctl, - .compat_ioctl = switchtec_dev_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static void link_event_work(struct work_struct *work) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 04791ea5d97b..e4d0697e07d6 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -886,7 +886,7 @@ static const struct file_operations wmi_fops = { .read = wmi_char_read, .open = wmi_char_open, .unlocked_ioctl = wmi_ioctl, - .compat_ioctl = wmi_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static int wmi_dev_probe(struct device *dev) diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c index a76b963a7e50..02aefb2b2d47 100644 --- a/drivers/rpmsg/rpmsg_char.c +++ b/drivers/rpmsg/rpmsg_char.c @@ -285,7 +285,7 @@ static const struct file_operations rpmsg_eptdev_fops = { .write = rpmsg_eptdev_write, .poll = rpmsg_eptdev_poll, .unlocked_ioctl = rpmsg_eptdev_ioctl, - .compat_ioctl = rpmsg_eptdev_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static ssize_t name_show(struct device *dev, struct device_attribute *attr, @@ -446,7 +446,7 @@ static const struct file_operations rpmsg_ctrldev_fops = { .open = rpmsg_ctrldev_open, .release = rpmsg_ctrldev_release, .unlocked_ioctl = rpmsg_ctrldev_ioctl, - .compat_ioctl = rpmsg_ctrldev_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static void rpmsg_ctrldev_release_device(struct device *dev) diff --git a/drivers/sbus/char/display7seg.c b/drivers/sbus/char/display7seg.c index 5c8ed7350a04..064fe7247eb2 100644 --- a/drivers/sbus/char/display7seg.c +++ b/drivers/sbus/char/display7seg.c @@ -155,7 +155,7 @@ static long d7s_ioctl(struct file *file, unsigned int cmd, unsigned long arg) static const struct file_operations d7s_fops = { .owner = THIS_MODULE, .unlocked_ioctl = d7s_ioctl, - .compat_ioctl = d7s_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = d7s_open, .release = d7s_release, .llseek = noop_llseek, diff --git a/drivers/sbus/char/envctrl.c b/drivers/sbus/char/envctrl.c index 56e962a01493..a26665ccea56 100644 --- a/drivers/sbus/char/envctrl.c +++ b/drivers/sbus/char/envctrl.c @@ -714,9 +714,7 @@ static const struct file_operations envctrl_fops = { .owner = THIS_MODULE, .read = envctrl_read, .unlocked_ioctl = envctrl_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = envctrl_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = envctrl_open, .release = envctrl_release, .llseek = noop_llseek, diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c index 471366945bd4..86c9f22a152f 100644 --- a/drivers/scsi/3w-xxxx.c +++ b/drivers/scsi/3w-xxxx.c @@ -1047,9 +1047,7 @@ static int tw_chrdev_open(struct inode *inode, struct file *file) static const struct file_operations tw_fops = { .owner = THIS_MODULE, .unlocked_ioctl = tw_chrdev_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = tw_chrdev_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = tw_chrdev_open, .release = NULL, .llseek = noop_llseek, diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index 6637116529aa..d968efeb50e8 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -3596,7 +3596,7 @@ static const struct file_operations cxlflash_chr_fops = { .owner = THIS_MODULE, .open = cxlflash_chr_open, .unlocked_ioctl = cxlflash_chr_ioctl, - .compat_ioctl = cxlflash_chr_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; /** diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c index c07118617d89..95142292e702 100644 --- a/drivers/scsi/esas2r/esas2r_main.c +++ b/drivers/scsi/esas2r/esas2r_main.c @@ -614,7 +614,7 @@ static int __init esas2r_init(void) /* Handle ioctl calls to "/proc/scsi/esas2r/ATTOnode" */ static const struct file_operations esas2r_proc_fops = { - .compat_ioctl = esas2r_proc_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .unlocked_ioctl = esas2r_proc_ioctl, }; diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c index 4e86994e10e8..8a8c73d3bdad 100644 --- a/drivers/scsi/pmcraid.c +++ b/drivers/scsi/pmcraid.c @@ -3999,9 +3999,7 @@ static const struct file_operations pmcraid_fops = { .open = pmcraid_chr_open, .fasync = pmcraid_chr_fasync, .unlocked_ioctl = pmcraid_chr_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = pmcraid_chr_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .llseek = noop_llseek, }; diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 99073325b0c0..ef727c235392 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -484,9 +484,7 @@ int ion_query_heaps(struct ion_heap_query *query) static const struct file_operations ion_fops = { .owner = THIS_MODULE, .unlocked_ioctl = ion_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = ion_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static int debug_shrink_set(void *data, u64 val) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index 6a33aaa1a49f..568700ffd2f2 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -494,7 +494,7 @@ static const struct file_operations vme_user_fops = { .write = vme_user_write, .llseek = vme_user_llseek, .unlocked_ioctl = vme_user_unlocked_ioctl, - .compat_ioctl = vme_user_unlocked_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .mmap = vme_user_mmap, }; diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index dd46b758852a..cb79f28be894 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -670,7 +670,7 @@ static const struct file_operations tee_fops = { .open = tee_open, .release = tee_release, .unlocked_ioctl = tee_ioctl, - .compat_ioctl = tee_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static void tee_release_device(struct device *dev) diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index bec581fb7c63..6e4998c8e64f 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -724,7 +724,7 @@ static const struct file_operations wdm_fops = { .release = wdm_release, .poll = wdm_poll, .unlocked_ioctl = wdm_ioctl, - .compat_ioctl = wdm_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .llseek = noop_llseek, }; diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c index 83ffa5a14c3d..d5da47c4c462 100644 --- a/drivers/usb/class/usbtmc.c +++ b/drivers/usb/class/usbtmc.c @@ -1460,9 +1460,7 @@ static const struct file_operations fops = { .open = usbtmc_open, .release = usbtmc_release, .unlocked_ioctl = usbtmc_ioctl, -#ifdef CONFIG_COMPAT - .compat_ioctl = usbtmc_ioctl, -#endif + .compat_ioctl = generic_compat_ioctl_ptrarg, .fasync = usbtmc_fasync, .poll = usbtmc_poll, .llseek = default_llseek, diff --git a/drivers/video/fbdev/ps3fb.c b/drivers/video/fbdev/ps3fb.c index 5ed2db39d823..f9f8ffaf1c4a 100644 --- a/drivers/video/fbdev/ps3fb.c +++ b/drivers/video/fbdev/ps3fb.c @@ -949,7 +949,7 @@ static struct fb_ops ps3fb_ops = { .fb_mmap = ps3fb_mmap, .fb_blank = ps3fb_blank, .fb_ioctl = ps3fb_ioctl, - .fb_compat_ioctl = ps3fb_ioctl + .fb_compat_ioctl = generic_compat_ioctl_ptrarg, }; static const struct fb_fix_screeninfo ps3fb_fix = { diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c index 8ba726e600e9..406b7e492214 100644 --- a/drivers/virt/fsl_hypervisor.c +++ b/drivers/virt/fsl_hypervisor.c @@ -703,7 +703,7 @@ static const struct file_operations fsl_hv_fops = { .poll = fsl_hv_poll, .read = fsl_hv_read, .unlocked_ioctl = fsl_hv_ioctl, - .compat_ioctl = fsl_hv_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; static struct miscdevice fsl_hv_misc_dev = { diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 6601c9aa5e35..2b5a8ad86305 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -2352,7 +2352,7 @@ static const struct super_operations btrfs_super_ops = { static const struct file_operations btrfs_ctl_fops = { .open = btrfs_control_open, .unlocked_ioctl = btrfs_control_ioctl, - .compat_ioctl = btrfs_control_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .owner = THIS_MODULE, .llseek = noop_llseek, }; diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c index da73f29d7faa..eb869fe6774d 100644 --- a/fs/ceph/dir.c +++ b/fs/ceph/dir.c @@ -1489,7 +1489,7 @@ const struct file_operations ceph_dir_fops = { .open = ceph_open, .release = ceph_release, .unlocked_ioctl = ceph_ioctl, - .compat_ioctl = ceph_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .fsync = ceph_fsync, .lock = ceph_lock, .flock = ceph_flock, diff --git a/fs/ceph/file.c b/fs/ceph/file.c index 92ab20433682..85094042cfac 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -1842,7 +1842,7 @@ const struct file_operations ceph_file_fops = { .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .unlocked_ioctl = ceph_ioctl, - .compat_ioctl = ceph_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .fallocate = ceph_fallocate, }; diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 11ea2c4a38ab..a6d4a24963ed 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -2258,7 +2258,7 @@ const struct file_operations fuse_dev_operations = { .release = fuse_dev_release, .fasync = fuse_dev_fasync, .unlocked_ioctl = fuse_dev_ioctl, - .compat_ioctl = fuse_dev_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, }; EXPORT_SYMBOL_GPL(fuse_dev_operations); diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 69054886915b..fc4193b384cf 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -447,7 +447,7 @@ static const struct file_operations fanotify_fops = { .fasync = NULL, .release = fanotify_release, .unlocked_ioctl = fanotify_ioctl, - .compat_ioctl = fanotify_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .llseek = noop_llseek, }; diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index bfa0ec69f924..bc9118b58a8a 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -1878,7 +1878,7 @@ static const struct file_operations userfaultfd_fops = { .poll = userfaultfd_poll, .read = userfaultfd_read, .unlocked_ioctl = userfaultfd_ioctl, - .compat_ioctl = userfaultfd_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .llseek = noop_llseek, }; diff --git a/net/rfkill/core.c b/net/rfkill/core.c index 1355f5ca8d22..ba68b53f58ab 100644 --- a/net/rfkill/core.c +++ b/net/rfkill/core.c @@ -1323,7 +1323,7 @@ static const struct file_operations rfkill_fops = { .release = rfkill_fop_release, #ifdef CONFIG_RFKILL_INPUT .unlocked_ioctl = rfkill_fop_ioctl, - .compat_ioctl = rfkill_fop_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, #endif .llseek = no_llseek, }; -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 06/17] compat_ioctl: move rtc handling into rtc-dev.c 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann @ 2018-09-12 15:08 ` Arnd Bergmann 2018-09-12 20:00 ` Alexandre Belloni 2018-09-12 15:08 ` [PATCH v2 07/17] compat_ioctl: move tape handling into drivers Arnd Bergmann ` (10 subsequent siblings) 11 siblings, 1 reply; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:08 UTC (permalink / raw) To: viro Cc: linux-fsdevel, Arnd Bergmann, Alessandro Zummo, Alexandre Belloni, linux-rtc, linux-kernel We no longer need the rtc compat handling to be in common code, now that all drivers are either moved to the rtc-class framework, or (rarely) exist in drivers/char for architectures without compat mode (m68k, alpha and ia64, respectively). I checked the list of ioctl commands in drivers, and the ones that are not already handled are all compatible, again with the one exception of m68k driver, which implements RTC_PLL_GET and RTC_PLL_SET, but has no compat mode. Since the ioctl commands are either compatible or differ in both structure and command code between 32-bit and 64-bit, we can merge the compat handler into the native one and just implement the two common compat commands (RTC_IRQP_READ, RTC_IRQP_SET) there. The old conversion handler also deals with RTC_EPOCH_READ and RTC_EPOCH_SET, which are not handled in rtc-dev.c but only in a single device driver (rtc-vr41xx), so I'm adding the compat version in the same place. I don't expect other drivers to need those commands in the future. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- v2: merge compat handler into ioctl function to avoid the compat_alloc_user_space() roundtrip, based on feedback from Al Viro. --- drivers/rtc/rtc-dev.c | 13 +++++++++- drivers/rtc/rtc-vr41xx.c | 10 ++++++++ fs/compat_ioctl.c | 54 ---------------------------------------- 3 files changed, 22 insertions(+), 55 deletions(-) diff --git a/drivers/rtc/rtc-dev.c b/drivers/rtc/rtc-dev.c index 43d962a9c210..ccd5b1795e20 100644 --- a/drivers/rtc/rtc-dev.c +++ b/drivers/rtc/rtc-dev.c @@ -13,6 +13,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/compat.h> #include <linux/module.h> #include <linux/rtc.h> #include <linux/sched/signal.h> @@ -364,10 +365,19 @@ static long rtc_dev_ioctl(struct file *file, mutex_unlock(&rtc->ops_lock); return rtc_update_irq_enable(rtc, 0); +#ifdef CONFIG_COMPAT +#define RTC_IRQP_SET32 _IOW('p', 0x0c, compat_ulong_t) +#define RTC_IRQP_READ32 _IOR('p', 0x0b, compat_ulong_t) + case RTC_IRQP_SET32: + err = rtc_irq_set_freq(rtc, arg); + break; + case RTC_IRQP_READ32: + err = put_user(rtc->irq_freq, (unsigned int __user *)uarg); + break; +#endif case RTC_IRQP_SET: err = rtc_irq_set_freq(rtc, arg); break; - case RTC_IRQP_READ: err = put_user(rtc->irq_freq, (unsigned long __user *)uarg); break; @@ -439,6 +449,7 @@ static const struct file_operations rtc_dev_fops = { .read = rtc_dev_read, .poll = rtc_dev_poll, .unlocked_ioctl = rtc_dev_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = rtc_dev_open, .release = rtc_dev_release, .fasync = rtc_dev_fasync, diff --git a/drivers/rtc/rtc-vr41xx.c b/drivers/rtc/rtc-vr41xx.c index 70f013e692b0..1d90bde59d21 100644 --- a/drivers/rtc/rtc-vr41xx.c +++ b/drivers/rtc/rtc-vr41xx.c @@ -17,6 +17,7 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +#include <linux/compat.h> #include <linux/err.h> #include <linux/fs.h> #include <linux/init.h> @@ -79,6 +80,10 @@ static void __iomem *rtc2_base; #define rtc2_read(offset) readw(rtc2_base + (offset)) #define rtc2_write(offset, value) writew((value), rtc2_base + (offset)) +/* 32-bit compat for ioctls that nobody else uses */ +#define RTC_EPOCH_READ32 _IOR('p', 0x0d, compat_ulong_t) +#define RTC_EPOCH_SET32 _IOW('p', 0x0e, compat_ulong_t) + static unsigned long epoch = 1970; /* Jan 1 1970 00:00:00 */ static DEFINE_SPINLOCK(rtc_lock); @@ -195,6 +200,11 @@ static int vr41xx_rtc_ioctl(struct device *dev, unsigned int cmd, unsigned long switch (cmd) { case RTC_EPOCH_READ: return put_user(epoch, (unsigned long __user *)arg); +#ifdef CONFIG_COMPAT + case RTC_EPOCH_READ32: + return put_user(epoch, (unsigned int __user *)arg); + case RTC_EPOCH_SET32: +#endif case RTC_EPOCH_SET: /* Doesn't support before 1900 */ if (arg < 1900) diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index 2d7c7e149083..312b52b4e974 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -46,7 +46,6 @@ #include <linux/raw.h> #include <linux/blkdev.h> #include <linux/elevator.h> -#include <linux/rtc.h> #include <linux/pci.h> #include <linux/serial.h> #include <linux/if_tun.h> @@ -633,37 +632,6 @@ static int serial_struct_ioctl(struct file *file, return err; } -#define RTC_IRQP_READ32 _IOR('p', 0x0b, compat_ulong_t) -#define RTC_IRQP_SET32 _IOW('p', 0x0c, compat_ulong_t) -#define RTC_EPOCH_READ32 _IOR('p', 0x0d, compat_ulong_t) -#define RTC_EPOCH_SET32 _IOW('p', 0x0e, compat_ulong_t) - -static int rtc_ioctl(struct file *file, - unsigned cmd, void __user *argp) -{ - unsigned long __user *valp = compat_alloc_user_space(sizeof(*valp)); - int ret; - - if (valp == NULL) - return -EFAULT; - switch (cmd) { - case RTC_IRQP_READ32: - case RTC_EPOCH_READ32: - ret = do_ioctl(file, (cmd == RTC_IRQP_READ32) ? - RTC_IRQP_READ : RTC_EPOCH_READ, - (unsigned long)valp); - if (ret) - return ret; - return convert_in_user(valp, (unsigned int __user *)argp); - case RTC_IRQP_SET32: - return do_ioctl(file, RTC_IRQP_SET, (unsigned long)argp); - case RTC_EPOCH_SET32: - return do_ioctl(file, RTC_EPOCH_SET, (unsigned long)argp); - } - - return -ENOIOCTLCMD; -} - /* on ia32 l_start is on a 32-bit boundary */ #if defined(CONFIG_IA64) || defined(CONFIG_X86_64) struct space_resv_32 { @@ -816,21 +784,6 @@ COMPATIBLE_IOCTL(SCSI_IOCTL_GET_PCI) /* Big V (don't complain on serial console) */ IGNORE_IOCTL(VT_OPENQRY) IGNORE_IOCTL(VT_GETMODE) -/* Little p (/dev/rtc, /dev/envctrl, etc.) */ -COMPATIBLE_IOCTL(RTC_AIE_ON) -COMPATIBLE_IOCTL(RTC_AIE_OFF) -COMPATIBLE_IOCTL(RTC_UIE_ON) -COMPATIBLE_IOCTL(RTC_UIE_OFF) -COMPATIBLE_IOCTL(RTC_PIE_ON) -COMPATIBLE_IOCTL(RTC_PIE_OFF) -COMPATIBLE_IOCTL(RTC_WIE_ON) -COMPATIBLE_IOCTL(RTC_WIE_OFF) -COMPATIBLE_IOCTL(RTC_ALM_SET) -COMPATIBLE_IOCTL(RTC_ALM_READ) -COMPATIBLE_IOCTL(RTC_RD_TIME) -COMPATIBLE_IOCTL(RTC_SET_TIME) -COMPATIBLE_IOCTL(RTC_WKALM_SET) -COMPATIBLE_IOCTL(RTC_WKALM_RD) /* * These two are only for the sbus rtc driver, but * hwclock tries them on every rtc device first when @@ -1307,13 +1260,6 @@ static long do_ioctl_trans(unsigned int cmd, case TIOCGSERIAL: case TIOCSSERIAL: return serial_struct_ioctl(file, cmd, argp); - /* Not implemented in the native kernel */ - case RTC_IRQP_READ32: - case RTC_IRQP_SET32: - case RTC_EPOCH_READ32: - case RTC_EPOCH_SET32: - return rtc_ioctl(file, cmd, argp); - /* dvb */ case VIDEO_GET_EVENT: return do_video_get_event(file, cmd, argp); -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v2 06/17] compat_ioctl: move rtc handling into rtc-dev.c 2018-09-12 15:08 ` [PATCH v2 06/17] compat_ioctl: move rtc handling into rtc-dev.c Arnd Bergmann @ 2018-09-12 20:00 ` Alexandre Belloni 0 siblings, 0 replies; 48+ messages in thread From: Alexandre Belloni @ 2018-09-12 20:00 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, Alessandro Zummo, linux-rtc, linux-kernel On 12/09/2018 17:08:53+0200, Arnd Bergmann wrote: > We no longer need the rtc compat handling to be in common code, now that > all drivers are either moved to the rtc-class framework, or (rarely) > exist in drivers/char for architectures without compat mode (m68k, > alpha and ia64, respectively). > > I checked the list of ioctl commands in drivers, and the ones that are > not already handled are all compatible, again with the one exception of > m68k driver, which implements RTC_PLL_GET and RTC_PLL_SET, but has no > compat mode. > > Since the ioctl commands are either compatible or differ in both structure > and command code between 32-bit and 64-bit, we can merge the compat > handler into the native one and just implement the two common compat > commands (RTC_IRQP_READ, RTC_IRQP_SET) there. > > The old conversion handler also deals with RTC_EPOCH_READ and > RTC_EPOCH_SET, which are not handled in rtc-dev.c but only in > a single device driver (rtc-vr41xx), so I'm adding the compat > version in the same place. I don't expect other drivers to need > those commands in the future. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > --- > v2: merge compat handler into ioctl function to avoid the > compat_alloc_user_space() roundtrip, based on feedback > from Al Viro. > --- > drivers/rtc/rtc-dev.c | 13 +++++++++- > drivers/rtc/rtc-vr41xx.c | 10 ++++++++ > fs/compat_ioctl.c | 54 ---------------------------------------- > 3 files changed, 22 insertions(+), 55 deletions(-) > > diff --git a/drivers/rtc/rtc-dev.c b/drivers/rtc/rtc-dev.c > index 43d962a9c210..ccd5b1795e20 100644 > --- a/drivers/rtc/rtc-dev.c > +++ b/drivers/rtc/rtc-dev.c > @@ -13,6 +13,7 @@ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > +#include <linux/compat.h> > #include <linux/module.h> > #include <linux/rtc.h> > #include <linux/sched/signal.h> > @@ -364,10 +365,19 @@ static long rtc_dev_ioctl(struct file *file, > mutex_unlock(&rtc->ops_lock); > return rtc_update_irq_enable(rtc, 0); > > +#ifdef CONFIG_COMPAT > +#define RTC_IRQP_SET32 _IOW('p', 0x0c, compat_ulong_t) > +#define RTC_IRQP_READ32 _IOR('p', 0x0b, compat_ulong_t) > + case RTC_IRQP_SET32: > + err = rtc_irq_set_freq(rtc, arg); > + break; > + case RTC_IRQP_READ32: > + err = put_user(rtc->irq_freq, (unsigned int __user *)uarg); > + break; > +#endif > case RTC_IRQP_SET: > err = rtc_irq_set_freq(rtc, arg); > break; > - > case RTC_IRQP_READ: > err = put_user(rtc->irq_freq, (unsigned long __user *)uarg); > break; > @@ -439,6 +449,7 @@ static const struct file_operations rtc_dev_fops = { > .read = rtc_dev_read, > .poll = rtc_dev_poll, > .unlocked_ioctl = rtc_dev_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .open = rtc_dev_open, > .release = rtc_dev_release, > .fasync = rtc_dev_fasync, > diff --git a/drivers/rtc/rtc-vr41xx.c b/drivers/rtc/rtc-vr41xx.c > index 70f013e692b0..1d90bde59d21 100644 > --- a/drivers/rtc/rtc-vr41xx.c > +++ b/drivers/rtc/rtc-vr41xx.c > @@ -17,6 +17,7 @@ > * along with this program; if not, write to the Free Software > * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > */ > +#include <linux/compat.h> > #include <linux/err.h> > #include <linux/fs.h> > #include <linux/init.h> > @@ -79,6 +80,10 @@ static void __iomem *rtc2_base; > #define rtc2_read(offset) readw(rtc2_base + (offset)) > #define rtc2_write(offset, value) writew((value), rtc2_base + (offset)) > > +/* 32-bit compat for ioctls that nobody else uses */ > +#define RTC_EPOCH_READ32 _IOR('p', 0x0d, compat_ulong_t) > +#define RTC_EPOCH_SET32 _IOW('p', 0x0e, compat_ulong_t) > + > static unsigned long epoch = 1970; /* Jan 1 1970 00:00:00 */ > > static DEFINE_SPINLOCK(rtc_lock); > @@ -195,6 +200,11 @@ static int vr41xx_rtc_ioctl(struct device *dev, unsigned int cmd, unsigned long > switch (cmd) { > case RTC_EPOCH_READ: > return put_user(epoch, (unsigned long __user *)arg); > +#ifdef CONFIG_COMPAT > + case RTC_EPOCH_READ32: > + return put_user(epoch, (unsigned int __user *)arg); > + case RTC_EPOCH_SET32: > +#endif > case RTC_EPOCH_SET: > /* Doesn't support before 1900 */ > if (arg < 1900) > diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c > index 2d7c7e149083..312b52b4e974 100644 > --- a/fs/compat_ioctl.c > +++ b/fs/compat_ioctl.c > @@ -46,7 +46,6 @@ > #include <linux/raw.h> > #include <linux/blkdev.h> > #include <linux/elevator.h> > -#include <linux/rtc.h> > #include <linux/pci.h> > #include <linux/serial.h> > #include <linux/if_tun.h> > @@ -633,37 +632,6 @@ static int serial_struct_ioctl(struct file *file, > return err; > } > > -#define RTC_IRQP_READ32 _IOR('p', 0x0b, compat_ulong_t) > -#define RTC_IRQP_SET32 _IOW('p', 0x0c, compat_ulong_t) > -#define RTC_EPOCH_READ32 _IOR('p', 0x0d, compat_ulong_t) > -#define RTC_EPOCH_SET32 _IOW('p', 0x0e, compat_ulong_t) > - > -static int rtc_ioctl(struct file *file, > - unsigned cmd, void __user *argp) > -{ > - unsigned long __user *valp = compat_alloc_user_space(sizeof(*valp)); > - int ret; > - > - if (valp == NULL) > - return -EFAULT; > - switch (cmd) { > - case RTC_IRQP_READ32: > - case RTC_EPOCH_READ32: > - ret = do_ioctl(file, (cmd == RTC_IRQP_READ32) ? > - RTC_IRQP_READ : RTC_EPOCH_READ, > - (unsigned long)valp); > - if (ret) > - return ret; > - return convert_in_user(valp, (unsigned int __user *)argp); > - case RTC_IRQP_SET32: > - return do_ioctl(file, RTC_IRQP_SET, (unsigned long)argp); > - case RTC_EPOCH_SET32: > - return do_ioctl(file, RTC_EPOCH_SET, (unsigned long)argp); > - } > - > - return -ENOIOCTLCMD; > -} > - > /* on ia32 l_start is on a 32-bit boundary */ > #if defined(CONFIG_IA64) || defined(CONFIG_X86_64) > struct space_resv_32 { > @@ -816,21 +784,6 @@ COMPATIBLE_IOCTL(SCSI_IOCTL_GET_PCI) > /* Big V (don't complain on serial console) */ > IGNORE_IOCTL(VT_OPENQRY) > IGNORE_IOCTL(VT_GETMODE) > -/* Little p (/dev/rtc, /dev/envctrl, etc.) */ > -COMPATIBLE_IOCTL(RTC_AIE_ON) > -COMPATIBLE_IOCTL(RTC_AIE_OFF) > -COMPATIBLE_IOCTL(RTC_UIE_ON) > -COMPATIBLE_IOCTL(RTC_UIE_OFF) > -COMPATIBLE_IOCTL(RTC_PIE_ON) > -COMPATIBLE_IOCTL(RTC_PIE_OFF) > -COMPATIBLE_IOCTL(RTC_WIE_ON) > -COMPATIBLE_IOCTL(RTC_WIE_OFF) > -COMPATIBLE_IOCTL(RTC_ALM_SET) > -COMPATIBLE_IOCTL(RTC_ALM_READ) > -COMPATIBLE_IOCTL(RTC_RD_TIME) > -COMPATIBLE_IOCTL(RTC_SET_TIME) > -COMPATIBLE_IOCTL(RTC_WKALM_SET) > -COMPATIBLE_IOCTL(RTC_WKALM_RD) > /* > * These two are only for the sbus rtc driver, but > * hwclock tries them on every rtc device first when > @@ -1307,13 +1260,6 @@ static long do_ioctl_trans(unsigned int cmd, > case TIOCGSERIAL: > case TIOCSSERIAL: > return serial_struct_ioctl(file, cmd, argp); > - /* Not implemented in the native kernel */ > - case RTC_IRQP_READ32: > - case RTC_IRQP_SET32: > - case RTC_EPOCH_READ32: > - case RTC_EPOCH_SET32: > - return rtc_ioctl(file, cmd, argp); > - > /* dvb */ > case VIDEO_GET_EVENT: > return do_video_get_event(file, cmd, argp); > -- > 2.18.0 > -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 07/17] compat_ioctl: move tape handling into drivers 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann 2018-09-12 15:08 ` [PATCH v2 06/17] compat_ioctl: move rtc handling into rtc-dev.c Arnd Bergmann @ 2018-09-12 15:08 ` Arnd Bergmann 2018-09-12 15:08 ` [PATCH v2 08/17] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann ` (9 subsequent siblings) 11 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:08 UTC (permalink / raw) To: viro Cc: linux-fsdevel, Arnd Bergmann, David S. Miller, Martin Schwidefsky, Heiko Carstens, Willem Riede, James E.J. Bottomley, Martin K. Petersen, Kai Mäkisara, Jens Axboe, Hannes Reinecke, Colin Ian King, linux-kernel, linux-ide, linux-s390, osst-users, linux-scsi MTIOCPOS and MTIOCGET are incompatible between 32-bit and 64-bit user space, and traditionally have been translated in fs/compat_ioctl.c. To get rid of that translation handler, move a corresponding implementation into each of the four drivers implementing those commands. The interesting part of that is now in a new linux/mtio.h header that wraps the existing uapi/linux/mtio.h header and provides an abstraction to let drivers handle both cases easily. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/ide/ide-tape.c | 31 +++++++++++---- drivers/s390/char/tape_char.c | 41 ++++++++------------ drivers/scsi/osst.c | 34 ++++++++++------- drivers/scsi/st.c | 35 ++++++++++------- fs/compat_ioctl.c | 71 ----------------------------------- include/linux/mtio.h | 58 ++++++++++++++++++++++++++++ 6 files changed, 137 insertions(+), 133 deletions(-) create mode 100644 include/linux/mtio.h diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 34c1165226a4..137febf3658d 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -19,6 +19,7 @@ #define IDETAPE_VERSION "1.20" +#include <linux/compat.h> #include <linux/module.h> #include <linux/types.h> #include <linux/string.h> @@ -1368,7 +1369,7 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count) * ide-tape ioctls are supported on both interfaces. */ static long do_idetape_chrdev_ioctl(struct file *file, - unsigned int cmd, unsigned long arg) + unsigned int cmd, unsigned long arg, bool compat) { struct ide_tape_obj *tape = file->private_data; ide_drive_t *drive = tape->drive; @@ -1407,14 +1408,10 @@ static long do_idetape_chrdev_ioctl(struct file *file, if (tape->drv_write_prot) mtget.mt_gstat |= GMT_WR_PROT(0xffffffff); - if (copy_to_user(argp, &mtget, sizeof(struct mtget))) - return -EFAULT; - return 0; + return put_user_mtget(argp, &mtget, compat); case MTIOCPOS: mtpos.mt_blkno = position / tape->user_bs_factor - block_offset; - if (copy_to_user(argp, &mtpos, sizeof(struct mtpos))) - return -EFAULT; - return 0; + return put_user_mtpos(argp, &mtpos, compat); default: if (tape->chrdev_dir == IDETAPE_DIR_READ) ide_tape_discard_merge_buffer(drive, 1); @@ -1427,7 +1424,23 @@ static long idetape_chrdev_ioctl(struct file *file, { long ret; mutex_lock(&ide_tape_mutex); - ret = do_idetape_chrdev_ioctl(file, cmd, arg); + ret = do_idetape_chrdev_ioctl(file, cmd, arg, false); + mutex_unlock(&ide_tape_mutex); + return ret; +} + +static long idetape_chrdev_compat_ioctl(struct file *file, + unsigned int cmd, unsigned long arg) +{ + long ret; + + if (cmd == MTIOCPOS32) + cmd = MTIOCPOS; + else if (cmd == MTIOCGET32) + cmd = MTIOCGET; + + mutex_lock(&ide_tape_mutex); + ret = do_idetape_chrdev_ioctl(file, cmd, arg, true); mutex_unlock(&ide_tape_mutex); return ret; } @@ -1886,6 +1899,8 @@ static const struct file_operations idetape_fops = { .read = idetape_chrdev_read, .write = idetape_chrdev_write, .unlocked_ioctl = idetape_chrdev_ioctl, + .compat_ioctl = IS_ENABLED(CONFIG_COMPAT) ? + idetape_chrdev_compat_ioctl : NULL, .open = idetape_chrdev_open, .release = idetape_chrdev_release, .llseek = noop_llseek, diff --git a/drivers/s390/char/tape_char.c b/drivers/s390/char/tape_char.c index fc206c9d1c56..522ca9b836e3 100644 --- a/drivers/s390/char/tape_char.c +++ b/drivers/s390/char/tape_char.c @@ -341,14 +341,14 @@ tapechar_release(struct inode *inode, struct file *filp) */ static int __tapechar_ioctl(struct tape_device *device, - unsigned int no, unsigned long data) + unsigned int no, void __user *data, bool compat) { int rc; if (no == MTIOCTOP) { struct mtop op; - if (copy_from_user(&op, (char __user *) data, sizeof(op)) != 0) + if (copy_from_user(&op, data, sizeof(op)) != 0) return -EFAULT; if (op.mt_count < 0) return -EINVAL; @@ -392,9 +392,7 @@ __tapechar_ioctl(struct tape_device *device, if (rc < 0) return rc; pos.mt_blkno = rc; - if (copy_to_user((char __user *) data, &pos, sizeof(pos)) != 0) - return -EFAULT; - return 0; + return put_user_mtpos(data, &pos, compat); } if (no == MTIOCGET) { /* MTIOCGET: query the tape drive status. */ @@ -424,15 +422,12 @@ __tapechar_ioctl(struct tape_device *device, get.mt_blkno = rc; } - if (copy_to_user((char __user *) data, &get, sizeof(get)) != 0) - return -EFAULT; - - return 0; + return put_user_mtget(data, &get, compat); } /* Try the discipline ioctl function. */ if (device->discipline->ioctl_fn == NULL) return -EINVAL; - return device->discipline->ioctl_fn(device, no, data); + return device->discipline->ioctl_fn(device, no, (unsigned long)data); } static long @@ -445,7 +440,7 @@ tapechar_ioctl(struct file *filp, unsigned int no, unsigned long data) device = (struct tape_device *) filp->private_data; mutex_lock(&device->mutex); - rc = __tapechar_ioctl(device, no, data); + rc = __tapechar_ioctl(device, no, (void __user *)data, false); mutex_unlock(&device->mutex); return rc; } @@ -455,23 +450,17 @@ static long tapechar_compat_ioctl(struct file *filp, unsigned int no, unsigned long data) { struct tape_device *device = filp->private_data; - int rval = -ENOIOCTLCMD; - unsigned long argp; + long rc; - /* The 'arg' argument of any ioctl function may only be used for - * pointers because of the compat pointer conversion. - * Consider this when adding new ioctls. - */ - argp = (unsigned long) compat_ptr(data); - if (device->discipline->ioctl_fn) { - mutex_lock(&device->mutex); - rval = device->discipline->ioctl_fn(device, no, argp); - mutex_unlock(&device->mutex); - if (rval == -EINVAL) - rval = -ENOIOCTLCMD; - } + if (no == MTIOCPOS32) + no = MTIOCPOS; + else if (no == MTIOCGET32) + no = MTIOCGET; - return rval; + mutex_lock(&device->mutex); + rc = __tapechar_ioctl(device, no, compat_ptr(data), false); + mutex_unlock(&device->mutex); + return rc; } #endif /* CONFIG_COMPAT */ diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c index 7a1a1edde35d..842457b9134a 100644 --- a/drivers/scsi/osst.c +++ b/drivers/scsi/osst.c @@ -33,6 +33,7 @@ static const char * osst_version = "0.99.4"; #include <linux/module.h> +#include <linux/compat.h> #include <linux/fs.h> #include <linux/kernel.h> #include <linux/sched/signal.h> @@ -4941,7 +4942,7 @@ static int os_scsi_tape_close(struct inode * inode, struct file * filp) static long osst_ioctl(struct file * file, unsigned int cmd_in, unsigned long arg) { - int i, cmd_nr, cmd_type, blk, retval = 0; + int i, cmd_nr, cmd_type, cmd_size, blk, retval = 0; struct st_modedef * STm; struct st_partstat * STps; struct osst_request * SRpnt = NULL; @@ -4978,6 +4979,7 @@ static long osst_ioctl(struct file * file, cmd_type = _IOC_TYPE(cmd_in); cmd_nr = _IOC_NR(cmd_in); + cmd_size = _IOC_SIZE(cmd_in); #if DEBUG printk(OSST_DEB_MSG "%s:D: Ioctl %d,%d in %s mode\n", name, cmd_type, cmd_nr, STp->raw?"raw":"normal"); @@ -5179,7 +5181,8 @@ static long osst_ioctl(struct file * file, if (cmd_type == _IOC_TYPE(MTIOCGET) && cmd_nr == _IOC_NR(MTIOCGET)) { struct mtget mt_status; - if (_IOC_SIZE(cmd_in) != sizeof(struct mtget)) { + if (cmd_size != sizeof(struct mtget) && + cmd_size != sizeof(struct mtget32)) { retval = (-EINVAL); goto out; } @@ -5229,21 +5232,18 @@ static long osst_ioctl(struct file * file, STp->drv_buffer != 0) mt_status.mt_gstat |= GMT_IM_REP_EN(0xffffffff); - i = copy_to_user(p, &mt_status, sizeof(struct mtget)); - if (i) { - retval = (-EFAULT); - goto out; - } - - STp->recover_erreg = 0; /* Clear after read */ - retval = 0; + retval = put_user_mtget(p, &mt_status, + cmd_size == sizeof(struct mtget32)); + if (!retval) + STp->recover_erreg = 0; /* Clear after read */ goto out; } /* End of MTIOCGET */ if (cmd_type == _IOC_TYPE(MTIOCPOS) && cmd_nr == _IOC_NR(MTIOCPOS)) { struct mtpos mt_pos; - if (_IOC_SIZE(cmd_in) != sizeof(struct mtpos)) { + if (cmd_size != sizeof(struct mtpos) && + cmd_size != sizeof(struct mtpos32)) { retval = (-EINVAL); goto out; } @@ -5256,9 +5256,7 @@ static long osst_ioctl(struct file * file, goto out; } mt_pos.mt_blkno = blk; - i = copy_to_user(p, &mt_pos, sizeof(struct mtpos)); - if (i) - retval = -EFAULT; + retval = put_user_mtpos(p, &mt_pos, cmd_size == sizeof(struct mtpos32)); goto out; } if (SRpnt) osst_release_request(SRpnt); @@ -5284,6 +5282,14 @@ static long osst_compat_ioctl(struct file * file, unsigned int cmd_in, unsigned struct osst_tape *STp = file->private_data; struct scsi_device *sdev = STp->device; int ret = -ENOIOCTLCMD; + + switch (cmd_in) { + case MTIOCTOP: + case MTIOCPOS32: + case MTIOCGET32: + return osst_ioctl(file, cmd_in, (unsigned long)compat_ptr(arg)); + } + if (sdev->host->hostt->compat_ioctl) { ret = sdev->host->hostt->compat_ioctl(sdev, cmd_in, (void __user *)arg); diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c index 307df2fa39a3..62244ce53baa 100644 --- a/drivers/scsi/st.c +++ b/drivers/scsi/st.c @@ -21,6 +21,7 @@ static const char *verstr = "20160209"; #include <linux/module.h> +#include <linux/compat.h> #include <linux/fs.h> #include <linux/kernel.h> #include <linux/sched/signal.h> @@ -3498,7 +3499,7 @@ static int partition_tape(struct scsi_tape *STp, int size) /* The ioctl command */ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg) { - int i, cmd_nr, cmd_type, bt; + int i, cmd_nr, cmd_type, cmd_size, bt; int retval = 0; unsigned int blk; struct scsi_tape *STp = file->private_data; @@ -3532,6 +3533,7 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg) cmd_type = _IOC_TYPE(cmd_in); cmd_nr = _IOC_NR(cmd_in); + cmd_size = _IOC_SIZE(cmd_in); if (cmd_type == _IOC_TYPE(MTIOCTOP) && cmd_nr == _IOC_NR(MTIOCTOP)) { struct mtop mtc; @@ -3741,7 +3743,8 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg) if (cmd_type == _IOC_TYPE(MTIOCGET) && cmd_nr == _IOC_NR(MTIOCGET)) { struct mtget mt_status; - if (_IOC_SIZE(cmd_in) != sizeof(struct mtget)) { + if (cmd_size != sizeof(struct mtget) && + cmd_size != sizeof(struct mtget32)) { retval = (-EINVAL); goto out; } @@ -3796,19 +3799,16 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg) if (STp->cleaning_req) mt_status.mt_gstat |= GMT_CLN(0xffffffff); - i = copy_to_user(p, &mt_status, sizeof(struct mtget)); - if (i) { - retval = (-EFAULT); - goto out; - } + retval = put_user_mtget(p, &mt_status, + cmd_size == sizeof(struct mtget32)); STp->recover_reg = 0; /* Clear after read */ - retval = 0; goto out; } /* End of MTIOCGET */ if (cmd_type == _IOC_TYPE(MTIOCPOS) && cmd_nr == _IOC_NR(MTIOCPOS)) { struct mtpos mt_pos; - if (_IOC_SIZE(cmd_in) != sizeof(struct mtpos)) { + if (cmd_size != sizeof(struct mtpos) && + cmd_size != sizeof(struct mtpos32)) { retval = (-EINVAL); goto out; } @@ -3817,9 +3817,8 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg) goto out; } mt_pos.mt_blkno = blk; - i = copy_to_user(p, &mt_pos, sizeof(struct mtpos)); - if (i) - retval = (-EFAULT); + retval = put_user_mtpos(p, &mt_pos, + cmd_size == sizeof(struct mtpos32)); goto out; } mutex_unlock(&STp->lock); @@ -3853,14 +3852,22 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg) } #ifdef CONFIG_COMPAT -static long st_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +static long st_compat_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg) { struct scsi_tape *STp = file->private_data; struct scsi_device *sdev = STp->device; int ret = -ENOIOCTLCMD; + + switch (cmd_in) { + case MTIOCTOP: + case MTIOCPOS32: + case MTIOCGET32: + return st_ioctl(file, cmd_in, (unsigned long)compat_ptr(arg)); + } + if (sdev->host->hostt->compat_ioctl) { - ret = sdev->host->hostt->compat_ioctl(sdev, cmd, (void __user *)arg); + ret = sdev->host->hostt->compat_ioctl(sdev, cmd_in, (void __user *)arg); } return ret; diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index 312b52b4e974..1b47a60da455 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -37,7 +37,6 @@ #include <linux/ppp_defs.h> #include <linux/ppp-ioctl.h> #include <linux/if_pppox.h> -#include <linux/mtio.h> #include <linux/tty.h> #include <linux/vt_kern.h> #include <linux/fb.h> @@ -479,71 +478,6 @@ static int ppp_scompress(struct file *file, unsigned int cmd, return do_ioctl(file, PPPIOCSCOMPRESS, (unsigned long) odata); } -#ifdef CONFIG_BLOCK -struct mtget32 { - compat_long_t mt_type; - compat_long_t mt_resid; - compat_long_t mt_dsreg; - compat_long_t mt_gstat; - compat_long_t mt_erreg; - compat_daddr_t mt_fileno; - compat_daddr_t mt_blkno; -}; -#define MTIOCGET32 _IOR('m', 2, struct mtget32) - -struct mtpos32 { - compat_long_t mt_blkno; -}; -#define MTIOCPOS32 _IOR('m', 3, struct mtpos32) - -static int mt_ioctl_trans(struct file *file, - unsigned int cmd, void __user *argp) -{ - /* NULL initialization to make gcc shut up */ - struct mtget __user *get = NULL; - struct mtget32 __user *umget32; - struct mtpos __user *pos = NULL; - struct mtpos32 __user *upos32; - unsigned long kcmd; - void *karg; - int err = 0; - - switch(cmd) { - case MTIOCPOS32: - kcmd = MTIOCPOS; - pos = compat_alloc_user_space(sizeof(*pos)); - karg = pos; - break; - default: /* MTIOCGET32 */ - kcmd = MTIOCGET; - get = compat_alloc_user_space(sizeof(*get)); - karg = get; - break; - } - if (karg == NULL) - return -EFAULT; - err = do_ioctl(file, kcmd, (unsigned long)karg); - if (err) - return err; - switch (cmd) { - case MTIOCPOS32: - upos32 = argp; - err = convert_in_user(&pos->mt_blkno, &upos32->mt_blkno); - break; - case MTIOCGET32: - umget32 = argp; - err = convert_in_user(&get->mt_type, &umget32->mt_type); - err |= convert_in_user(&get->mt_resid, &umget32->mt_resid); - err |= convert_in_user(&get->mt_dsreg, &umget32->mt_dsreg); - err |= convert_in_user(&get->mt_gstat, &umget32->mt_gstat); - err |= convert_in_user(&get->mt_erreg, &umget32->mt_erreg); - err |= convert_in_user(&get->mt_fileno, &umget32->mt_fileno); - err |= convert_in_user(&get->mt_blkno, &umget32->mt_blkno); - break; - } - return err ? -EFAULT: 0; -} - #endif /* CONFIG_BLOCK */ /* Bluetooth ioctls */ @@ -792,8 +726,6 @@ IGNORE_IOCTL(VT_GETMODE) */ COMPATIBLE_IOCTL(_IOR('p', 20, int[7])) /* RTCGET */ COMPATIBLE_IOCTL(_IOW('p', 21, int[7])) /* RTCSET */ -/* Little m */ -COMPATIBLE_IOCTL(MTIOCTOP) /* Socket level stuff */ COMPATIBLE_IOCTL(FIOQSIZE) #ifdef CONFIG_BLOCK @@ -1252,9 +1184,6 @@ static long do_ioctl_trans(unsigned int cmd, return sg_ioctl_trans(file, cmd, argp); case SG_GET_REQUEST_TABLE: return sg_grt_trans(file, cmd, argp); - case MTIOCGET32: - case MTIOCPOS32: - return mt_ioctl_trans(file, cmd, argp); #endif /* Serial */ case TIOCGSERIAL: diff --git a/include/linux/mtio.h b/include/linux/mtio.h new file mode 100644 index 000000000000..02640756a40d --- /dev/null +++ b/include/linux/mtio.h @@ -0,0 +1,58 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_MTIO_COMPAT_H +#define _LINUX_MTIO_COMPAT_H + +#include <uapi/linux/mtio.h> +#include <linux/uaccess.h> + +/* + * helper functions for implementing compat ioctls on the four tape + * drivers: we define the 32-bit layout of each incompatible strucucture, + * plus a wrapper function to copy it to user space in either format. + */ + +struct mtget32 { + s32 mt_type; + s32 mt_resid; + s32 mt_dsreg; + s32 mt_gstat; + s32 mt_erreg; + s32 mt_fileno; + s32 mt_blkno; +}; +#define MTIOCGET32 _IOR('m', 2, struct mtget32) + +struct mtpos32 { + s32 mt_blkno; +}; +#define MTIOCPOS32 _IOR('m', 3, struct mtpos32) + +static inline int put_user_mtget(void __user *u, struct mtget *k, bool compat) +{ + struct mtget32 k32 = { + .mt_type = k->mt_type, + .mt_resid = k->mt_resid, + .mt_dsreg = k->mt_dsreg, + .mt_gstat = k->mt_gstat, + .mt_fileno = k->mt_fileno, + .mt_blkno = k->mt_blkno, + }; + int ret; + + if (IS_ENABLED(CONFIG_COMPAT) && compat) + ret = copy_to_user(u, &k32, sizeof(k32)); + else + ret = copy_to_user(u, k, sizeof(*k)); + + return ret ? -EFAULT : 0; +} + +static inline int put_user_mtpos(void __user *u, struct mtpos *k, bool compat) +{ + if (IS_ENABLED(CONFIG_COMPAT) && compat) + return put_user(k->mt_blkno, (u32 __user *)u); + else + return put_user(k->mt_blkno, (long __user *)u); +} + +#endif -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 08/17] compat_ioctl: remove keyboard ioctl translation 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann 2018-09-12 15:08 ` [PATCH v2 06/17] compat_ioctl: move rtc handling into rtc-dev.c Arnd Bergmann 2018-09-12 15:08 ` [PATCH v2 07/17] compat_ioctl: move tape handling into drivers Arnd Bergmann @ 2018-09-12 15:08 ` Arnd Bergmann 2018-09-12 15:08 ` [PATCH v2 09/17] compat_ioctl: remove HIDIO translation Arnd Bergmann ` (8 subsequent siblings) 11 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:08 UTC (permalink / raw) To: viro; +Cc: linux-fsdevel, Arnd Bergmann, linux-kernel The KD* family of ioctls is implemented in two drivers: drivers/tty/vt and drivers/s390/char/tty3270.c. Both of them have compat handlers for all their ioctl commands, so translation in fs/compat_ioctl.c is never used. Commit fb07a5f857ac ("compat_ioctl: remove all VT ioctl handling") removed the compat handling for all the other VT ioctls back in 2009, but it seems I missed the keyboard ones back then. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/compat_ioctl.c | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index 1b47a60da455..e245deb684b5 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -687,23 +687,6 @@ COMPATIBLE_IOCTL(FIGETBSZ) COMPATIBLE_IOCTL(FIFREEZE) COMPATIBLE_IOCTL(FITHAW) COMPATIBLE_IOCTL(FITRIM) -COMPATIBLE_IOCTL(KDGETKEYCODE) -COMPATIBLE_IOCTL(KDSETKEYCODE) -COMPATIBLE_IOCTL(KDGKBTYPE) -COMPATIBLE_IOCTL(KDGETMODE) -COMPATIBLE_IOCTL(KDGKBMODE) -COMPATIBLE_IOCTL(KDGKBMETA) -COMPATIBLE_IOCTL(KDGKBENT) -COMPATIBLE_IOCTL(KDSKBENT) -COMPATIBLE_IOCTL(KDGKBSENT) -COMPATIBLE_IOCTL(KDSKBSENT) -COMPATIBLE_IOCTL(KDGKBDIACR) -COMPATIBLE_IOCTL(KDSKBDIACR) -COMPATIBLE_IOCTL(KDGKBDIACRUC) -COMPATIBLE_IOCTL(KDSKBDIACRUC) -COMPATIBLE_IOCTL(KDKBDREP) -COMPATIBLE_IOCTL(KDGKBLED) -COMPATIBLE_IOCTL(KDGETLED) #ifdef CONFIG_BLOCK /* Big S */ COMPATIBLE_IOCTL(SCSI_IOCTL_GET_IDLUN) @@ -1210,15 +1193,6 @@ static long do_ioctl_trans(unsigned int cmd, case HOT_ADD_DISK: case SET_DISK_FAULTY: case SET_BITMAP_FILE: - /* Big K */ - case KDSIGACCEPT: - case KIOCSOUND: - case KDMKTONE: - case KDSETMODE: - case KDSKBMODE: - case KDSKBMETA: - case KDSKBLED: - case KDSETLED: return vfs_ioctl(file, cmd, arg); } -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 09/17] compat_ioctl: remove HIDIO translation 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann ` (2 preceding siblings ...) 2018-09-12 15:08 ` [PATCH v2 08/17] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann @ 2018-09-12 15:08 ` Arnd Bergmann 2018-09-12 15:56 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Jason Gunthorpe ` (7 subsequent siblings) 11 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:08 UTC (permalink / raw) To: viro; +Cc: linux-fsdevel, Arnd Bergmann, linux-kernel The two drivers implementing these both gained proper compat_ioctl() handlers a long time ago with commits bb6c8d8fa9b5 ("HID: hiddev: Add 32bit ioctl compatibilty") and ae5e49c79c05 ("HID: hidraw: add compatibility ioctl() for 32-bit applications."), so the lists in fs/compat_ioctl.c are no longer used. It appears that the lists were also incomplete, so the translation didn't actually work correctly when it was still in use. Remove them as cleanup. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/compat_ioctl.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index e245deb684b5..001c9fd89d30 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -1023,23 +1023,6 @@ COMPATIBLE_IOCTL(PCIIOC_CONTROLLER) COMPATIBLE_IOCTL(PCIIOC_MMAP_IS_IO) COMPATIBLE_IOCTL(PCIIOC_MMAP_IS_MEM) COMPATIBLE_IOCTL(PCIIOC_WRITE_COMBINE) -/* hiddev */ -COMPATIBLE_IOCTL(HIDIOCGVERSION) -COMPATIBLE_IOCTL(HIDIOCAPPLICATION) -COMPATIBLE_IOCTL(HIDIOCGDEVINFO) -COMPATIBLE_IOCTL(HIDIOCGSTRING) -COMPATIBLE_IOCTL(HIDIOCINITREPORT) -COMPATIBLE_IOCTL(HIDIOCGREPORT) -COMPATIBLE_IOCTL(HIDIOCSREPORT) -COMPATIBLE_IOCTL(HIDIOCGREPORTINFO) -COMPATIBLE_IOCTL(HIDIOCGFIELDINFO) -COMPATIBLE_IOCTL(HIDIOCGUSAGE) -COMPATIBLE_IOCTL(HIDIOCSUSAGE) -COMPATIBLE_IOCTL(HIDIOCGUCODE) -COMPATIBLE_IOCTL(HIDIOCGFLAG) -COMPATIBLE_IOCTL(HIDIOCSFLAG) -COMPATIBLE_IOCTL(HIDIOCGCOLLECTIONINDEX) -COMPATIBLE_IOCTL(HIDIOCGCOLLECTIONINFO) /* dvb */ COMPATIBLE_IOCTL(AUDIO_STOP) COMPATIBLE_IOCTL(AUDIO_PLAY) @@ -1094,7 +1077,6 @@ COMPATIBLE_IOCTL(CEC_S_MODE) COMPATIBLE_IOCTL(CEC_TRANSMIT) COMPATIBLE_IOCTL(CEC_RECEIVE) COMPATIBLE_IOCTL(CEC_DQEVENT) - /* joystick */ COMPATIBLE_IOCTL(JSIOCGVERSION) COMPATIBLE_IOCTL(JSIOCGAXES) -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann ` (3 preceding siblings ...) 2018-09-12 15:08 ` [PATCH v2 09/17] compat_ioctl: remove HIDIO translation Arnd Bergmann @ 2018-09-12 15:56 ` Jason Gunthorpe 2018-09-12 15:58 ` Daniel Vetter ` (6 subsequent siblings) 11 siblings, 0 replies; 48+ messages in thread From: Jason Gunthorpe @ 2018-09-12 15:56 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, Greg Kroah-Hartman, David S. Miller, devel, linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, netdev On Wed, Sep 12, 2018 at 05:08:52PM +0200, Arnd Bergmann wrote: > The .ioctl and .compat_ioctl file operations have the same prototype so > they can both point to the same function, which works great almost all > the time when all the commands are compatible. > > One exception is the s390 architecture, where a compat pointer is only > 31 bit wide, and converting it into a 64-bit pointer requires calling > compat_ptr(). Most drivers here will ever run in s390, but since we now > have a generic helper for it, it's easy enough to use it consistently. > > I double-checked all these drivers to ensure that all ioctl arguments > are used as pointers or are ignored, but are not interpreted as integer > values. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > drivers/android/binder.c | 2 +- > drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +- > drivers/dma-buf/dma-buf.c | 4 +--- > drivers/dma-buf/sw_sync.c | 2 +- > drivers/dma-buf/sync_file.c | 2 +- > drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +- > drivers/hid/hidraw.c | 4 +--- > drivers/iio/industrialio-core.c | 2 +- > drivers/infiniband/core/uverbs_main.c | 4 ++-- > drivers/media/rc/lirc_dev.c | 4 +--- > drivers/mfd/cros_ec_dev.c | 4 +--- > drivers/misc/vmw_vmci/vmci_host.c | 2 +- > drivers/nvdimm/bus.c | 4 ++-- > drivers/nvme/host/core.c | 2 +- > drivers/pci/switch/switchtec.c | 2 +- > drivers/platform/x86/wmi.c | 2 +- > drivers/rpmsg/rpmsg_char.c | 4 ++-- > drivers/sbus/char/display7seg.c | 2 +- > drivers/sbus/char/envctrl.c | 4 +--- > drivers/scsi/3w-xxxx.c | 4 +--- > drivers/scsi/cxlflash/main.c | 2 +- > drivers/scsi/esas2r/esas2r_main.c | 2 +- > drivers/scsi/pmcraid.c | 4 +--- > drivers/staging/android/ion/ion.c | 4 +--- > drivers/staging/vme/devices/vme_user.c | 2 +- > drivers/tee/tee_core.c | 2 +- > drivers/usb/class/cdc-wdm.c | 2 +- > drivers/usb/class/usbtmc.c | 4 +--- > drivers/video/fbdev/ps3fb.c | 2 +- > drivers/virt/fsl_hypervisor.c | 2 +- > fs/btrfs/super.c | 2 +- > fs/ceph/dir.c | 2 +- > fs/ceph/file.c | 2 +- > fs/fuse/dev.c | 2 +- > fs/notify/fanotify/fanotify_user.c | 2 +- > fs/userfaultfd.c | 2 +- > net/rfkill/core.c | 2 +- > 37 files changed, 40 insertions(+), 58 deletions(-) > diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c > index 823beca448e1..f4755c1c9cfa 100644 > +++ b/drivers/infiniband/core/uverbs_main.c > @@ -930,7 +930,7 @@ static const struct file_operations uverbs_fops = { > .release = ib_uverbs_close, > .llseek = no_llseek, > .unlocked_ioctl = ib_uverbs_ioctl, > - .compat_ioctl = ib_uverbs_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static const struct file_operations uverbs_mmap_fops = { > @@ -941,7 +941,7 @@ static const struct file_operations uverbs_mmap_fops = { > .release = ib_uverbs_close, > .llseek = no_llseek, > .unlocked_ioctl = ib_uverbs_ioctl, > - .compat_ioctl = ib_uverbs_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static struct ib_client uverbs_client = { For uverbs: Acked-by: Jason Gunthorpe <jgg@mellanox.com> It is very strange, this patch did not appear in the RDMA patchworks, I almost missed it :| Jason ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann ` (4 preceding siblings ...) 2018-09-12 15:56 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Jason Gunthorpe @ 2018-09-12 15:58 ` Daniel Vetter 2018-09-12 16:01 ` Mauro Carvalho Chehab ` (5 subsequent siblings) 11 siblings, 0 replies; 48+ messages in thread From: Daniel Vetter @ 2018-09-12 15:58 UTC (permalink / raw) To: Arnd Bergmann Cc: Al Viro, Linux Fbdev development list, linux-iio, Linux PCI, linux-remoteproc, linux-nvme, platform-driver-x86, sparclinux, driverdevel, linux-scsi, linux-nvdimm, linux-rdma, qat-linux, amd-gfx list, linux-input, open list:DMA BUFFER SHARING FRAMEWORK, moderated list:DMA BUFFER SHARING FRAMEWORK, dri-devel, ceph-devel, Greg Kroah-Hartman, USB list, linux-wireless, Linux Kernel Mailing List, linux-crypto, netdev, linux-fsdevel, linuxppc-dev, David S. Miller, linux-btrfs On Wed, Sep 12, 2018 at 5:08 PM, Arnd Bergmann <arnd@arndb.de> wrote: > The .ioctl and .compat_ioctl file operations have the same prototype so > they can both point to the same function, which works great almost all > the time when all the commands are compatible. > > One exception is the s390 architecture, where a compat pointer is only > 31 bit wide, and converting it into a 64-bit pointer requires calling > compat_ptr(). Most drivers here will ever run in s390, but since we now > have a generic helper for it, it's easy enough to use it consistently. > > I double-checked all these drivers to ensure that all ioctl arguments > are used as pointers or are ignored, but are not interpreted as integer > values. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> At least for the drm and dma-buf bits. -Daniel > --- > drivers/android/binder.c | 2 +- > drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +- > drivers/dma-buf/dma-buf.c | 4 +--- > drivers/dma-buf/sw_sync.c | 2 +- > drivers/dma-buf/sync_file.c | 2 +- > drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +- > drivers/hid/hidraw.c | 4 +--- > drivers/iio/industrialio-core.c | 2 +- > drivers/infiniband/core/uverbs_main.c | 4 ++-- > drivers/media/rc/lirc_dev.c | 4 +--- > drivers/mfd/cros_ec_dev.c | 4 +--- > drivers/misc/vmw_vmci/vmci_host.c | 2 +- > drivers/nvdimm/bus.c | 4 ++-- > drivers/nvme/host/core.c | 2 +- > drivers/pci/switch/switchtec.c | 2 +- > drivers/platform/x86/wmi.c | 2 +- > drivers/rpmsg/rpmsg_char.c | 4 ++-- > drivers/sbus/char/display7seg.c | 2 +- > drivers/sbus/char/envctrl.c | 4 +--- > drivers/scsi/3w-xxxx.c | 4 +--- > drivers/scsi/cxlflash/main.c | 2 +- > drivers/scsi/esas2r/esas2r_main.c | 2 +- > drivers/scsi/pmcraid.c | 4 +--- > drivers/staging/android/ion/ion.c | 4 +--- > drivers/staging/vme/devices/vme_user.c | 2 +- > drivers/tee/tee_core.c | 2 +- > drivers/usb/class/cdc-wdm.c | 2 +- > drivers/usb/class/usbtmc.c | 4 +--- > drivers/video/fbdev/ps3fb.c | 2 +- > drivers/virt/fsl_hypervisor.c | 2 +- > fs/btrfs/super.c | 2 +- > fs/ceph/dir.c | 2 +- > fs/ceph/file.c | 2 +- > fs/fuse/dev.c | 2 +- > fs/notify/fanotify/fanotify_user.c | 2 +- > fs/userfaultfd.c | 2 +- > net/rfkill/core.c | 2 +- > 37 files changed, 40 insertions(+), 58 deletions(-) > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > index d58763b6b009..d2464f5759f8 100644 > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -5576,7 +5576,7 @@ static const struct file_operations binder_fops = { > .owner = THIS_MODULE, > .poll = binder_poll, > .unlocked_ioctl = binder_ioctl, > - .compat_ioctl = binder_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .mmap = binder_mmap, > .open = binder_open, > .flush = binder_flush, > diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c b/drivers/crypto/qat/qat_common/adf_ctl_drv.c > index abc7a7f64d64..8ff77a70addc 100644 > --- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c > +++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c > @@ -68,7 +68,7 @@ static long adf_ctl_ioctl(struct file *fp, unsigned int cmd, unsigned long arg); > static const struct file_operations adf_ctl_ops = { > .owner = THIS_MODULE, > .unlocked_ioctl = adf_ctl_ioctl, > - .compat_ioctl = adf_ctl_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > struct adf_ctl_drv_info { > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index 13884474d158..a6d7dc4cf7e9 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -325,9 +325,7 @@ static const struct file_operations dma_buf_fops = { > .llseek = dma_buf_llseek, > .poll = dma_buf_poll, > .unlocked_ioctl = dma_buf_ioctl, > -#ifdef CONFIG_COMPAT > - .compat_ioctl = dma_buf_ioctl, > -#endif > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > /* > diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c > index 53c1d6d36a64..bc810506d487 100644 > --- a/drivers/dma-buf/sw_sync.c > +++ b/drivers/dma-buf/sw_sync.c > @@ -419,5 +419,5 @@ const struct file_operations sw_sync_debugfs_fops = { > .open = sw_sync_debugfs_open, > .release = sw_sync_debugfs_release, > .unlocked_ioctl = sw_sync_ioctl, > - .compat_ioctl = sw_sync_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c > index 35dd06479867..1c64ed60c658 100644 > --- a/drivers/dma-buf/sync_file.c > +++ b/drivers/dma-buf/sync_file.c > @@ -488,5 +488,5 @@ static const struct file_operations sync_file_fops = { > .release = sync_file_release, > .poll = sync_file_poll, > .unlocked_ioctl = sync_file_ioctl, > - .compat_ioctl = sync_file_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > index 297b36c26a05..1d7b1e3c3ebe 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > @@ -47,7 +47,7 @@ static const char kfd_dev_name[] = "kfd"; > static const struct file_operations kfd_fops = { > .owner = THIS_MODULE, > .unlocked_ioctl = kfd_ioctl, > - .compat_ioctl = kfd_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .open = kfd_open, > .mmap = kfd_mmap, > }; > diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c > index 4a44e48e08b2..e44b64812850 100644 > --- a/drivers/hid/hidraw.c > +++ b/drivers/hid/hidraw.c > @@ -476,9 +476,7 @@ static const struct file_operations hidraw_ops = { > .release = hidraw_release, > .unlocked_ioctl = hidraw_ioctl, > .fasync = hidraw_fasync, > -#ifdef CONFIG_COMPAT > - .compat_ioctl = hidraw_ioctl, > -#endif > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .llseek = noop_llseek, > }; > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index a062cfddc5af..22844b94b0e9 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -1630,7 +1630,7 @@ static const struct file_operations iio_buffer_fileops = { > .owner = THIS_MODULE, > .llseek = noop_llseek, > .unlocked_ioctl = iio_ioctl, > - .compat_ioctl = iio_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static int iio_check_unique_scan_index(struct iio_dev *indio_dev) > diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c > index 823beca448e1..f4755c1c9cfa 100644 > --- a/drivers/infiniband/core/uverbs_main.c > +++ b/drivers/infiniband/core/uverbs_main.c > @@ -930,7 +930,7 @@ static const struct file_operations uverbs_fops = { > .release = ib_uverbs_close, > .llseek = no_llseek, > .unlocked_ioctl = ib_uverbs_ioctl, > - .compat_ioctl = ib_uverbs_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static const struct file_operations uverbs_mmap_fops = { > @@ -941,7 +941,7 @@ static const struct file_operations uverbs_mmap_fops = { > .release = ib_uverbs_close, > .llseek = no_llseek, > .unlocked_ioctl = ib_uverbs_ioctl, > - .compat_ioctl = ib_uverbs_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static struct ib_client uverbs_client = { > diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c > index f862f1b7f996..077209f414ed 100644 > --- a/drivers/media/rc/lirc_dev.c > +++ b/drivers/media/rc/lirc_dev.c > @@ -730,9 +730,7 @@ static const struct file_operations lirc_fops = { > .owner = THIS_MODULE, > .write = ir_lirc_transmit_ir, > .unlocked_ioctl = ir_lirc_ioctl, > -#ifdef CONFIG_COMPAT > - .compat_ioctl = ir_lirc_ioctl, > -#endif > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .read = ir_lirc_read, > .poll = ir_lirc_poll, > .open = ir_lirc_open, > diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c > index 999dac752bcc..35a04bcf55da 100644 > --- a/drivers/mfd/cros_ec_dev.c > +++ b/drivers/mfd/cros_ec_dev.c > @@ -258,9 +258,7 @@ static const struct file_operations fops = { > .release = ec_device_release, > .read = ec_device_read, > .unlocked_ioctl = ec_device_ioctl, > -#ifdef CONFIG_COMPAT > - .compat_ioctl = ec_device_ioctl, > -#endif > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static void cros_ec_sensors_register(struct cros_ec_dev *ec) > diff --git a/drivers/misc/vmw_vmci/vmci_host.c b/drivers/misc/vmw_vmci/vmci_host.c > index 83e0c95d20a4..1308f889e53b 100644 > --- a/drivers/misc/vmw_vmci/vmci_host.c > +++ b/drivers/misc/vmw_vmci/vmci_host.c > @@ -983,7 +983,7 @@ static const struct file_operations vmuser_fops = { > .release = vmci_host_close, > .poll = vmci_host_poll, > .unlocked_ioctl = vmci_host_unlocked_ioctl, > - .compat_ioctl = vmci_host_unlocked_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static struct miscdevice vmci_host_miscdev = { > diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c > index 8aae6dcc839f..7449cbc55df7 100644 > --- a/drivers/nvdimm/bus.c > +++ b/drivers/nvdimm/bus.c > @@ -1133,7 +1133,7 @@ static const struct file_operations nvdimm_bus_fops = { > .owner = THIS_MODULE, > .open = nd_open, > .unlocked_ioctl = nd_ioctl, > - .compat_ioctl = nd_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .llseek = noop_llseek, > }; > > @@ -1141,7 +1141,7 @@ static const struct file_operations nvdimm_fops = { > .owner = THIS_MODULE, > .open = nd_open, > .unlocked_ioctl = nvdimm_ioctl, > - .compat_ioctl = nvdimm_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .llseek = noop_llseek, > }; > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index dd8ec1dd9219..2d986f573a29 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -2579,7 +2579,7 @@ static const struct file_operations nvme_dev_fops = { > .owner = THIS_MODULE, > .open = nvme_dev_open, > .unlocked_ioctl = nvme_dev_ioctl, > - .compat_ioctl = nvme_dev_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static ssize_t nvme_sysfs_reset(struct device *dev, > diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c > index 9940cc70f38b..4296919c784e 100644 > --- a/drivers/pci/switch/switchtec.c > +++ b/drivers/pci/switch/switchtec.c > @@ -967,7 +967,7 @@ static const struct file_operations switchtec_fops = { > .read = switchtec_dev_read, > .poll = switchtec_dev_poll, > .unlocked_ioctl = switchtec_dev_ioctl, > - .compat_ioctl = switchtec_dev_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static void link_event_work(struct work_struct *work) > diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c > index 04791ea5d97b..e4d0697e07d6 100644 > --- a/drivers/platform/x86/wmi.c > +++ b/drivers/platform/x86/wmi.c > @@ -886,7 +886,7 @@ static const struct file_operations wmi_fops = { > .read = wmi_char_read, > .open = wmi_char_open, > .unlocked_ioctl = wmi_ioctl, > - .compat_ioctl = wmi_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static int wmi_dev_probe(struct device *dev) > diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c > index a76b963a7e50..02aefb2b2d47 100644 > --- a/drivers/rpmsg/rpmsg_char.c > +++ b/drivers/rpmsg/rpmsg_char.c > @@ -285,7 +285,7 @@ static const struct file_operations rpmsg_eptdev_fops = { > .write = rpmsg_eptdev_write, > .poll = rpmsg_eptdev_poll, > .unlocked_ioctl = rpmsg_eptdev_ioctl, > - .compat_ioctl = rpmsg_eptdev_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static ssize_t name_show(struct device *dev, struct device_attribute *attr, > @@ -446,7 +446,7 @@ static const struct file_operations rpmsg_ctrldev_fops = { > .open = rpmsg_ctrldev_open, > .release = rpmsg_ctrldev_release, > .unlocked_ioctl = rpmsg_ctrldev_ioctl, > - .compat_ioctl = rpmsg_ctrldev_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static void rpmsg_ctrldev_release_device(struct device *dev) > diff --git a/drivers/sbus/char/display7seg.c b/drivers/sbus/char/display7seg.c > index 5c8ed7350a04..064fe7247eb2 100644 > --- a/drivers/sbus/char/display7seg.c > +++ b/drivers/sbus/char/display7seg.c > @@ -155,7 +155,7 @@ static long d7s_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > static const struct file_operations d7s_fops = { > .owner = THIS_MODULE, > .unlocked_ioctl = d7s_ioctl, > - .compat_ioctl = d7s_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .open = d7s_open, > .release = d7s_release, > .llseek = noop_llseek, > diff --git a/drivers/sbus/char/envctrl.c b/drivers/sbus/char/envctrl.c > index 56e962a01493..a26665ccea56 100644 > --- a/drivers/sbus/char/envctrl.c > +++ b/drivers/sbus/char/envctrl.c > @@ -714,9 +714,7 @@ static const struct file_operations envctrl_fops = { > .owner = THIS_MODULE, > .read = envctrl_read, > .unlocked_ioctl = envctrl_ioctl, > -#ifdef CONFIG_COMPAT > - .compat_ioctl = envctrl_ioctl, > -#endif > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .open = envctrl_open, > .release = envctrl_release, > .llseek = noop_llseek, > diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c > index 471366945bd4..86c9f22a152f 100644 > --- a/drivers/scsi/3w-xxxx.c > +++ b/drivers/scsi/3w-xxxx.c > @@ -1047,9 +1047,7 @@ static int tw_chrdev_open(struct inode *inode, struct file *file) > static const struct file_operations tw_fops = { > .owner = THIS_MODULE, > .unlocked_ioctl = tw_chrdev_ioctl, > -#ifdef CONFIG_COMPAT > - .compat_ioctl = tw_chrdev_ioctl, > -#endif > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .open = tw_chrdev_open, > .release = NULL, > .llseek = noop_llseek, > diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c > index 6637116529aa..d968efeb50e8 100644 > --- a/drivers/scsi/cxlflash/main.c > +++ b/drivers/scsi/cxlflash/main.c > @@ -3596,7 +3596,7 @@ static const struct file_operations cxlflash_chr_fops = { > .owner = THIS_MODULE, > .open = cxlflash_chr_open, > .unlocked_ioctl = cxlflash_chr_ioctl, > - .compat_ioctl = cxlflash_chr_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > /** > diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c > index c07118617d89..95142292e702 100644 > --- a/drivers/scsi/esas2r/esas2r_main.c > +++ b/drivers/scsi/esas2r/esas2r_main.c > @@ -614,7 +614,7 @@ static int __init esas2r_init(void) > > /* Handle ioctl calls to "/proc/scsi/esas2r/ATTOnode" */ > static const struct file_operations esas2r_proc_fops = { > - .compat_ioctl = esas2r_proc_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .unlocked_ioctl = esas2r_proc_ioctl, > }; > > diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c > index 4e86994e10e8..8a8c73d3bdad 100644 > --- a/drivers/scsi/pmcraid.c > +++ b/drivers/scsi/pmcraid.c > @@ -3999,9 +3999,7 @@ static const struct file_operations pmcraid_fops = { > .open = pmcraid_chr_open, > .fasync = pmcraid_chr_fasync, > .unlocked_ioctl = pmcraid_chr_ioctl, > -#ifdef CONFIG_COMPAT > - .compat_ioctl = pmcraid_chr_ioctl, > -#endif > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .llseek = noop_llseek, > }; > > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index 99073325b0c0..ef727c235392 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -484,9 +484,7 @@ int ion_query_heaps(struct ion_heap_query *query) > static const struct file_operations ion_fops = { > .owner = THIS_MODULE, > .unlocked_ioctl = ion_ioctl, > -#ifdef CONFIG_COMPAT > - .compat_ioctl = ion_ioctl, > -#endif > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static int debug_shrink_set(void *data, u64 val) > diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c > index 6a33aaa1a49f..568700ffd2f2 100644 > --- a/drivers/staging/vme/devices/vme_user.c > +++ b/drivers/staging/vme/devices/vme_user.c > @@ -494,7 +494,7 @@ static const struct file_operations vme_user_fops = { > .write = vme_user_write, > .llseek = vme_user_llseek, > .unlocked_ioctl = vme_user_unlocked_ioctl, > - .compat_ioctl = vme_user_unlocked_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .mmap = vme_user_mmap, > }; > > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c > index dd46b758852a..cb79f28be894 100644 > --- a/drivers/tee/tee_core.c > +++ b/drivers/tee/tee_core.c > @@ -670,7 +670,7 @@ static const struct file_operations tee_fops = { > .open = tee_open, > .release = tee_release, > .unlocked_ioctl = tee_ioctl, > - .compat_ioctl = tee_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static void tee_release_device(struct device *dev) > diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c > index bec581fb7c63..6e4998c8e64f 100644 > --- a/drivers/usb/class/cdc-wdm.c > +++ b/drivers/usb/class/cdc-wdm.c > @@ -724,7 +724,7 @@ static const struct file_operations wdm_fops = { > .release = wdm_release, > .poll = wdm_poll, > .unlocked_ioctl = wdm_ioctl, > - .compat_ioctl = wdm_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .llseek = noop_llseek, > }; > > diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c > index 83ffa5a14c3d..d5da47c4c462 100644 > --- a/drivers/usb/class/usbtmc.c > +++ b/drivers/usb/class/usbtmc.c > @@ -1460,9 +1460,7 @@ static const struct file_operations fops = { > .open = usbtmc_open, > .release = usbtmc_release, > .unlocked_ioctl = usbtmc_ioctl, > -#ifdef CONFIG_COMPAT > - .compat_ioctl = usbtmc_ioctl, > -#endif > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .fasync = usbtmc_fasync, > .poll = usbtmc_poll, > .llseek = default_llseek, > diff --git a/drivers/video/fbdev/ps3fb.c b/drivers/video/fbdev/ps3fb.c > index 5ed2db39d823..f9f8ffaf1c4a 100644 > --- a/drivers/video/fbdev/ps3fb.c > +++ b/drivers/video/fbdev/ps3fb.c > @@ -949,7 +949,7 @@ static struct fb_ops ps3fb_ops = { > .fb_mmap = ps3fb_mmap, > .fb_blank = ps3fb_blank, > .fb_ioctl = ps3fb_ioctl, > - .fb_compat_ioctl = ps3fb_ioctl > + .fb_compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static const struct fb_fix_screeninfo ps3fb_fix = { > diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c > index 8ba726e600e9..406b7e492214 100644 > --- a/drivers/virt/fsl_hypervisor.c > +++ b/drivers/virt/fsl_hypervisor.c > @@ -703,7 +703,7 @@ static const struct file_operations fsl_hv_fops = { > .poll = fsl_hv_poll, > .read = fsl_hv_read, > .unlocked_ioctl = fsl_hv_ioctl, > - .compat_ioctl = fsl_hv_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static struct miscdevice fsl_hv_misc_dev = { > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c > index 6601c9aa5e35..2b5a8ad86305 100644 > --- a/fs/btrfs/super.c > +++ b/fs/btrfs/super.c > @@ -2352,7 +2352,7 @@ static const struct super_operations btrfs_super_ops = { > static const struct file_operations btrfs_ctl_fops = { > .open = btrfs_control_open, > .unlocked_ioctl = btrfs_control_ioctl, > - .compat_ioctl = btrfs_control_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .owner = THIS_MODULE, > .llseek = noop_llseek, > }; > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c > index da73f29d7faa..eb869fe6774d 100644 > --- a/fs/ceph/dir.c > +++ b/fs/ceph/dir.c > @@ -1489,7 +1489,7 @@ const struct file_operations ceph_dir_fops = { > .open = ceph_open, > .release = ceph_release, > .unlocked_ioctl = ceph_ioctl, > - .compat_ioctl = ceph_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .fsync = ceph_fsync, > .lock = ceph_lock, > .flock = ceph_flock, > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > index 92ab20433682..85094042cfac 100644 > --- a/fs/ceph/file.c > +++ b/fs/ceph/file.c > @@ -1842,7 +1842,7 @@ const struct file_operations ceph_file_fops = { > .splice_read = generic_file_splice_read, > .splice_write = iter_file_splice_write, > .unlocked_ioctl = ceph_ioctl, > - .compat_ioctl = ceph_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .fallocate = ceph_fallocate, > }; > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 11ea2c4a38ab..a6d4a24963ed 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -2258,7 +2258,7 @@ const struct file_operations fuse_dev_operations = { > .release = fuse_dev_release, > .fasync = fuse_dev_fasync, > .unlocked_ioctl = fuse_dev_ioctl, > - .compat_ioctl = fuse_dev_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > EXPORT_SYMBOL_GPL(fuse_dev_operations); > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > index 69054886915b..fc4193b384cf 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -447,7 +447,7 @@ static const struct file_operations fanotify_fops = { > .fasync = NULL, > .release = fanotify_release, > .unlocked_ioctl = fanotify_ioctl, > - .compat_ioctl = fanotify_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .llseek = noop_llseek, > }; > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index bfa0ec69f924..bc9118b58a8a 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -1878,7 +1878,7 @@ static const struct file_operations userfaultfd_fops = { > .poll = userfaultfd_poll, > .read = userfaultfd_read, > .unlocked_ioctl = userfaultfd_ioctl, > - .compat_ioctl = userfaultfd_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .llseek = noop_llseek, > }; > > diff --git a/net/rfkill/core.c b/net/rfkill/core.c > index 1355f5ca8d22..ba68b53f58ab 100644 > --- a/net/rfkill/core.c > +++ b/net/rfkill/core.c > @@ -1323,7 +1323,7 @@ static const struct file_operations rfkill_fops = { > .release = rfkill_fop_release, > #ifdef CONFIG_RFKILL_INPUT > .unlocked_ioctl = rfkill_fop_ioctl, > - .compat_ioctl = rfkill_fop_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > #endif > .llseek = no_llseek, > }; > -- > 2.18.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann ` (5 preceding siblings ...) 2018-09-12 15:58 ` Daniel Vetter @ 2018-09-12 16:01 ` Mauro Carvalho Chehab 2018-09-12 18:13 ` Greg Kroah-Hartman ` (4 subsequent siblings) 11 siblings, 0 replies; 48+ messages in thread From: Mauro Carvalho Chehab @ 2018-09-12 16:01 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, Greg Kroah-Hartman, David S. Miller, devel, linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, netdev Em Wed, 12 Sep 2018 17:08:52 +0200 Arnd Bergmann <arnd@arndb.de> escreveu: > The .ioctl and .compat_ioctl file operations have the same prototype so > they can both point to the same function, which works great almost all > the time when all the commands are compatible. > > One exception is the s390 architecture, where a compat pointer is only > 31 bit wide, and converting it into a 64-bit pointer requires calling > compat_ptr(). Most drivers here will ever run in s390, but since we now > have a generic helper for it, it's easy enough to use it consistently. > > I double-checked all these drivers to ensure that all ioctl arguments > are used as pointers or are ignored, but are not interpreted as integer > values. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/media/rc/lirc_dev.c | 4 +--- > diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c > index f862f1b7f996..077209f414ed 100644 > --- a/drivers/media/rc/lirc_dev.c > +++ b/drivers/media/rc/lirc_dev.c > @@ -730,9 +730,7 @@ static const struct file_operations lirc_fops = { > .owner = THIS_MODULE, > .write = ir_lirc_transmit_ir, > .unlocked_ioctl = ir_lirc_ioctl, > -#ifdef CONFIG_COMPAT > - .compat_ioctl = ir_lirc_ioctl, > -#endif > + .compat_ioctl = generic_compat_ioctl_ptrarg, > .read = ir_lirc_read, > .poll = ir_lirc_poll, > .open = ir_lirc_open, Adding an infrared remote controller to a s390 mainframe sounds fun :-) I suspect that one could implement it on a s390 platform using gpio-ir-recv and/or gpio-ir-tx drivers. Perhaps one possible practical usage would be to let the mainframe to send remote controller codes to adjust the air conditioning system ;-) From lirc driver's PoV, there's nothing that really prevents one to do that and use lirc API, and the driver is generic enough to work on any hardware platform. I didn't check the implementation of generic_compat_ioctl_ptrarg(), but assuming it is ok, Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> Thanks, Mauro ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann ` (6 preceding siblings ...) 2018-09-12 16:01 ` Mauro Carvalho Chehab @ 2018-09-12 18:13 ` Greg Kroah-Hartman 2018-09-14 14:23 ` David Sterba ` (3 subsequent siblings) 11 siblings, 0 replies; 48+ messages in thread From: Greg Kroah-Hartman @ 2018-09-12 18:13 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, David S. Miller, devel, linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, netdev On Wed, Sep 12, 2018 at 05:08:52PM +0200, Arnd Bergmann wrote: > The .ioctl and .compat_ioctl file operations have the same prototype so > they can both point to the same function, which works great almost all > the time when all the commands are compatible. > > One exception is the s390 architecture, where a compat pointer is only > 31 bit wide, and converting it into a 64-bit pointer requires calling > compat_ptr(). Most drivers here will ever run in s390, but since we now > have a generic helper for it, it's easy enough to use it consistently. > > I double-checked all these drivers to ensure that all ioctl arguments > are used as pointers or are ignored, but are not interpreted as integer > values. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann ` (7 preceding siblings ...) 2018-09-12 18:13 ` Greg Kroah-Hartman @ 2018-09-14 14:23 ` David Sterba 2018-09-14 20:35 ` Darren Hart ` (2 subsequent siblings) 11 siblings, 0 replies; 48+ messages in thread From: David Sterba @ 2018-09-14 14:23 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, Greg Kroah-Hartman, David S. Miller, devel, linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, netdev On Wed, Sep 12, 2018 at 05:08:52PM +0200, Arnd Bergmann wrote: > The .ioctl and .compat_ioctl file operations have the same prototype so > they can both point to the same function, which works great almost all > the time when all the commands are compatible. > > One exception is the s390 architecture, where a compat pointer is only > 31 bit wide, and converting it into a 64-bit pointer requires calling > compat_ptr(). Most drivers here will ever run in s390, but since we now > have a generic helper for it, it's easy enough to use it consistently. > > I double-checked all these drivers to ensure that all ioctl arguments > are used as pointers or are ignored, but are not interpreted as integer > values. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > fs/btrfs/super.c | 2 +- Acked-by: David Sterba <dsterba@suse.com> ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann ` (8 preceding siblings ...) 2018-09-14 14:23 ` David Sterba @ 2018-09-14 20:35 ` Darren Hart 2018-09-14 20:57 ` Al Viro 2018-09-17 9:33 ` Jonathan Cameron 2018-10-06 7:05 ` Bjorn Andersson 11 siblings, 1 reply; 48+ messages in thread From: Darren Hart @ 2018-09-14 20:35 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, Greg Kroah-Hartman, David S. Miller, devel, linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, netdev On Wed, Sep 12, 2018 at 05:08:52PM +0200, Arnd Bergmann wrote: > The .ioctl and .compat_ioctl file operations have the same prototype so > they can both point to the same function, which works great almost all > the time when all the commands are compatible. > > One exception is the s390 architecture, where a compat pointer is only > 31 bit wide, and converting it into a 64-bit pointer requires calling > compat_ptr(). Most drivers here will ever run in s390, but since we now > have a generic helper for it, it's easy enough to use it consistently. > > I double-checked all these drivers to ensure that all ioctl arguments > are used as pointers or are ignored, but are not interpreted as integer > values. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- ... > drivers/platform/x86/wmi.c | 2 +- ... > static void link_event_work(struct work_struct *work) > diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c > index 04791ea5d97b..e4d0697e07d6 100644 > --- a/drivers/platform/x86/wmi.c > +++ b/drivers/platform/x86/wmi.c > @@ -886,7 +886,7 @@ static const struct file_operations wmi_fops = { > .read = wmi_char_read, > .open = wmi_char_open, > .unlocked_ioctl = wmi_ioctl, > - .compat_ioctl = wmi_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; For platform/drivers/x86: Acked-by: Darren Hart (VMware) <dvhart@infradead.org> As for a longer term solution, would it be possible to init fops in such a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg so we don't have to duplicate this boilerplate for every ioctl fops structure? -- Darren Hart VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-14 20:35 ` Darren Hart @ 2018-09-14 20:57 ` Al Viro 2018-09-18 17:51 ` Darren Hart 0 siblings, 1 reply; 48+ messages in thread From: Al Viro @ 2018-09-14 20:57 UTC (permalink / raw) To: Darren Hart Cc: Arnd Bergmann, linux-fsdevel, Greg Kroah-Hartman, David S. Miller, devel, linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, netdev On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > Acked-by: Darren Hart (VMware) <dvhart@infradead.org> > > As for a longer term solution, would it be possible to init fops in such > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > so we don't have to duplicate this boilerplate for every ioctl fops > structure? Bad idea, that... Because several years down the road somebody will add an ioctl that takes an unsigned int for argument. Without so much as looking at your magical mystery macro being used to initialize file_operations. FWIW, I would name that helper in more blunt way - something like compat_ioctl_only_compat_pointer_ioctls_here()... ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-14 20:57 ` Al Viro @ 2018-09-18 17:51 ` Darren Hart 2018-09-18 17:59 ` Jason Gunthorpe 0 siblings, 1 reply; 48+ messages in thread From: Darren Hart @ 2018-09-18 17:51 UTC (permalink / raw) To: Al Viro Cc: Arnd Bergmann, linux-fsdevel, Greg Kroah-Hartman, David S. Miller, devel, linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, netdev On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > Acked-by: Darren Hart (VMware) <dvhart@infradead.org> > > > > As for a longer term solution, would it be possible to init fops in such > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > so we don't have to duplicate this boilerplate for every ioctl fops > > structure? > > Bad idea, that... Because several years down the road somebody will add > an ioctl that takes an unsigned int for argument. Without so much as looking > at your magical mystery macro being used to initialize file_operations. Fair, being explicit in the declaration as it is currently may be preferable then. -- Darren Hart VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-18 17:51 ` Darren Hart @ 2018-09-18 17:59 ` Jason Gunthorpe 2018-09-24 20:18 ` Arnd Bergmann 0 siblings, 1 reply; 48+ messages in thread From: Jason Gunthorpe @ 2018-09-18 17:59 UTC (permalink / raw) To: Darren Hart Cc: Al Viro, Arnd Bergmann, linux-fsdevel, Greg Kroah-Hartman, David S. Miller, devel, linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, netdev On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > Acked-by: Darren Hart (VMware) <dvhart@infradead.org> > > > > > > As for a longer term solution, would it be possible to init fops in such > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > structure? > > > > Bad idea, that... Because several years down the road somebody will add > > an ioctl that takes an unsigned int for argument. Without so much as looking > > at your magical mystery macro being used to initialize file_operations. > > Fair, being explicit in the declaration as it is currently may be > preferable then. It would be much cleaner and safer if you could arrange things to add something like this to struct file_operations: long (*ptr_ioctl) (struct file *, unsigned int, void __user *); Where the core code automatically converts the unsigned long to the void __user * as appropriate. Then it just works right always and the compiler will help address Al's concern down the road. Cheers, Jason ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-18 17:59 ` Jason Gunthorpe @ 2018-09-24 20:18 ` Arnd Bergmann 2018-09-24 20:35 ` Jason Gunthorpe 0 siblings, 1 reply; 48+ messages in thread From: Arnd Bergmann @ 2018-09-24 20:18 UTC (permalink / raw) To: Jason Gunthorpe Cc: Darren Hart, Al Viro, Linux FS-devel Mailing List, gregkh, David Miller, driverdevel, Linux Kernel Mailing List, qat-linux, open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Linux Media Mailing List, dri-devel, linaro-mm-sig, amd-gfx, open list:HID CORE LAYER, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, Platform Driver, linux-remoteproc, sparclinux, linux-scsi, USB list, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, Networking On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > > > Acked-by: Darren Hart (VMware) <dvhart@infradead.org> > > > > > > > > As for a longer term solution, would it be possible to init fops in such > > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > > structure? > > > > > > Bad idea, that... Because several years down the road somebody will add > > > an ioctl that takes an unsigned int for argument. Without so much as looking > > > at your magical mystery macro being used to initialize file_operations. > > > > Fair, being explicit in the declaration as it is currently may be > > preferable then. > > It would be much cleaner and safer if you could arrange things to add > something like this to struct file_operations: > > long (*ptr_ioctl) (struct file *, unsigned int, void __user *); > > Where the core code automatically converts the unsigned long to the > void __user * as appropriate. > > Then it just works right always and the compiler will help address > Al's concern down the road. I think if we wanted to do this with a new file operation, the best way would be to do the copy_from_user()/copy_to_user() in the caller as well. We already do this inside of some subsystems, notably drivers/media/, and it simplifies the implementation of the ioctl handler function significantly. We obviously cannot do this in general, both because of traditional drivers that have 16-bit command codes (drivers/tty and others) and also because of drivers that by accident defined the commands incorrectly and use the wrong type or the wrong direction in the definition. Arnd ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-24 20:18 ` Arnd Bergmann @ 2018-09-24 20:35 ` Jason Gunthorpe 2018-09-24 21:17 ` Arnd Bergmann 0 siblings, 1 reply; 48+ messages in thread From: Jason Gunthorpe @ 2018-09-24 20:35 UTC (permalink / raw) To: Arnd Bergmann Cc: Darren Hart, Al Viro, Linux FS-devel Mailing List, gregkh, David Miller, driverdevel, Linux Kernel Mailing List, qat-linux, open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Linux Media Mailing List, dri-devel, linaro-mm-sig, amd-gfx, open list:HID CORE LAYER, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, Platform Driver, linux-remoteproc, sparclinux, linux-scsi, USB list, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, Networking On Mon, Sep 24, 2018 at 10:18:52PM +0200, Arnd Bergmann wrote: > On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > > > > > Acked-by: Darren Hart (VMware) <dvhart@infradead.org> > > > > > > > > > > As for a longer term solution, would it be possible to init fops in such > > > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > > > structure? > > > > > > > > Bad idea, that... Because several years down the road somebody will add > > > > an ioctl that takes an unsigned int for argument. Without so much as looking > > > > at your magical mystery macro being used to initialize file_operations. > > > > > > Fair, being explicit in the declaration as it is currently may be > > > preferable then. > > > > It would be much cleaner and safer if you could arrange things to add > > something like this to struct file_operations: > > > > long (*ptr_ioctl) (struct file *, unsigned int, void __user *); > > > > Where the core code automatically converts the unsigned long to the > > void __user * as appropriate. > > > > Then it just works right always and the compiler will help address > > Al's concern down the road. > > I think if we wanted to do this with a new file operation, the best > way would be to do the copy_from_user()/copy_to_user() in the caller > as well. > > We already do this inside of some subsystems, notably drivers/media/, > and it simplifies the implementation of the ioctl handler function > significantly. We obviously cannot do this in general, both because of > traditional drivers that have 16-bit command codes (drivers/tty and others) > and also because of drivers that by accident defined the commands > incorrectly and use the wrong type or the wrong direction in the > definition. That could work well, but the first idea could be done globally and mechanically, while this would require very careful per-driver investigation. Particularly if the core code has worse performance.. ie due to kmalloc calls or something. I think it would make more sense to start by having the core do the case to __user and then add another entry point to have the core do the copy_from_user, and so on. Jason ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-24 20:35 ` Jason Gunthorpe @ 2018-09-24 21:17 ` Arnd Bergmann 0 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-24 21:17 UTC (permalink / raw) To: Jason Gunthorpe Cc: Darren Hart, Al Viro, Linux FS-devel Mailing List, gregkh, David Miller, driverdevel, Linux Kernel Mailing List, qat-linux, open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Linux Media Mailing List, dri-devel, linaro-mm-sig, amd-gfx, open list:HID CORE LAYER, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, Platform Driver, linux-remoteproc, sparclinux, linux-scsi, USB list, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, Networking On Mon, Sep 24, 2018 at 10:35 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > On Mon, Sep 24, 2018 at 10:18:52PM +0200, Arnd Bergmann wrote: > > On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > We already do this inside of some subsystems, notably drivers/media/, > > and it simplifies the implementation of the ioctl handler function > > significantly. We obviously cannot do this in general, both because of > > traditional drivers that have 16-bit command codes (drivers/tty and others) > > and also because of drivers that by accident defined the commands > > incorrectly and use the wrong type or the wrong direction in the > > definition. > > That could work well, but the first idea could be done globally and > mechanically, while this would require very careful per-driver > investigation. > > Particularly if the core code has worse performance.. ie due to > kmalloc calls or something. > > I think it would make more sense to start by having the core do the > case to __user and then add another entry point to have the core do > the copy_from_user, and so on. Having six separate callback pointers to implement a single system call seems a bit excessive though. Arnd ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann ` (9 preceding siblings ...) 2018-09-14 20:35 ` Darren Hart @ 2018-09-17 9:33 ` Jonathan Cameron 2018-10-06 7:05 ` Bjorn Andersson 11 siblings, 0 replies; 48+ messages in thread From: Jonathan Cameron @ 2018-09-17 9:33 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, Greg Kroah-Hartman, David S. Miller, devel, linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, netdev On Wed, 12 Sep 2018 17:08:52 +0200 Arnd Bergmann <arnd@arndb.de> wrote: > The .ioctl and .compat_ioctl file operations have the same prototype so > they can both point to the same function, which works great almost all > the time when all the commands are compatible. > > One exception is the s390 architecture, where a compat pointer is only > 31 bit wide, and converting it into a 64-bit pointer requires calling > compat_ptr(). Most drivers here will ever run in s390, but since we now > have a generic helper for it, it's easy enough to use it consistently. > > I double-checked all these drivers to ensure that all ioctl arguments > are used as pointers or are ignored, but are not interpreted as integer > values. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- For IIO part. Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Thanks, > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index a062cfddc5af..22844b94b0e9 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -1630,7 +1630,7 @@ static const struct file_operations iio_buffer_fileops = { > .owner = THIS_MODULE, > .llseek = noop_llseek, > .unlocked_ioctl = iio_ioctl, > - .compat_ioctl = iio_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann ` (10 preceding siblings ...) 2018-09-17 9:33 ` Jonathan Cameron @ 2018-10-06 7:05 ` Bjorn Andersson 11 siblings, 0 replies; 48+ messages in thread From: Bjorn Andersson @ 2018-10-06 7:05 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, Greg Kroah-Hartman, David S. Miller, devel, linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs, ceph-devel, linux-wireless, netdev On Wed 12 Sep 08:08 PDT 2018, Arnd Bergmann wrote: [..] > diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c > index a76b963a7e50..02aefb2b2d47 100644 > --- a/drivers/rpmsg/rpmsg_char.c > +++ b/drivers/rpmsg/rpmsg_char.c > @@ -285,7 +285,7 @@ static const struct file_operations rpmsg_eptdev_fops = { > .write = rpmsg_eptdev_write, > .poll = rpmsg_eptdev_poll, > .unlocked_ioctl = rpmsg_eptdev_ioctl, > - .compat_ioctl = rpmsg_eptdev_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > > static ssize_t name_show(struct device *dev, struct device_attribute *attr, > @@ -446,7 +446,7 @@ static const struct file_operations rpmsg_ctrldev_fops = { > .open = rpmsg_ctrldev_open, > .release = rpmsg_ctrldev_release, > .unlocked_ioctl = rpmsg_ctrldev_ioctl, > - .compat_ioctl = rpmsg_ctrldev_ioctl, > + .compat_ioctl = generic_compat_ioctl_ptrarg, > }; > For rpmsg part Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls 2018-09-12 15:01 [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() Arnd Bergmann ` (3 preceding siblings ...) 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann @ 2018-09-12 15:13 ` Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 11/17] compat_ioctl: remove isdn ioctl translation Arnd Bergmann ` (7 more replies) 2018-09-13 2:07 ` [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() Al Viro 5 siblings, 8 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:13 UTC (permalink / raw) To: viro Cc: linux-fsdevel, Arnd Bergmann, Jeff Dike, Richard Weinberger, Jaroslav Kysela, Takashi Iwai, Mauro Carvalho Chehab, linux-um, linux-kernel, alsa-devel The SNDCTL_* and SOUND_* commands are the old OSS user interface. I checked all the sound ioctl commands listed in fs/compat_ioctl.c to see if we still need the translation handlers. Here is what I found: - sound/oss/ is (almost) gone from the kernel, this is what actually needed all the translations - The ALSA emulation for OSS correctly handles all compat_ioctl commands already. - sound/oss/dmasound/ is the last holdout of the original OSS code, this is only used on arch/m68k, which has no 64-bit mode and hence needs no compat handlers - arch/um/drivers/hostaudio_kern.c may run in 64-bit mode with 32-bit x86 user space underneath it. This rare corner case is the only one that still needs the compat handlers. By adding a simple redirect of .compat_ioctl to .unlocked_ioctl in the UML driver, we can remove all the COMPATIBLE_IOCTL() annotations without a change in functionality. For completeness, I'm adding the same thing to the dmasound file, knowing that it makes no difference. The compat_ioctl list contains one comment about SNDCTL_DSP_MAPINBUF and SNDCTL_DSP_MAPOUTBUF, which actually would need a translation handler if implemented. However, the native implementation just returns -EINVAL, so we don't care. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- arch/um/drivers/hostaudio_kern.c | 1 + fs/compat_ioctl.c | 157 ----------------------------- sound/core/oss/pcm_oss.c | 4 + sound/oss/dmasound/dmasound_core.c | 2 + 4 files changed, 7 insertions(+), 157 deletions(-) diff --git a/arch/um/drivers/hostaudio_kern.c b/arch/um/drivers/hostaudio_kern.c index 7f9dbdbc4eb7..9dfd209ed316 100644 --- a/arch/um/drivers/hostaudio_kern.c +++ b/arch/um/drivers/hostaudio_kern.c @@ -298,6 +298,7 @@ static const struct file_operations hostaudio_fops = { .write = hostaudio_write, .poll = hostaudio_poll, .unlocked_ioctl = hostaudio_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .mmap = NULL, .open = hostaudio_open, .release = hostaudio_release, diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index 001c9fd89d30..c28fd9f6b1e8 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -77,7 +77,6 @@ #include <linux/if_bonding.h> #include <linux/watchdog.h> -#include <linux/soundcard.h> #include <linux/lp.h> #include <linux/ppdev.h> @@ -775,162 +774,6 @@ COMPATIBLE_IOCTL(PPPIOCGL2TPSTATS) /* PPPOX */ COMPATIBLE_IOCTL(PPPOEIOCSFWD) COMPATIBLE_IOCTL(PPPOEIOCDFWD) -/* Big A */ -/* sparc only */ -/* Big Q for sound/OSS */ -COMPATIBLE_IOCTL(SNDCTL_SEQ_RESET) -COMPATIBLE_IOCTL(SNDCTL_SEQ_SYNC) -COMPATIBLE_IOCTL(SNDCTL_SYNTH_INFO) -COMPATIBLE_IOCTL(SNDCTL_SEQ_CTRLRATE) -COMPATIBLE_IOCTL(SNDCTL_SEQ_GETOUTCOUNT) -COMPATIBLE_IOCTL(SNDCTL_SEQ_GETINCOUNT) -COMPATIBLE_IOCTL(SNDCTL_SEQ_PERCMODE) -COMPATIBLE_IOCTL(SNDCTL_FM_LOAD_INSTR) -COMPATIBLE_IOCTL(SNDCTL_SEQ_TESTMIDI) -COMPATIBLE_IOCTL(SNDCTL_SEQ_RESETSAMPLES) -COMPATIBLE_IOCTL(SNDCTL_SEQ_NRSYNTHS) -COMPATIBLE_IOCTL(SNDCTL_SEQ_NRMIDIS) -COMPATIBLE_IOCTL(SNDCTL_MIDI_INFO) -COMPATIBLE_IOCTL(SNDCTL_SEQ_THRESHOLD) -COMPATIBLE_IOCTL(SNDCTL_SYNTH_MEMAVL) -COMPATIBLE_IOCTL(SNDCTL_FM_4OP_ENABLE) -COMPATIBLE_IOCTL(SNDCTL_SEQ_PANIC) -COMPATIBLE_IOCTL(SNDCTL_SEQ_OUTOFBAND) -COMPATIBLE_IOCTL(SNDCTL_SEQ_GETTIME) -COMPATIBLE_IOCTL(SNDCTL_SYNTH_ID) -COMPATIBLE_IOCTL(SNDCTL_SYNTH_CONTROL) -COMPATIBLE_IOCTL(SNDCTL_SYNTH_REMOVESAMPLE) -/* Big T for sound/OSS */ -COMPATIBLE_IOCTL(SNDCTL_TMR_TIMEBASE) -COMPATIBLE_IOCTL(SNDCTL_TMR_START) -COMPATIBLE_IOCTL(SNDCTL_TMR_STOP) -COMPATIBLE_IOCTL(SNDCTL_TMR_CONTINUE) -COMPATIBLE_IOCTL(SNDCTL_TMR_TEMPO) -COMPATIBLE_IOCTL(SNDCTL_TMR_SOURCE) -COMPATIBLE_IOCTL(SNDCTL_TMR_METRONOME) -COMPATIBLE_IOCTL(SNDCTL_TMR_SELECT) -/* Little m for sound/OSS */ -COMPATIBLE_IOCTL(SNDCTL_MIDI_PRETIME) -COMPATIBLE_IOCTL(SNDCTL_MIDI_MPUMODE) -COMPATIBLE_IOCTL(SNDCTL_MIDI_MPUCMD) -/* Big P for sound/OSS */ -COMPATIBLE_IOCTL(SNDCTL_DSP_RESET) -COMPATIBLE_IOCTL(SNDCTL_DSP_SYNC) -COMPATIBLE_IOCTL(SNDCTL_DSP_SPEED) -COMPATIBLE_IOCTL(SNDCTL_DSP_STEREO) -COMPATIBLE_IOCTL(SNDCTL_DSP_GETBLKSIZE) -COMPATIBLE_IOCTL(SNDCTL_DSP_CHANNELS) -COMPATIBLE_IOCTL(SOUND_PCM_WRITE_FILTER) -COMPATIBLE_IOCTL(SNDCTL_DSP_POST) -COMPATIBLE_IOCTL(SNDCTL_DSP_SUBDIVIDE) -COMPATIBLE_IOCTL(SNDCTL_DSP_SETFRAGMENT) -COMPATIBLE_IOCTL(SNDCTL_DSP_GETFMTS) -COMPATIBLE_IOCTL(SNDCTL_DSP_SETFMT) -COMPATIBLE_IOCTL(SNDCTL_DSP_GETOSPACE) -COMPATIBLE_IOCTL(SNDCTL_DSP_GETISPACE) -COMPATIBLE_IOCTL(SNDCTL_DSP_NONBLOCK) -COMPATIBLE_IOCTL(SNDCTL_DSP_GETCAPS) -COMPATIBLE_IOCTL(SNDCTL_DSP_GETTRIGGER) -COMPATIBLE_IOCTL(SNDCTL_DSP_SETTRIGGER) -COMPATIBLE_IOCTL(SNDCTL_DSP_GETIPTR) -COMPATIBLE_IOCTL(SNDCTL_DSP_GETOPTR) -/* SNDCTL_DSP_MAPINBUF, XXX needs translation */ -/* SNDCTL_DSP_MAPOUTBUF, XXX needs translation */ -COMPATIBLE_IOCTL(SNDCTL_DSP_SETSYNCRO) -COMPATIBLE_IOCTL(SNDCTL_DSP_SETDUPLEX) -COMPATIBLE_IOCTL(SNDCTL_DSP_GETODELAY) -COMPATIBLE_IOCTL(SNDCTL_DSP_PROFILE) -COMPATIBLE_IOCTL(SOUND_PCM_READ_RATE) -COMPATIBLE_IOCTL(SOUND_PCM_READ_CHANNELS) -COMPATIBLE_IOCTL(SOUND_PCM_READ_BITS) -COMPATIBLE_IOCTL(SOUND_PCM_READ_FILTER) -/* Big C for sound/OSS */ -COMPATIBLE_IOCTL(SNDCTL_COPR_RESET) -COMPATIBLE_IOCTL(SNDCTL_COPR_LOAD) -COMPATIBLE_IOCTL(SNDCTL_COPR_RDATA) -COMPATIBLE_IOCTL(SNDCTL_COPR_RCODE) -COMPATIBLE_IOCTL(SNDCTL_COPR_WDATA) -COMPATIBLE_IOCTL(SNDCTL_COPR_WCODE) -COMPATIBLE_IOCTL(SNDCTL_COPR_RUN) -COMPATIBLE_IOCTL(SNDCTL_COPR_HALT) -COMPATIBLE_IOCTL(SNDCTL_COPR_SENDMSG) -COMPATIBLE_IOCTL(SNDCTL_COPR_RCVMSG) -/* Big M for sound/OSS */ -COMPATIBLE_IOCTL(SOUND_MIXER_READ_VOLUME) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_BASS) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_TREBLE) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_SYNTH) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_PCM) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_SPEAKER) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_LINE) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_MIC) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_CD) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_IMIX) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_ALTPCM) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_RECLEV) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_IGAIN) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_OGAIN) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_LINE1) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_LINE2) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_LINE3) -COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_DIGITAL1)) -COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_DIGITAL2)) -COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_DIGITAL3)) -COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_PHONEIN)) -COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_PHONEOUT)) -COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_VIDEO)) -COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_RADIO)) -COMPATIBLE_IOCTL(MIXER_READ(SOUND_MIXER_MONITOR)) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_MUTE) -/* SOUND_MIXER_READ_ENHANCE, same value as READ_MUTE */ -/* SOUND_MIXER_READ_LOUD, same value as READ_MUTE */ -COMPATIBLE_IOCTL(SOUND_MIXER_READ_RECSRC) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_DEVMASK) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_RECMASK) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_STEREODEVS) -COMPATIBLE_IOCTL(SOUND_MIXER_READ_CAPS) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_VOLUME) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_BASS) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_TREBLE) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_SYNTH) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_PCM) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_SPEAKER) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_LINE) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_MIC) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_CD) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_IMIX) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_ALTPCM) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_RECLEV) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_IGAIN) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_OGAIN) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_LINE1) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_LINE2) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_LINE3) -COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_DIGITAL1)) -COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_DIGITAL2)) -COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_DIGITAL3)) -COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_PHONEIN)) -COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_PHONEOUT)) -COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_VIDEO)) -COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_RADIO)) -COMPATIBLE_IOCTL(MIXER_WRITE(SOUND_MIXER_MONITOR)) -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_MUTE) -/* SOUND_MIXER_WRITE_ENHANCE, same value as WRITE_MUTE */ -/* SOUND_MIXER_WRITE_LOUD, same value as WRITE_MUTE */ -COMPATIBLE_IOCTL(SOUND_MIXER_WRITE_RECSRC) -COMPATIBLE_IOCTL(SOUND_MIXER_INFO) -COMPATIBLE_IOCTL(SOUND_OLD_MIXER_INFO) -COMPATIBLE_IOCTL(SOUND_MIXER_ACCESS) -COMPATIBLE_IOCTL(SOUND_MIXER_AGC) -COMPATIBLE_IOCTL(SOUND_MIXER_3DSE) -COMPATIBLE_IOCTL(SOUND_MIXER_PRIVATE1) -COMPATIBLE_IOCTL(SOUND_MIXER_PRIVATE2) -COMPATIBLE_IOCTL(SOUND_MIXER_PRIVATE3) -COMPATIBLE_IOCTL(SOUND_MIXER_PRIVATE4) -COMPATIBLE_IOCTL(SOUND_MIXER_PRIVATE5) -COMPATIBLE_IOCTL(SOUND_MIXER_GETLEVELS) -COMPATIBLE_IOCTL(SOUND_MIXER_SETLEVELS) -COMPATIBLE_IOCTL(OSS_GETVERSION) /* Raw devices */ COMPATIBLE_IOCTL(RAW_SETBIND) COMPATIBLE_IOCTL(RAW_GETBIND) diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c index f8d4a419f3af..3f25c9d7b7df 100644 --- a/sound/core/oss/pcm_oss.c +++ b/sound/core/oss/pcm_oss.c @@ -2732,6 +2732,10 @@ static long snd_pcm_oss_ioctl(struct file *file, unsigned int cmd, unsigned long static long snd_pcm_oss_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg) { + /* + * Everything is compatbile except SNDCTL_DSP_MAPINBUF/SNDCTL_DSP_MAPOUTBUF, + * which are not implemented for the native case either + */ return snd_pcm_oss_ioctl(file, cmd, (unsigned long)compat_ptr(arg)); } #else diff --git a/sound/oss/dmasound/dmasound_core.c b/sound/oss/dmasound/dmasound_core.c index fc9bcd47d6a4..cda6bbfedd22 100644 --- a/sound/oss/dmasound/dmasound_core.c +++ b/sound/oss/dmasound/dmasound_core.c @@ -384,6 +384,7 @@ static const struct file_operations mixer_fops = .owner = THIS_MODULE, .llseek = no_llseek, .unlocked_ioctl = mixer_unlocked_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = mixer_open, .release = mixer_release, }; @@ -1167,6 +1168,7 @@ static const struct file_operations sq_fops = .write = sq_write, .poll = sq_poll, .unlocked_ioctl = sq_unlocked_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .open = sq_open, .release = sq_release, }; -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 11/17] compat_ioctl: remove isdn ioctl translation 2018-09-12 15:13 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Arnd Bergmann @ 2018-09-12 15:13 ` Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 12/17] compat_ioctl: remove IGNORE_IOCTL() Arnd Bergmann ` (6 subsequent siblings) 7 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:13 UTC (permalink / raw) To: viro Cc: linux-fsdevel, Arnd Bergmann, Karsten Keil, Paul Bolle, Randy Dunlap, David S. Miller, netdev, linux-kernel, gigaset307x-common Neither the old isdn4linux interface nor the newer mISDN stack ever had working 32-bit compat mode as far as I can tell. However, the CAPI stack and the Gigaset ISDN driver (implemented on top of either CAPI or i4l) have some ioctl commands that are correctly listed in fs/compat_ioctl.c. We can trivially move all of those into the two corresponding files that implement the native handlers by adding a compat_ioctl redirect to that. I did notice that treating CAPI_MANUFACTURER_CMD() as compatible is broken, so I'm also adding a handler for that, realizing that in all likelyhood, nobody is ever going to call it. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/isdn/capi/capi.c | 31 +++++++++++++++++++++++++++++++ drivers/isdn/gigaset/interface.c | 11 +++++++++++ fs/compat_ioctl.c | 22 ---------------------- 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c index ef5560b848ab..4851dc3941eb 100644 --- a/drivers/isdn/capi/capi.c +++ b/drivers/isdn/capi/capi.c @@ -942,6 +942,34 @@ capi_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return ret; } +#ifdef CONFIG_COMPAT +static long +capi_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +{ + int ret; + + if (cmd == CAPI_MANUFACTURER_CMD) { + struct { + unsigned long cmd; + compat_uptr_t data; + } mcmd32; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + if (copy_from_user(&mcmd32, compat_ptr(arg), sizeof(mcmd32))) + return -EFAULT; + + mutex_lock(&capi_mutex); + ret = capi20_manufacturer(mcmd32.cmd, compat_ptr(mcmd32.data)); + mutex_unlock(&capi_mutex); + + return ret; + } + + return capi_unlocked_ioctl(file, cmd, (unsigned long)compat_ptr(arg)); +} +#endif + static int capi_open(struct inode *inode, struct file *file) { struct capidev *cdev; @@ -988,6 +1016,9 @@ static const struct file_operations capi_fops = .write = capi_write, .poll = capi_poll, .unlocked_ioctl = capi_unlocked_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = capi_compat_ioctl, +#endif .open = capi_open, .release = capi_release, }; diff --git a/drivers/isdn/gigaset/interface.c b/drivers/isdn/gigaset/interface.c index 600c79b030cd..e346756b0b45 100644 --- a/drivers/isdn/gigaset/interface.c +++ b/drivers/isdn/gigaset/interface.c @@ -233,6 +233,14 @@ static int if_ioctl(struct tty_struct *tty, return retval; } +#ifdef CONFIG_COMPAT +static long if_compat_ioctl(struct tty_struct *tty, + unsigned int cmd, unsigned long arg) +{ + return if_ioctl(tty, cmd, (unsigned long)compat_ptr(arg)); +} +#endif + static int if_tiocmget(struct tty_struct *tty) { struct cardstate *cs = tty->driver_data; @@ -472,6 +480,9 @@ static const struct tty_operations if_ops = { .open = if_open, .close = if_close, .ioctl = if_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = if_compat_ioctl, +#endif .write = if_write, .write_room = if_write_room, .chars_in_buffer = if_chars_in_buffer, diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index c28fd9f6b1e8..379e04647f83 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -60,9 +60,6 @@ #include <net/bluetooth/hci_sock.h> #include <net/bluetooth/rfcomm.h> -#include <linux/capi.h> -#include <linux/gigaset_dev.h> - #ifdef CONFIG_BLOCK #include <linux/cdrom.h> #include <linux/fd.h> @@ -840,25 +837,6 @@ COMPATIBLE_IOCTL(HIDPCONNADD) COMPATIBLE_IOCTL(HIDPCONNDEL) COMPATIBLE_IOCTL(HIDPGETCONNLIST) COMPATIBLE_IOCTL(HIDPGETCONNINFO) -/* CAPI */ -COMPATIBLE_IOCTL(CAPI_REGISTER) -COMPATIBLE_IOCTL(CAPI_GET_MANUFACTURER) -COMPATIBLE_IOCTL(CAPI_GET_VERSION) -COMPATIBLE_IOCTL(CAPI_GET_SERIAL) -COMPATIBLE_IOCTL(CAPI_GET_PROFILE) -COMPATIBLE_IOCTL(CAPI_MANUFACTURER_CMD) -COMPATIBLE_IOCTL(CAPI_GET_ERRCODE) -COMPATIBLE_IOCTL(CAPI_INSTALLED) -COMPATIBLE_IOCTL(CAPI_GET_FLAGS) -COMPATIBLE_IOCTL(CAPI_SET_FLAGS) -COMPATIBLE_IOCTL(CAPI_CLR_FLAGS) -COMPATIBLE_IOCTL(CAPI_NCCI_OPENCOUNT) -COMPATIBLE_IOCTL(CAPI_NCCI_GETUNIT) -/* Siemens Gigaset */ -COMPATIBLE_IOCTL(GIGASET_REDIR) -COMPATIBLE_IOCTL(GIGASET_CONFIG) -COMPATIBLE_IOCTL(GIGASET_BRKCHARS) -COMPATIBLE_IOCTL(GIGASET_VERSION) /* Misc. */ COMPATIBLE_IOCTL(0x41545900) /* ATYIO_CLKR */ COMPATIBLE_IOCTL(0x41545901) /* ATYIO_CLKW */ -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 12/17] compat_ioctl: remove IGNORE_IOCTL() 2018-09-12 15:13 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 11/17] compat_ioctl: remove isdn ioctl translation Arnd Bergmann @ 2018-09-12 15:13 ` Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 13/17] compat_ioctl: remove /dev/random commands Arnd Bergmann ` (5 subsequent siblings) 7 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:13 UTC (permalink / raw) To: viro; +Cc: linux-fsdevel, Arnd Bergmann, linux-kernel Since commit 07d106d0a33d ("vfs: fix up ENOIOCTLCMD error handling"), we don't warn about unhandled compat-ioctl command code any more, but just return the same error that a native file descriptor returns when there is no handler. This means the IGNORE_IOCTL() annotations are completely useless and can all be removed. TIOCSTART/TIOCSTOP and KDGHWCLK/KDSHWCLK fall into the same category, but for some reason were listed as COMPATIBLE_IOCTL(). Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/compat_ioctl.c | 56 ----------------------------------------------- 1 file changed, 56 deletions(-) diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index 379e04647f83..ca7f83976a3f 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -604,20 +604,7 @@ static int compat_ioctl_preallocate(struct file *file, #define XFORM(i) (((i) ^ ((i) << 27) ^ ((i) << 17)) & 0xffffffff) #define COMPATIBLE_IOCTL(cmd) XFORM((u32)cmd), -/* ioctl should not be warned about even if it's not implemented. - Valid reasons to use this: - - It is implemented with ->compat_ioctl on some device, but programs - call it on others too. - - The ioctl is not implemented in the native kernel, but programs - call it commonly anyways. - Most other reasons are not valid. */ -#define IGNORE_IOCTL(cmd) COMPATIBLE_IOCTL(cmd) - static unsigned int ioctl_pointer[] = { -/* compatible ioctls first */ -COMPATIBLE_IOCTL(0x4B50) /* KDGHWCLK - not in the kernel, but don't complain */ -COMPATIBLE_IOCTL(0x4B51) /* KDSHWCLK - not in the kernel, but don't complain */ - /* Big T */ COMPATIBLE_IOCTL(TCGETA) COMPATIBLE_IOCTL(TCSETA) @@ -694,9 +681,6 @@ COMPATIBLE_IOCTL(SCSI_IOCTL_SEND_COMMAND) COMPATIBLE_IOCTL(SCSI_IOCTL_PROBE_HOST) COMPATIBLE_IOCTL(SCSI_IOCTL_GET_PCI) #endif -/* Big V (don't complain on serial console) */ -IGNORE_IOCTL(VT_OPENQRY) -IGNORE_IOCTL(VT_GETMODE) /* * These two are only for the sbus rtc driver, but * hwclock tries them on every rtc device first when @@ -708,11 +692,6 @@ COMPATIBLE_IOCTL(_IOW('p', 21, int[7])) /* RTCSET */ /* Socket level stuff */ COMPATIBLE_IOCTL(FIOQSIZE) #ifdef CONFIG_BLOCK -/* md calls this on random blockdevs */ -IGNORE_IOCTL(RAID_VERSION) -/* qemu/qemu-img might call these two on plain files for probing */ -IGNORE_IOCTL(CDROM_DRIVE_STATUS) -IGNORE_IOCTL(FDGETPRM32) /* SG stuff */ COMPATIBLE_IOCTL(SG_SET_TIMEOUT) COMPATIBLE_IOCTL(SG_GET_TIMEOUT) @@ -908,41 +887,6 @@ COMPATIBLE_IOCTL(JSIOCGNAME(0)) COMPATIBLE_IOCTL(TIOCGLTC) COMPATIBLE_IOCTL(TIOCSLTC) #endif -#ifdef TIOCSTART -/* - * For these two we have definitions in ioctls.h and/or termios.h on - * some architectures but no actual implemention. Some applications - * like bash call them if they are defined in the headers, so we provide - * entries here to avoid syslog message spew. - */ -COMPATIBLE_IOCTL(TIOCSTART) -COMPATIBLE_IOCTL(TIOCSTOP) -#endif - -/* fat 'r' ioctls. These are handled by fat with ->compat_ioctl, - but we don't want warnings on other file systems. So declare - them as compatible here. */ -#define VFAT_IOCTL_READDIR_BOTH32 _IOR('r', 1, struct compat_dirent[2]) -#define VFAT_IOCTL_READDIR_SHORT32 _IOR('r', 2, struct compat_dirent[2]) - -IGNORE_IOCTL(VFAT_IOCTL_READDIR_BOTH32) -IGNORE_IOCTL(VFAT_IOCTL_READDIR_SHORT32) - -#ifdef CONFIG_SPARC -/* Sparc framebuffers, handled in sbusfb_compat_ioctl() */ -IGNORE_IOCTL(FBIOGTYPE) -IGNORE_IOCTL(FBIOSATTR) -IGNORE_IOCTL(FBIOGATTR) -IGNORE_IOCTL(FBIOSVIDEO) -IGNORE_IOCTL(FBIOGVIDEO) -IGNORE_IOCTL(FBIOSCURPOS) -IGNORE_IOCTL(FBIOGCURPOS) -IGNORE_IOCTL(FBIOGCURMAX) -IGNORE_IOCTL(FBIOPUTCMAP32) -IGNORE_IOCTL(FBIOGETCMAP32) -IGNORE_IOCTL(FBIOSCURSOR32) -IGNORE_IOCTL(FBIOGCURSOR32) -#endif }; /* -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 13/17] compat_ioctl: remove /dev/random commands 2018-09-12 15:13 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 11/17] compat_ioctl: remove isdn ioctl translation Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 12/17] compat_ioctl: remove IGNORE_IOCTL() Arnd Bergmann @ 2018-09-12 15:13 ` Arnd Bergmann 2018-09-12 18:13 ` Greg Kroah-Hartman 2018-09-12 15:13 ` [PATCH v2 14/17] compat_ioctl: remove joystick ioctl translation Arnd Bergmann ` (4 subsequent siblings) 7 siblings, 1 reply; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:13 UTC (permalink / raw) To: viro Cc: linux-fsdevel, Arnd Bergmann, Theodore Ts'o, Greg Kroah-Hartman, Jann Horn, linux-kernel These are all handled by the random driver, so instead of listing each ioctl, we can just use the same function to deal with both native and compat commands. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/char/random.c | 1 + fs/compat_ioctl.c | 7 ------- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index bf5f99fc36f1..d1b2cdebc598 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -2021,6 +2021,7 @@ const struct file_operations random_fops = { .write = random_write, .poll = random_poll, .unlocked_ioctl = random_ioctl, + .compat_ioctl = generic_compat_ioctl_ptrarg, .fasync = random_fasync, .llseek = noop_llseek, }; diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index ca7f83976a3f..d5b8380fb1b1 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -764,13 +764,6 @@ COMPATIBLE_IOCTL(WDIOC_SETTIMEOUT) COMPATIBLE_IOCTL(WDIOC_GETTIMEOUT) COMPATIBLE_IOCTL(WDIOC_SETPRETIMEOUT) COMPATIBLE_IOCTL(WDIOC_GETPRETIMEOUT) -/* Big R */ -COMPATIBLE_IOCTL(RNDGETENTCNT) -COMPATIBLE_IOCTL(RNDADDTOENTCNT) -COMPATIBLE_IOCTL(RNDGETPOOL) -COMPATIBLE_IOCTL(RNDADDENTROPY) -COMPATIBLE_IOCTL(RNDZAPENTCNT) -COMPATIBLE_IOCTL(RNDCLEARPOOL) /* Bluetooth */ COMPATIBLE_IOCTL(HCIDEVUP) COMPATIBLE_IOCTL(HCIDEVDOWN) -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v2 13/17] compat_ioctl: remove /dev/random commands 2018-09-12 15:13 ` [PATCH v2 13/17] compat_ioctl: remove /dev/random commands Arnd Bergmann @ 2018-09-12 18:13 ` Greg Kroah-Hartman 0 siblings, 0 replies; 48+ messages in thread From: Greg Kroah-Hartman @ 2018-09-12 18:13 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, linux-fsdevel, Theodore Ts'o, Jann Horn, linux-kernel On Wed, Sep 12, 2018 at 05:13:05PM +0200, Arnd Bergmann wrote: > These are all handled by the random driver, so instead of listing > each ioctl, we can just use the same function to deal with both > native and compat commands. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/char/random.c | 1 + > fs/compat_ioctl.c | 7 ------- > 2 files changed, 1 insertion(+), 7 deletions(-) Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 14/17] compat_ioctl: remove joystick ioctl translation 2018-09-12 15:13 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Arnd Bergmann ` (2 preceding siblings ...) 2018-09-12 15:13 ` [PATCH v2 13/17] compat_ioctl: remove /dev/random commands Arnd Bergmann @ 2018-09-12 15:13 ` Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 15/17] compat_ioctl: remove PCI " Arnd Bergmann ` (3 subsequent siblings) 7 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:13 UTC (permalink / raw) To: viro; +Cc: linux-fsdevel, Arnd Bergmann, linux-kernel The joystick driver already handles these just fine, so the entries in the table are not needed any more. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/compat_ioctl.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index d5b8380fb1b1..0e3a879339c2 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -11,8 +11,6 @@ * ioctls. */ -#include <linux/joystick.h> - #include <linux/types.h> #include <linux/compat.h> #include <linux/kernel.h> @@ -870,12 +868,6 @@ COMPATIBLE_IOCTL(CEC_S_MODE) COMPATIBLE_IOCTL(CEC_TRANSMIT) COMPATIBLE_IOCTL(CEC_RECEIVE) COMPATIBLE_IOCTL(CEC_DQEVENT) -/* joystick */ -COMPATIBLE_IOCTL(JSIOCGVERSION) -COMPATIBLE_IOCTL(JSIOCGAXES) -COMPATIBLE_IOCTL(JSIOCGBUTTONS) -COMPATIBLE_IOCTL(JSIOCGNAME(0)) - #ifdef TIOCGLTC COMPATIBLE_IOCTL(TIOCGLTC) COMPATIBLE_IOCTL(TIOCSLTC) -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 15/17] compat_ioctl: remove PCI ioctl translation 2018-09-12 15:13 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Arnd Bergmann ` (3 preceding siblings ...) 2018-09-12 15:13 ` [PATCH v2 14/17] compat_ioctl: remove joystick ioctl translation Arnd Bergmann @ 2018-09-12 15:13 ` Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 16/17] compat_ioctl: remove /dev/raw " Arnd Bergmann ` (2 subsequent siblings) 7 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:13 UTC (permalink / raw) To: viro; +Cc: linux-fsdevel, Arnd Bergmann, linux-kernel The /proc/pci/ implementation already handles these just fine, so the entries in the table are not needed any more. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/compat_ioctl.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index 0e3a879339c2..8510cd652bb0 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -43,7 +43,6 @@ #include <linux/raw.h> #include <linux/blkdev.h> #include <linux/elevator.h> -#include <linux/pci.h> #include <linux/serial.h> #include <linux/if_tun.h> #include <linux/ctype.h> @@ -810,10 +809,6 @@ COMPATIBLE_IOCTL(HIDPGETCONNINFO) /* Misc. */ COMPATIBLE_IOCTL(0x41545900) /* ATYIO_CLKR */ COMPATIBLE_IOCTL(0x41545901) /* ATYIO_CLKW */ -COMPATIBLE_IOCTL(PCIIOC_CONTROLLER) -COMPATIBLE_IOCTL(PCIIOC_MMAP_IS_IO) -COMPATIBLE_IOCTL(PCIIOC_MMAP_IS_MEM) -COMPATIBLE_IOCTL(PCIIOC_WRITE_COMBINE) /* dvb */ COMPATIBLE_IOCTL(AUDIO_STOP) COMPATIBLE_IOCTL(AUDIO_PLAY) -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 16/17] compat_ioctl: remove /dev/raw ioctl translation 2018-09-12 15:13 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Arnd Bergmann ` (4 preceding siblings ...) 2018-09-12 15:13 ` [PATCH v2 15/17] compat_ioctl: remove PCI " Arnd Bergmann @ 2018-09-12 15:13 ` Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 17/17] compat_ioctl: remove last RAID handling code Arnd Bergmann 2018-09-13 13:37 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Takashi Iwai 7 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:13 UTC (permalink / raw) To: viro; +Cc: linux-fsdevel, Arnd Bergmann, linux-kernel The /dev/rawX implementation already handles these just fine, so the entries in the table are not needed any more. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/compat_ioctl.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index 8510cd652bb0..79ad7c3afd70 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -40,7 +40,6 @@ #include <linux/fb.h> #include <linux/videodev2.h> #include <linux/netdevice.h> -#include <linux/raw.h> #include <linux/blkdev.h> #include <linux/elevator.h> #include <linux/serial.h> @@ -747,9 +746,6 @@ COMPATIBLE_IOCTL(PPPIOCGL2TPSTATS) /* PPPOX */ COMPATIBLE_IOCTL(PPPOEIOCSFWD) COMPATIBLE_IOCTL(PPPOEIOCDFWD) -/* Raw devices */ -COMPATIBLE_IOCTL(RAW_SETBIND) -COMPATIBLE_IOCTL(RAW_GETBIND) /* Watchdog */ COMPATIBLE_IOCTL(WDIOC_GETSUPPORT) COMPATIBLE_IOCTL(WDIOC_GETSTATUS) -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 17/17] compat_ioctl: remove last RAID handling code 2018-09-12 15:13 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Arnd Bergmann ` (5 preceding siblings ...) 2018-09-12 15:13 ` [PATCH v2 16/17] compat_ioctl: remove /dev/raw " Arnd Bergmann @ 2018-09-12 15:13 ` Arnd Bergmann 2018-09-13 13:37 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Takashi Iwai 7 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-09-12 15:13 UTC (permalink / raw) To: viro; +Cc: linux-fsdevel, Arnd Bergmann, linux-kernel Commit aa98aa31987a ("md: move compat_ioctl handling into md.c") already removed the COMPATIBLE_IOCTL() table entries and added a complete implementation, but a few lines got left behind and should also be removed here. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/compat_ioctl.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index 79ad7c3afd70..37862bf91f44 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -911,11 +911,6 @@ static long do_ioctl_trans(unsigned int cmd, case TCSBRKP: case TIOCMIWAIT: case TIOCSCTTY: - /* RAID */ - case HOT_REMOVE_DISK: - case HOT_ADD_DISK: - case SET_DISK_FAULTY: - case SET_BITMAP_FILE: return vfs_ioctl(file, cmd, arg); } -- 2.18.0 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls 2018-09-12 15:13 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Arnd Bergmann ` (6 preceding siblings ...) 2018-09-12 15:13 ` [PATCH v2 17/17] compat_ioctl: remove last RAID handling code Arnd Bergmann @ 2018-09-13 13:37 ` Takashi Iwai 7 siblings, 0 replies; 48+ messages in thread From: Takashi Iwai @ 2018-09-13 13:37 UTC (permalink / raw) To: Arnd Bergmann Cc: viro, Jeff Dike, alsa-devel, Mauro Carvalho Chehab, linux-um, Richard Weinberger, Jaroslav Kysela, linux-fsdevel, linux-kernel On Wed, 12 Sep 2018 17:13:02 +0200, Arnd Bergmann wrote: > > The SNDCTL_* and SOUND_* commands are the old OSS user interface. > > I checked all the sound ioctl commands listed in fs/compat_ioctl.c > to see if we still need the translation handlers. Here is what I > found: > > - sound/oss/ is (almost) gone from the kernel, this is what actually > needed all the translations > - The ALSA emulation for OSS correctly handles all compat_ioctl > commands already. > - sound/oss/dmasound/ is the last holdout of the original OSS code, > this is only used on arch/m68k, which has no 64-bit mode and > hence needs no compat handlers > - arch/um/drivers/hostaudio_kern.c may run in 64-bit mode with > 32-bit x86 user space underneath it. This rare corner case is > the only one that still needs the compat handlers. > > By adding a simple redirect of .compat_ioctl to .unlocked_ioctl in the > UML driver, we can remove all the COMPATIBLE_IOCTL() annotations without > a change in functionality. For completeness, I'm adding the same thing > to the dmasound file, knowing that it makes no difference. > > The compat_ioctl list contains one comment about SNDCTL_DSP_MAPINBUF and > SNDCTL_DSP_MAPOUTBUF, which actually would need a translation handler > if implemented. However, the native implementation just returns -EINVAL, > so we don't care. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Looks good to me. Reviewed-by: Takashi Iwai <tiwai@suse.de> thanks, Takashi ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() 2018-09-12 15:01 [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() Arnd Bergmann ` (4 preceding siblings ...) 2018-09-12 15:13 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Arnd Bergmann @ 2018-09-13 2:07 ` Al Viro 2018-09-13 10:29 ` Arnd Bergmann 5 siblings, 1 reply; 48+ messages in thread From: Al Viro @ 2018-09-13 2:07 UTC (permalink / raw) To: Arnd Bergmann; +Cc: linux-fsdevel, linux-kernel On Wed, Sep 12, 2018 at 05:01:02PM +0200, Arnd Bergmann wrote: > Many drivers have ioctl() handlers that are completely compatible > between 32-bit and 64-bit architectures, except for the argument > that is passed down from user space and may have to be passed > through compat_ptr() in order to become a valid 64-bit pointer. > > Using ".compat_ptr=generic_compat_ioctl_ptrarg" in file operations > should let us simplify a lot of those drivers to avoid #ifdef > checks, and convert additional drivers that don't have proper > compat handling yet. Just keep in mind that this should *only* be used when all ioctls implemented in a given instance do take pointers. Because otherwise you are asking for trouble - e.g. if one of them takes an u32 used as a bitmap, this will run into trouble as soon as somebody uses bit 31. With no visible warnings. IOW, it shouldn't be used blindly and it should come with big fat warning. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() 2018-09-13 2:07 ` [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() Al Viro @ 2018-09-13 10:29 ` Arnd Bergmann 2018-10-28 17:07 ` Al Viro 0 siblings, 1 reply; 48+ messages in thread From: Arnd Bergmann @ 2018-09-13 10:29 UTC (permalink / raw) To: Al Viro; +Cc: Linux FS-devel Mailing List, Linux Kernel Mailing List On Thu, Sep 13, 2018 at 4:07 AM Al Viro <viro@zeniv.linux.org.uk> wrote: > > On Wed, Sep 12, 2018 at 05:01:02PM +0200, Arnd Bergmann wrote: > > Many drivers have ioctl() handlers that are completely compatible > > between 32-bit and 64-bit architectures, except for the argument > > that is passed down from user space and may have to be passed > > through compat_ptr() in order to become a valid 64-bit pointer. > > > > Using ".compat_ptr=generic_compat_ioctl_ptrarg" in file operations > > should let us simplify a lot of those drivers to avoid #ifdef > > checks, and convert additional drivers that don't have proper > > compat handling yet. > > Just keep in mind that this should *only* be used when all > ioctls implemented in a given instance do take pointers. > Because otherwise you are asking for trouble - e.g. if one of > them takes an u32 used as a bitmap, this will run into trouble > as soon as somebody uses bit 31. With no visible warnings. > > IOW, it shouldn't be used blindly and it should come with big > fat warning. I was hoping that the _ptrarg suffix gives enough warning here, but maybe not. I was careful to only use it in cases that I checked are safe, either using only pointer arguments, or no arguments. What we might do for further clarification (besides adding a comment next to the declaration), would be to add a complementary generic_compat_ioctl_intarg() that skips the compat_ptr(). There are only a handful of drivers that would use this though. Arnd ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() 2018-09-13 10:29 ` Arnd Bergmann @ 2018-10-28 17:07 ` Al Viro 2018-10-29 9:50 ` Arnd Bergmann 0 siblings, 1 reply; 48+ messages in thread From: Al Viro @ 2018-10-28 17:07 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Linux FS-devel Mailing List, Linux Kernel Mailing List On Thu, Sep 13, 2018 at 12:29:02PM +0200, Arnd Bergmann wrote: > I was hoping that the _ptrarg suffix gives enough warning here, > but maybe not. I was careful to only use it in cases that I > checked are safe, either using only pointer arguments, or > no arguments. > > What we might do for further clarification (besides adding a > comment next to the declaration), would be to add a > complementary generic_compat_ioctl_intarg() that skips > the compat_ptr(). There are only a handful of drivers that > would use this though. ... and the next Monday zeniv went down until the end of September, so I'd missed any resends that might've happened in that window. It's _probably_ too late for this cycle, but let's deal with that thing properly for the next one. A couple of comments from rereading the thread: * generic_compat_ioctl_fthagn^H^H^H^H^H^Hptrarg should not be EXPORT_SYMBOL_GPL(). I'm sorry, but this is beyond ridiculous - "call native ioctl, with the last argument interpreted as an address from 32bit process POV and converted to 64bit equivalent" should not be copyrightable at all, and there's really only one natural way to express that. Use EXPORT_SYMBOL(). And I'd consider names like compat_ptr_ioctl() - easier to type and less opaque... * rtc patch makes RTC_IRQP_SET32 et.al. accepted by 64bit syscall. Which is a behaviour change that might or might not be OK, but it needs to be clearly stated. Could you resend the series, with ACKs attached, etc., either based on -next (if done now) or on -rc1 (once released)? ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() 2018-10-28 17:07 ` Al Viro @ 2018-10-29 9:50 ` Arnd Bergmann 0 siblings, 0 replies; 48+ messages in thread From: Arnd Bergmann @ 2018-10-29 9:50 UTC (permalink / raw) To: Al Viro; +Cc: Linux FS-devel Mailing List, Linux Kernel Mailing List On Sun, Oct 28, 2018 at 6:07 PM Al Viro <viro@zeniv.linux.org.uk> wrote: > > On Thu, Sep 13, 2018 at 12:29:02PM +0200, Arnd Bergmann wrote: > > > I was hoping that the _ptrarg suffix gives enough warning here, > > but maybe not. I was careful to only use it in cases that I > > checked are safe, either using only pointer arguments, or > > no arguments. > > > > What we might do for further clarification (besides adding a > > comment next to the declaration), would be to add a > > complementary generic_compat_ioctl_intarg() that skips > > the compat_ptr(). There are only a handful of drivers that > > would use this though. > > ... and the next Monday zeniv went down until the end of September, > so I'd missed any resends that might've happened in that window. > > It's _probably_ too late for this cycle, but let's deal with that > thing properly for the next one. Right, I don't think we're in a hurry, so I'll rebase my patches once -rc1 is out and send you the new version. > A couple of comments from rereading the thread: > * generic_compat_ioctl_fthagn^H^H^H^H^H^Hptrarg should not > be EXPORT_SYMBOL_GPL(). I'm sorry, but this is beyond ridiculous - > "call native ioctl, with the last argument interpreted as an address > from 32bit process POV and converted to 64bit equivalent" should > not be copyrightable at all, and there's really only one natural > way to express that. Use EXPORT_SYMBOL(). Sure, I didn't really give this much thought. > And I'd consider names > like compat_ptr_ioctl() - easier to type and less opaque... Good idea. > * rtc patch makes RTC_IRQP_SET32 et.al. accepted by 64bit > syscall. Which is a behaviour change that might or might not be > OK, but it needs to be clearly stated. Yes. I was aware of the change in behavior and have done similar changes for simplicity on y2038 ioctl patches in the past, but you are right that this should be mentioned in the changelog. Arnd ^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2018-10-29 18:38 UTC | newest] Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-12 15:01 [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() Arnd Bergmann 2018-09-12 15:01 ` [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg Arnd Bergmann 2018-09-12 15:33 ` Jason Gunthorpe 2018-09-12 16:20 ` Arnd Bergmann 2018-09-12 18:13 ` Greg Kroah-Hartman 2018-09-16 19:07 ` Jarkko Sakkinen 2018-09-12 15:01 ` [PATCH v2 03/17] compat_ioctl: use correct compat_ptr() translation in drivers Arnd Bergmann 2018-09-12 18:13 ` Greg Kroah-Hartman 2018-09-13 0:48 ` Andrew Donnellan 2018-09-13 11:03 ` Felipe Balbi 2018-09-12 15:01 ` [PATCH v2 04/17] ceph: fix compat_ioctl for ceph_dir_operations Arnd Bergmann 2018-09-12 16:12 ` David Laight 2018-09-12 16:25 ` Arnd Bergmann 2018-09-13 0:48 ` Yan, Zheng 2018-09-12 15:08 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Arnd Bergmann 2018-09-12 15:08 ` [PATCH v2 06/17] compat_ioctl: move rtc handling into rtc-dev.c Arnd Bergmann 2018-09-12 20:00 ` Alexandre Belloni 2018-09-12 15:08 ` [PATCH v2 07/17] compat_ioctl: move tape handling into drivers Arnd Bergmann 2018-09-12 15:08 ` [PATCH v2 08/17] compat_ioctl: remove keyboard ioctl translation Arnd Bergmann 2018-09-12 15:08 ` [PATCH v2 09/17] compat_ioctl: remove HIDIO translation Arnd Bergmann 2018-09-12 15:56 ` [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Jason Gunthorpe 2018-09-12 15:58 ` Daniel Vetter 2018-09-12 16:01 ` Mauro Carvalho Chehab 2018-09-12 18:13 ` Greg Kroah-Hartman 2018-09-14 14:23 ` David Sterba 2018-09-14 20:35 ` Darren Hart 2018-09-14 20:57 ` Al Viro 2018-09-18 17:51 ` Darren Hart 2018-09-18 17:59 ` Jason Gunthorpe 2018-09-24 20:18 ` Arnd Bergmann 2018-09-24 20:35 ` Jason Gunthorpe 2018-09-24 21:17 ` Arnd Bergmann 2018-09-17 9:33 ` Jonathan Cameron 2018-10-06 7:05 ` Bjorn Andersson 2018-09-12 15:13 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 11/17] compat_ioctl: remove isdn ioctl translation Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 12/17] compat_ioctl: remove IGNORE_IOCTL() Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 13/17] compat_ioctl: remove /dev/random commands Arnd Bergmann 2018-09-12 18:13 ` Greg Kroah-Hartman 2018-09-12 15:13 ` [PATCH v2 14/17] compat_ioctl: remove joystick ioctl translation Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 15/17] compat_ioctl: remove PCI " Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 16/17] compat_ioctl: remove /dev/raw " Arnd Bergmann 2018-09-12 15:13 ` [PATCH v2 17/17] compat_ioctl: remove last RAID handling code Arnd Bergmann 2018-09-13 13:37 ` [PATCH v2 10/17] compat_ioctl: remove translation for sound ioctls Takashi Iwai 2018-09-13 2:07 ` [PATCH v2 01/17] compat_ioctl: add generic_compat_ioctl_ptrarg() Al Viro 2018-09-13 10:29 ` Arnd Bergmann 2018-10-28 17:07 ` Al Viro 2018-10-29 9:50 ` 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).