[PATCH v3 00/26] compat_ioctl: cleanups

[PATCH v3 00/26] compat_ioctl: cleanups

[Arnd Bergmann]

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

[PATCH v3 10/26] compat_ioctl: use correct compat_ptr() translation in drivers

[Arnd Bergmann]

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 compat_ptr_ioctl()
helper makes it more correct in theory, and simplifies the code.

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
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 4d6836f19489..cb9cca35a226 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	= compat_ptr_ioctl,
 	.release	= device_close,
 };
 
diff --git a/drivers/misc/genwqe/card_dev.c b/drivers/misc/genwqe/card_dev.c
index 8c1b63a4337b..5de0796f2786 100644
--- a/drivers/misc/genwqe/card_dev.c
+++ b/drivers/misc/genwqe/card_dev.c
@@ -1221,34 +1221,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   = compat_ptr_ioctl,
 	.release	= genwqe_release,
 };
 
diff --git a/drivers/scsi/megaraid/megaraid_mm.c b/drivers/scsi/megaraid/megaraid_mm.c
index 3ce837e4b24c..21ee5751c04e 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 = compat_ptr_ioctl,
 	.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);
@@ -1232,25 +1225,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 20413c276c61..addc210d198a 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1347,14 +1347,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,
 
@@ -1363,9 +1355,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 = compat_ptr_ioctl,
 };
 
 
-- 
2.20.0

Re: [PATCH v3 00/26] compat_ioctl: cleanups

[Douglas Gilbert]

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
> 

Re: [PATCH v3 00/26] compat_ioctl: cleanups

[Arnd Bergmann]

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

Re: [PATCH v3 10/26] compat_ioctl: use correct compat_ptr() translation in drivers

[Al Viro]

On Tue, Apr 16, 2019 at 10:19:48PM +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 compat_ptr_ioctl()
> helper makes it more correct in theory, and simplifies the code.
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> 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 +-----------

ACK, provided that all their ioctls are taking pointers (which is
worth mentioning in commit message, assuming it's true)

Re: [PATCH v3 00/26] compat_ioctl: cleanups

[Andy Shevchenko]

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