* [PATCH v3 00/26] compat_ioctl: cleanups @ 2019-04-16 20:19 Arnd Bergmann [not found] ` <20190416202701.127745-1-arnd@arndb.de> ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Arnd Bergmann @ 2019-04-16 20:19 UTC (permalink / raw) To: Alexander Viro Cc: linux-fsdevel, y2038, linux-kernel, Arnd Bergmann, David S. Miller, Greg Kroah-Hartman, Karsten Keil, James E.J. Bottomley, Martin K. Petersen, Marcel Holtmann, netdev, devel, linux-integrity, qat-linux, linux-crypto, linux-media, dri-devel, linux1394-devel, amd-gfx, linux-input, linux-usb, linux-arm-kernel, linux-ide, linux-iio Hi Al, It took me way longer than I had hoped to revisit this series, see https://lore.kernel.org/lkml/20180912150142.157913-1-arnd@arndb.de/ for the previously posted version. I've come to the point where all conversion handlers and most COMPATIBLE_IOCTL() entries are gone from this file, but for now, this series only has the parts that have either been reviewed previously, or that are simple enough to include. The main missing piece is the SG_IO/SG_GET_REQUEST_TABLE conversion. I'll post the patches I made for that later, as they need more testing and review from the scsi maintainers. I hope you can still take these for the coming merge window, unless new problems come up. Arnd Arnd Bergmann (26): compat_ioctl: pppoe: fix PPPOEIOCSFWD handling compat_ioctl: move simple ppp command handling into driver compat_ioctl: avoid unused function warning for do_ioctl compat_ioctl: move PPPIOCSCOMPRESS32 to ppp-generic.c compat_ioctl: move PPPIOCSPASS32/PPPIOCSACTIVE32 to ppp_generic.c compat_ioctl: handle PPPIOCGIDLE for 64-bit time_t compat_ioctl: move rtc handling into rtc-dev.c compat_ioctl: add compat_ptr_ioctl() compat_ioctl: move drivers to compat_ptr_ioctl compat_ioctl: use correct compat_ptr() translation in drivers ceph: fix compat_ioctl for ceph_dir_operations compat_ioctl: move more drivers to compat_ptr_ioctl compat_ioctl: move tape handling into drivers compat_ioctl: move ATYFB_CLK handling to atyfb driver compat_ioctl: move isdn/capi ioctl translation into driver compat_ioctl: move rfcomm handlers into driver compat_ioctl: move hci_sock handlers into driver compat_ioctl: remove HCIUART handling compat_ioctl: remove HIDIO translation compat_ioctl: remove translation for sound ioctls compat_ioctl: remove IGNORE_IOCTL() compat_ioctl: remove /dev/random commands compat_ioctl: remove joystick ioctl translation compat_ioctl: remove PCI ioctl translation compat_ioctl: remove /dev/raw ioctl translation compat_ioctl: remove last RAID handling code Documentation/networking/ppp_generic.txt | 2 + arch/um/drivers/hostaudio_kern.c | 1 + drivers/android/binder.c | 2 +- drivers/char/ppdev.c | 12 +- drivers/char/random.c | 1 + drivers/char/tpm/tpm_vtpm_proxy.c | 12 +- 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/firewire/core-cdev.c | 12 +- drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +- drivers/hid/hidraw.c | 4 +- drivers/hid/usbhid/hiddev.c | 11 +- drivers/hwtracing/stm/core.c | 12 +- drivers/ide/ide-tape.c | 31 +- drivers/iio/industrialio-core.c | 2 +- drivers/infiniband/core/uverbs_main.c | 4 +- drivers/isdn/capi/capi.c | 31 + drivers/isdn/i4l/isdn_ppp.c | 14 +- drivers/media/rc/lirc_dev.c | 4 +- drivers/mfd/cros_ec_dev.c | 4 +- drivers/misc/cxl/flash.c | 8 +- drivers/misc/genwqe/card_dev.c | 23 +- drivers/misc/mei/main.c | 22 +- drivers/misc/vmw_vmci/vmci_host.c | 2 +- drivers/mtd/ubi/cdev.c | 36 +- drivers/net/ppp/ppp_generic.c | 99 +++- drivers/net/ppp/pppoe.c | 7 + drivers/net/ppp/pptp.c | 3 + drivers/net/tap.c | 12 +- 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/rtc/dev.c | 13 +- drivers/rtc/rtc-vr41xx.c | 10 + drivers/s390/char/tape_char.c | 41 +- 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/megaraid/megaraid_mm.c | 28 +- drivers/scsi/osst.c | 34 +- drivers/scsi/pmcraid.c | 4 +- drivers/scsi/st.c | 35 +- drivers/staging/android/ion/ion.c | 4 +- drivers/staging/pi433/pi433_if.c | 12 +- 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/usb/core/devio.c | 16 +- drivers/usb/gadget/function/f_fs.c | 12 +- 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 +- drivers/video/fbdev/aty/atyfb_base.c | 12 +- drivers/virt/fsl_hypervisor.c | 2 +- fs/btrfs/super.c | 2 +- fs/ceph/dir.c | 1 + fs/ceph/file.c | 2 +- fs/compat_ioctl.c | 602 +------------------- fs/fat/file.c | 13 +- fs/fuse/dev.c | 2 +- fs/notify/fanotify/fanotify_user.c | 2 +- fs/userfaultfd.c | 2 +- include/linux/fs.h | 7 + include/linux/if_pppox.h | 2 + include/linux/mtio.h | 58 ++ include/uapi/linux/ppp-ioctl.h | 2 + include/uapi/linux/ppp_defs.h | 14 + net/bluetooth/hci_sock.c | 21 +- net/bluetooth/rfcomm/sock.c | 14 +- net/l2tp/l2tp_ppp.c | 3 + net/rfkill/core.c | 2 +- sound/core/oss/pcm_oss.c | 4 + sound/oss/dmasound/dmasound_core.c | 2 + 82 files changed, 452 insertions(+), 1034 deletions(-) create mode 100644 include/linux/mtio.h -- 2.20.0 Cc: "David S. Miller" <davem@davemloft.net> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Karsten Keil <isdn@linux-pingi.de> Cc: "James E.J. Bottomley" <jejb@linux.ibm.com> Cc: "Martin K. Petersen" <martin.petersen@oracle.com> Cc: Marcel Holtmann <marcel@holtmann.org> Cc: netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: devel@driverdev.osuosl.org Cc: linux-integrity@vger.kernel.org Cc: qat-linux@intel.com Cc: linux-crypto@vger.kernel.org Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux1394-devel@lists.sourceforge.net Cc: amd-gfx@lists.freedesktop.org Cc: linux-input@vger.kernel.org Cc: linux-usb@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-ide@vger.kernel.org Cc: linux-iio@vger.kernel.org Cc: linux-rdma@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-mtd@lists.infradead.org Cc: linux-ppp@vger.kernel.org Cc: linux-nvme@lists.infradead.org Cc: platform-driver-x86@vger.kernel.org Cc: linux-remoteproc@vger.kernel.org Cc: linux-rtc@vger.kernel.org Cc: linux-s390@vger.kernel.org Cc: sparclinux@vger.kernel.org Cc: linux-scsi@vger.kernel.org Cc: linux-fbdev@vger.kernel.org Cc: linux-btrfs@vger.kernel.org Cc: ceph-devel@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org Cc: linux-bluetooth@vger.kernel.org Cc: linux-wireless@vger.kernel.org Cc: alsa-devel@alsa-project.org Cc: y2038@lists.linaro.org ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <20190416202701.127745-1-arnd@arndb.de>]
* [PATCH v3 13/26] compat_ioctl: move tape handling into drivers [not found] ` <20190416202701.127745-1-arnd@arndb.de> @ 2019-04-16 20:25 ` Arnd Bergmann 0 siblings, 0 replies; 5+ messages in thread From: Arnd Bergmann @ 2019-04-16 20:25 UTC (permalink / raw) To: Alexander Viro Cc: linux-fsdevel, y2038, linux-kernel, Arnd Bergmann, David S. Miller, Martin Schwidefsky, Heiko Carstens, Willem Riede, James E.J. Bottomley, Martin K. Petersen, Kai Mäkisara, 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 | 70 ----------------------------------- include/linux/mtio.h | 58 +++++++++++++++++++++++++++++ 6 files changed, 137 insertions(+), 132 deletions(-) create mode 100644 include/linux/mtio.h diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index db1a65f4b490..54560ae5c054 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 be3c73ebbfde..edfde6edfc18 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 19c022e66d63..f0cb35964a30 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> @@ -3502,7 +3503,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; @@ -3536,6 +3537,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; @@ -3745,7 +3747,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; } @@ -3800,19 +3803,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; } @@ -3821,9 +3821,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); @@ -3857,14 +3856,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 ab2ff530313b..6eb7a3f51702 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -27,7 +27,6 @@ #include <linux/file.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/raw.h> @@ -279,70 +278,6 @@ static int sg_grt_trans(struct file *file, return err; } -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 */ @@ -443,8 +378,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 @@ -775,9 +708,6 @@ static long do_ioctl_trans(unsigned int cmd, return sg_ioctl_trans(file, cmd, compat_ptr(arg)); case SG_GET_REQUEST_TABLE: return sg_grt_trans(file, cmd, compat_ptr(arg)); - case MTIOCGET32: - case MTIOCPOS32: - return mt_ioctl_trans(file, cmd, compat_ptr(arg)); #endif } 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.20.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 00/26] compat_ioctl: cleanups 2019-04-16 20:19 [PATCH v3 00/26] compat_ioctl: cleanups Arnd Bergmann [not found] ` <20190416202701.127745-1-arnd@arndb.de> @ 2019-04-16 22:33 ` Douglas Gilbert 2019-04-17 9:26 ` Arnd Bergmann 2019-05-06 9:03 ` Andy Shevchenko 2 siblings, 1 reply; 5+ messages in thread From: Douglas Gilbert @ 2019-04-16 22:33 UTC (permalink / raw) To: Arnd Bergmann, Alexander Viro Cc: linux-fsdevel, y2038, linux-kernel, David S. Miller, Greg Kroah-Hartman, Karsten Keil, James E.J. Bottomley, Martin K. Petersen, Marcel Holtmann, netdev, devel, linux-integrity, qat-linux, linux-crypto, linux-media, dri-devel, linux1394-devel, amd-gfx, linux-input, linux-usb, linux-arm-kernel, linux-ide, linux-iio, linux-rdma On 2019-04-16 4:19 p.m., Arnd Bergmann wrote: > Hi Al, > > It took me way longer than I had hoped to revisit this series, see > https://lore.kernel.org/lkml/20180912150142.157913-1-arnd@arndb.de/ > for the previously posted version. > > I've come to the point where all conversion handlers and most > COMPATIBLE_IOCTL() entries are gone from this file, but for > now, this series only has the parts that have either been reviewed > previously, or that are simple enough to include. > > The main missing piece is the SG_IO/SG_GET_REQUEST_TABLE conversion. > I'll post the patches I made for that later, as they need more > testing and review from the scsi maintainers. Perhaps you could look at the document in this url: http://sg.danny.cz/sg/sg_v40.html It is work-in-progress to modernize the SCSI generic driver. It extends ioctl(sg_fd, SG_IO, &pt_obj) to additionally accept the sg v4 interface as defined in include/uapi/linux/bsg.h . Currently only the bsg driver uses the sg v4 interface. Since struct sg_io_v4 is all explicitly sized integers, I'm guessing it is immune "compat" problems. [I can see no reference to bsg nor struct sg_io_v4 in the current fs/compat_ioctl.c file.] Other additions described in the that document are these new ioctls: - SG_IOSUBMIT ultimately to replace write(sg_fd, ...) - SG_IORECEIVE to replace read(sg_fd, ...) - SG_IOABORT abort SCSI cmd in progress; new functionality - SG_SET_GET_EXTENDED has associated struct sg_extended_info The first three take a pointer to a struct sg_io_hdr (v3 interface) or a struct sg_io_v4 object. Both objects start with a 32 bit integer: 'S' identifies the v3 interface while 'Q' identifies the v4 interface. The SG_SET_GET_EXTENDED ioctl takes a pointer to a struct sg_extended_info object which contains explicitly sized integers so it may also be immune from "compat" problems. The ioctls section (13) of that document referenced above has a table showing how many "sets and gets" are hiding in the SG_SET_GET_EXTENDED ioctl. BTW No change is proposed for this case: ioctl(normal_block_device, SG_IO, &sg_v3_obj) which is handled by block/scsi_ioctl.c This would be a good time for me to address any "compat" concerns in the proposed sg driver update. Doug Gilbert > I hope you can still take these for the coming merge window, unless > new problems come up. > > Arnd > > Arnd Bergmann (26): > compat_ioctl: pppoe: fix PPPOEIOCSFWD handling > compat_ioctl: move simple ppp command handling into driver > compat_ioctl: avoid unused function warning for do_ioctl > compat_ioctl: move PPPIOCSCOMPRESS32 to ppp-generic.c > compat_ioctl: move PPPIOCSPASS32/PPPIOCSACTIVE32 to ppp_generic.c > compat_ioctl: handle PPPIOCGIDLE for 64-bit time_t > compat_ioctl: move rtc handling into rtc-dev.c > compat_ioctl: add compat_ptr_ioctl() > compat_ioctl: move drivers to compat_ptr_ioctl > compat_ioctl: use correct compat_ptr() translation in drivers > ceph: fix compat_ioctl for ceph_dir_operations > compat_ioctl: move more drivers to compat_ptr_ioctl > compat_ioctl: move tape handling into drivers > compat_ioctl: move ATYFB_CLK handling to atyfb driver > compat_ioctl: move isdn/capi ioctl translation into driver > compat_ioctl: move rfcomm handlers into driver > compat_ioctl: move hci_sock handlers into driver > compat_ioctl: remove HCIUART handling > compat_ioctl: remove HIDIO translation > compat_ioctl: remove translation for sound ioctls > compat_ioctl: remove IGNORE_IOCTL() > compat_ioctl: remove /dev/random commands > compat_ioctl: remove joystick ioctl translation > compat_ioctl: remove PCI ioctl translation > compat_ioctl: remove /dev/raw ioctl translation > compat_ioctl: remove last RAID handling code > > Documentation/networking/ppp_generic.txt | 2 + > arch/um/drivers/hostaudio_kern.c | 1 + > drivers/android/binder.c | 2 +- > drivers/char/ppdev.c | 12 +- > drivers/char/random.c | 1 + > drivers/char/tpm/tpm_vtpm_proxy.c | 12 +- > 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/firewire/core-cdev.c | 12 +- > drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +- > drivers/hid/hidraw.c | 4 +- > drivers/hid/usbhid/hiddev.c | 11 +- > drivers/hwtracing/stm/core.c | 12 +- > drivers/ide/ide-tape.c | 31 +- > drivers/iio/industrialio-core.c | 2 +- > drivers/infiniband/core/uverbs_main.c | 4 +- > drivers/isdn/capi/capi.c | 31 + > drivers/isdn/i4l/isdn_ppp.c | 14 +- > drivers/media/rc/lirc_dev.c | 4 +- > drivers/mfd/cros_ec_dev.c | 4 +- > drivers/misc/cxl/flash.c | 8 +- > drivers/misc/genwqe/card_dev.c | 23 +- > drivers/misc/mei/main.c | 22 +- > drivers/misc/vmw_vmci/vmci_host.c | 2 +- > drivers/mtd/ubi/cdev.c | 36 +- > drivers/net/ppp/ppp_generic.c | 99 +++- > drivers/net/ppp/pppoe.c | 7 + > drivers/net/ppp/pptp.c | 3 + > drivers/net/tap.c | 12 +- > 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/rtc/dev.c | 13 +- > drivers/rtc/rtc-vr41xx.c | 10 + > drivers/s390/char/tape_char.c | 41 +- > 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/megaraid/megaraid_mm.c | 28 +- > drivers/scsi/osst.c | 34 +- > drivers/scsi/pmcraid.c | 4 +- > drivers/scsi/st.c | 35 +- > drivers/staging/android/ion/ion.c | 4 +- > drivers/staging/pi433/pi433_if.c | 12 +- > 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/usb/core/devio.c | 16 +- > drivers/usb/gadget/function/f_fs.c | 12 +- > 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 +- > drivers/video/fbdev/aty/atyfb_base.c | 12 +- > drivers/virt/fsl_hypervisor.c | 2 +- > fs/btrfs/super.c | 2 +- > fs/ceph/dir.c | 1 + > fs/ceph/file.c | 2 +- > fs/compat_ioctl.c | 602 +------------------- > fs/fat/file.c | 13 +- > fs/fuse/dev.c | 2 +- > fs/notify/fanotify/fanotify_user.c | 2 +- > fs/userfaultfd.c | 2 +- > include/linux/fs.h | 7 + > include/linux/if_pppox.h | 2 + > include/linux/mtio.h | 58 ++ > include/uapi/linux/ppp-ioctl.h | 2 + > include/uapi/linux/ppp_defs.h | 14 + > net/bluetooth/hci_sock.c | 21 +- > net/bluetooth/rfcomm/sock.c | 14 +- > net/l2tp/l2tp_ppp.c | 3 + > net/rfkill/core.c | 2 +- > sound/core/oss/pcm_oss.c | 4 + > sound/oss/dmasound/dmasound_core.c | 2 + > 82 files changed, 452 insertions(+), 1034 deletions(-) > create mode 100644 include/linux/mtio.h > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 00/26] compat_ioctl: cleanups 2019-04-16 22:33 ` [PATCH v3 00/26] compat_ioctl: cleanups Douglas Gilbert @ 2019-04-17 9:26 ` Arnd Bergmann 0 siblings, 0 replies; 5+ messages in thread From: Arnd Bergmann @ 2019-04-17 9:26 UTC (permalink / raw) To: dgilbert Cc: linux-nvme, linux-iio, linux-remoteproc, Linux Fbdev development list, dri-devel, Platform Driver, IDE-ML, linux-mtd, sparclinux, linux1394-devel, driverdevel, linux-s390, linux-scsi, Bluez mailing list, y2038 Mailman List, qat-linux, amd-gfx, open list:HID CORE LAYER, Marcel Holtmann, Linux Media Mailing List, linux-rtc, ALSA On Wed, Apr 17, 2019 at 12:33 AM Douglas Gilbert <dgilbert@interlog.com> wrote: > > On 2019-04-16 4:19 p.m., Arnd Bergmann wrote: > > Hi Al, > > > > It took me way longer than I had hoped to revisit this series, see > > https://lore.kernel.org/lkml/20180912150142.157913-1-arnd@arndb.de/ > > for the previously posted version. > > > > I've come to the point where all conversion handlers and most > > COMPATIBLE_IOCTL() entries are gone from this file, but for > > now, this series only has the parts that have either been reviewed > > previously, or that are simple enough to include. > > > > The main missing piece is the SG_IO/SG_GET_REQUEST_TABLE conversion. > > I'll post the patches I made for that later, as they need more > > testing and review from the scsi maintainers. > > Perhaps you could look at the document in this url: > http://sg.danny.cz/sg/sg_v40.html > > It is work-in-progress to modernize the SCSI generic driver. It > extends ioctl(sg_fd, SG_IO, &pt_obj) to additionally accept the sg v4 > interface as defined in include/uapi/linux/bsg.h . Currently only the > bsg driver uses the sg v4 interface. Since struct sg_io_v4 is all > explicitly sized integers, I'm guessing it is immune "compat" problems. > [I can see no reference to bsg nor struct sg_io_v4 in the current > fs/compat_ioctl.c file.] Ok, I've taken a brief look at your series now. Unfortunately it clashes quite hard with my series, but it's probably for the better to have your stuff get merged first. A few (unsorted) comments from going through your patches: - the added ioctls are all compatible when using the v4 structures and mostly don't need handlers for compat mode, but they need to be called from .compat_ioctl to actually be usable in compat mode. With my patches you get that. - One exception for the v4 layout is the use of iovec pointers, as 'struct iovec' is incompatible. We should probably merge the generic compat_import_iovec() into import_iovec() with a 'in_compat_syscall()' check, which would be helpful in general. bsg.c does not iovec, so it is not affected by this at the moment, maybe it would be better to stay compatible with that and also not support them in sg.c? - Is there a need for the new sg_ioctl_iosubmit/sg_ioctl_ioreceive to support the v3 structures? Those are /not/ compatible, so you need extra code to handle the v3-compat layout as well. Supporting only v4 would simplify this. - the lack of changeset descriptions is a bit irritating and makes it much harder to understand what you are doing. - try to keep patches that move code around separate from those that change it in any other way, for better reviewing. - in "sg: preparation for request sharing", you seem to inadvertently change the size of "struct sg_extended_info", making it 4 bytes longer by adding two members. - You should never use IS_ERR_OR_NULL() in normal code, that is just a sign of a bad API. Make each function have consistent error behavior. - The "#if 0 /* temporary to shorten big patch */" trick breaks bisection, that is probably worse than the larger patch. - The split access_ok()/__copy_from_user() has fallen out of favor because it has caused too many bugs in the past, just use the combined copy_from_user() instead. - ktime_to_ns(ktime_get_with_offset(TK_OFFS_BOOT)) followed by a 64-bit division won't work on 32-bit machines, use ktime_get_boottime_ts64() instead. > Other additions described in the that document are these new ioctls: > - SG_IOSUBMIT ultimately to replace write(sg_fd, ...) > - SG_IORECEIVE to replace read(sg_fd, ...) > - SG_IOABORT abort SCSI cmd in progress; new functionality > - SG_SET_GET_EXTENDED has associated struct sg_extended_info > > The first three take a pointer to a struct sg_io_hdr (v3 interface) or > a struct sg_io_v4 object. Both objects start with a 32 bit integer: > 'S' identifies the v3 interface while 'Q' identifies the v4 interface. I think the magic character was a mistake in the original design, just like versioned interfaces in general. If you are extending an interface in an incompatible way, the normal way would be to have separate command codes, like SG_IORECEIVE_V3 and SG_IORECEIVE_V4, if you absolutely have to maintain compatiblity with the old interface (which I think you don't in case of SG_IORECEIVE). For SG_IO, I can see why you want to support both the v3 and v4 structures plus the compat-v3 version, but I'd try to keep them as separate as possible, and do something like static int sg_ctl_sg_io(struct file *filp, struct sg_device *sdp, struct sg_fd *sfp, void __user *p) { int ret; ret = sg_io_v4(filp, sdp, sfp, (struct sg_io_v4 __user *)p); if (ret != -ENOIOCTLCMD || !S_ENABLED(CONFIG_SG_IO_V3)) return ret; if (in_compat_syscall()) ret = sg_io_compat_(filp, sdp, sfp, (struct compat_sg_io_hdr __user *)p); else ret = sg_io_v3(filp, sdp, sfp, (struct sg_io_hdr __user *)p); } In my patch series, I combined the latter two cases and used a shared get_sg_io_hdr()/put_sg_io_hdr() helper as well as a wrapper for the iovec issue. > The SG_SET_GET_EXTENDED ioctl takes a pointer to a struct > sg_extended_info object which contains explicitly sized integers so it > may also be immune from "compat" problems. The ioctls section (13) of > that document referenced above has a table showing how many "sets and > gets" are hiding in the SG_SET_GET_EXTENDED ioctl. Agreed, SG_SET_GET_EXTENDED looks fine to me from a compat perspective. I've uploaded my patches to git://git.kernel.org:/pub/scm/linux/kernel/git/arnd/playground.git compat-ioctl-v3 This contains both the series I posted here, and my scsi ioctl rework. Maybe you can take the bits you need from that to handle the v3-compat structures and integrate it into your series? Arnd _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 00/26] compat_ioctl: cleanups 2019-04-16 20:19 [PATCH v3 00/26] compat_ioctl: cleanups Arnd Bergmann [not found] ` <20190416202701.127745-1-arnd@arndb.de> 2019-04-16 22:33 ` [PATCH v3 00/26] compat_ioctl: cleanups Douglas Gilbert @ 2019-05-06 9:03 ` Andy Shevchenko 2 siblings, 0 replies; 5+ messages in thread From: Andy Shevchenko @ 2019-05-06 9:03 UTC (permalink / raw) To: Arnd Bergmann Cc: Linux NVMe Mailinglist, linux-iio, linux-remoteproc, linux-fbdev, dri-devel, Platform Driver, linux-ide, open list:MEMORY TECHNOLOGY..., sparclinux, linux1394-devel, devel, linux-s390, linux-scsi, linux-bluetooth, y2038, qat-linux, amd-gfx, linux-input, Marcel Holtmann, Linux Media Mailing List, open list:REAL TIME CLOCK (RTC) SUBSYSTEM, ALSA Development Mailing List On Tue, Apr 16, 2019 at 11:23 PM Arnd Bergmann <arnd@arndb.de> wrote: > > Hi Al, > > It took me way longer than I had hoped to revisit this series, see > https://lore.kernel.org/lkml/20180912150142.157913-1-arnd@arndb.de/ > for the previously posted version. > > I've come to the point where all conversion handlers and most > COMPATIBLE_IOCTL() entries are gone from this file, but for > now, this series only has the parts that have either been reviewed > previously, or that are simple enough to include. > > The main missing piece is the SG_IO/SG_GET_REQUEST_TABLE conversion. > I'll post the patches I made for that later, as they need more > testing and review from the scsi maintainers. > > I hope you can still take these for the coming merge window, unless > new problems come up. > drivers/platform/x86/wmi.c | 2 +- Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> -- With Best Regards, Andy Shevchenko _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-06 9:03 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-16 20:19 [PATCH v3 00/26] compat_ioctl: cleanups Arnd Bergmann [not found] ` <20190416202701.127745-1-arnd@arndb.de> 2019-04-16 20:25 ` [PATCH v3 13/26] compat_ioctl: move tape handling into drivers Arnd Bergmann 2019-04-16 22:33 ` [PATCH v3 00/26] compat_ioctl: cleanups Douglas Gilbert 2019-04-17 9:26 ` Arnd Bergmann 2019-05-06 9:03 ` Andy Shevchenko
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).