[PATCH 00/24] block, scsi: final compat_ioctl cleanup

[PATCH 00/24] block, scsi: final compat_ioctl cleanup

[Arnd Bergmann]

Hi Jens, James and Martin,

This series concludes the work I did for linux-5.5 on the compat_ioctl()
cleanup, killing off fs/compat_ioctl.c and block/compat_ioctl.c by moving
everything into drivers.

Overall this would be a reduction both in complexity and line count, but
as I'm also adding documentation the overall number of lines increases
in the end.

My plan was originally to keep the SCSI and block parts separate.
This did not work easily because of interdependencies: I cannot
do the final SCSI cleanup in a good way without first addressing the
CDROM ioctls, so this is one series that I hope could be merged through
either the block or the scsi git trees, or possibly both if you can
pull in the same branch.

The series comes in these steps:

1. clean up the sg v3 interface as suggested by Linus. I have
   talked about this with Doug Gilbert as well, and he would
   rebase his sg v4 patches on top of "compat: scsi: sg: fix v3
   compat read/write interface"

2. Four patches for missing block compat_ioctl handlers, to be
   backported into stable kernels. Separate patches because they
   are needed in different stable versions.

3. Actually moving handlers out of block/compat_ioctl.c and
   block/scsi_ioctl.c into drivers, mixed in with cleanup
   patches

4. Document how to do this right. I keep getting asked about this,
   and it helps to point to some documentation file.

The series is avaialable for testing at [1].

       Arnd

[1] https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=compat-ioctl-endgame

Arnd Bergmann (24):
  compat: ARM64: always include asm-generic/compat.h
  compat: scsi: sg: fix v3 compat read/write interface
  compat_ioctl: block: handle BLKREPORTZONE/BLKRESETZONE
  compat_ioctl: block: handle BLKGETZONESZ/BLKGETNRZONES
  compat_ioctl: block: handle add zone open, close and finish ioctl
  compat_ioctl: block: handle Persistent Reservations
  compaT_ioctl: ubd, aoe: use blkdev_compat_ptr_ioctl
  compat_ioctl: move CDROM_SEND_PACKET handling into scsi
  compat_ioctl: move CDROMREADADIO to cdrom.c
  compat_ioctl: cdrom: handle CDROM_LAST_WRITTEN
  compat_ioctl: block: handle cdrom compat ioctl in non-cdrom drivers
  compat_ioctl: add scsi_compat_ioctl
  compat_ioctl: bsg: add handler
  compat_ioctl: ide: floppy: add handler
  compat_ioctl: scsi: move ioctl handling into drivers
  compat_ioctl: move sys_compat_ioctl() to ioctl.c
  compat_ioctl: simplify the implementation
  compat_ioctl: move cdrom commands into cdrom.c
  compat_ioctl: scsi: handle HDIO commands from drivers
  compat_ioctl: move HDIO ioctl handling into drivers/ide
  compat_ioctl: block: move blkdev_compat_ioctl() into ioctl.c
  compat_ioctl: block: simplify compat_blkpg_ioctl()
  compat_ioctl: simplify up block/ioctl.c
  Documentation: document ioctl interfaces better

 Documentation/core-api/index.rst       |   1 +
 Documentation/core-api/ioctl.rst       | 250 +++++++++++++++
 arch/arm64/include/asm/compat.h        |   5 +-
 arch/um/drivers/ubd_kern.c             |   1 +
 block/Makefile                         |   1 -
 block/bsg.c                            |   1 +
 block/compat_ioctl.c                   | 411 -------------------------
 block/ioctl.c                          | 319 +++++++++++++++----
 block/scsi_ioctl.c                     | 214 ++++++++-----
 drivers/ata/libata-scsi.c              |   9 +
 drivers/block/aoe/aoeblk.c             |   1 +
 drivers/block/floppy.c                 |   3 +
 drivers/block/paride/pcd.c             |   3 +
 drivers/block/paride/pd.c              |   1 +
 drivers/block/paride/pf.c              |   1 +
 drivers/block/pktcdvd.c                |  26 +-
 drivers/block/sunvdc.c                 |   1 +
 drivers/block/virtio_blk.c             |   3 +
 drivers/block/xen-blkfront.c           |   1 +
 drivers/cdrom/cdrom.c                  |  35 ++-
 drivers/cdrom/gdrom.c                  |   3 +
 drivers/ide/ide-cd.c                   |  40 +++
 drivers/ide/ide-disk.c                 |   3 +
 drivers/ide/ide-floppy.c               |   4 +
 drivers/ide/ide-floppy.h               |   2 +
 drivers/ide/ide-floppy_ioctl.c         |  35 +++
 drivers/ide/ide-gd.c                   |  14 +
 drivers/ide/ide-ioctls.c               |  47 ++-
 drivers/ide/ide-tape.c                 |  14 +
 drivers/scsi/aic94xx/aic94xx_init.c    |   3 +
 drivers/scsi/ch.c                      |   9 +-
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c |   3 +
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c |   3 +
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c |   3 +
 drivers/scsi/ipr.c                     |   3 +
 drivers/scsi/isci/init.c               |   3 +
 drivers/scsi/mvsas/mv_init.c           |   3 +
 drivers/scsi/pm8001/pm8001_init.c      |   3 +
 drivers/scsi/scsi_ioctl.c              |  54 +++-
 drivers/scsi/sd.c                      |  50 ++-
 drivers/scsi/sg.c                      | 169 +++++-----
 drivers/scsi/sr.c                      |  53 +++-
 drivers/scsi/st.c                      |  51 +--
 fs/Makefile                            |   2 +-
 fs/compat_ioctl.c                      | 261 ----------------
 fs/internal.h                          |   6 -
 fs/ioctl.c                             | 131 +++++---
 include/linux/blkdev.h                 |   7 +
 include/linux/falloc.h                 |   2 -
 include/linux/fs.h                     |   4 -
 include/linux/ide.h                    |   2 +
 include/linux/libata.h                 |   6 +
 include/scsi/scsi_ioctl.h              |   1 +
 include/scsi/sg.h                      |  30 ++
 54 files changed, 1249 insertions(+), 1062 deletions(-)
 create mode 100644 Documentation/core-api/ioctl.rst
 delete mode 100644 block/compat_ioctl.c
 delete mode 100644 fs/compat_ioctl.c

-- 
2.20.0

Cc: corbet@lwn.net
Cc: catalin.marinas@arm.com
Cc: will@kernel.org
Cc: jdike@addtoit.com
Cc: richard@nod.at
Cc: anton.ivanov@cambridgegreys.com
Cc: fujita.tomonori@lab.ntt.co.jp
Cc: justin@coraid.com
Cc: efremov@linux.com
Cc: tim@cyberelk.net
Cc: mst@redhat.com
Cc: jasowang@redhat.com
Cc: pbonzini@redhat.com
Cc: stefanha@redhat.com
Cc: boris.ostrovsky@oracle.com
Cc: jgross@suse.com
Cc: sstabellini@kernel.org
Cc: konrad.wilk@oracle.com
Cc: roger.pau@citrix.com
Cc: bp@alien8.de
Cc: davem@davemloft.net
Cc: john.garry@huawei.com
Cc: brking@us.ibm.com
Cc: intel-linux-scu@intel.com
Cc: artur.paszkiewicz@intel.com
Cc: jinpu.wang@cloud.ionos.com
Cc: dgilbert@interlog.com
Cc: Kai.Makisara@kolumbus.fi
Cc: arnd@arndb.de
Cc: damien.lemoal@hgst.com
Cc: hare@suse.com
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-block@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-um@lists.infradead.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-ide@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Cc: xen-devel@lists.xenproject.org
Cc: linux-fsdevel@vger.kernel.org

[PATCH 02/24] compat: scsi: sg: fix v3 compat read/write interface

[Arnd Bergmann]

In the v5.4 merge window, a cleanup patch from Al Viro conflicted
with my rework of the compat handling for sg.c read(). Linus Torvalds
did a correct merge but pointed out that the resulting code is still
unsatisfactory.

I later noticed that the sg_new_read() function still gets the compat
mode wrong, when the 'count' argument is large enough to pass a
compat_sg_io_hdr object, but not a nativ sg_io_hdr.

To address both of these, move the definition of compat_sg_io_hdr
into a scsi/sg.h to make it visible to sg.c and rewrite the logic
for reading req_pack_id as well as the size check to a simpler
version that gets the expected results.

Fixes: c35a5cfb4150 ("scsi: sg: sg_read(): simplify reading ->pack_id of userland sg_io_hdr_t")
Fixes: 98aaaec4a150 ("compat_ioctl: reimplement SG_IO handling")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 block/scsi_ioctl.c |  29 +----------
 drivers/scsi/sg.c  | 125 +++++++++++++++++++++------------------------
 include/scsi/sg.h  |  30 +++++++++++
 3 files changed, 89 insertions(+), 95 deletions(-)

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 650bade5ea5a..b61dbf4d8443 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -20,6 +20,7 @@
 #include <scsi/scsi.h>
 #include <scsi/scsi_ioctl.h>
 #include <scsi/scsi_cmnd.h>
+#include <scsi/sg.h>
 
 struct blk_cmd_filter {
 	unsigned long read_ok[BLK_SCSI_CMD_PER_LONG];
@@ -550,34 +551,6 @@ static inline int blk_send_start_stop(struct request_queue *q,
 	return __blk_send_generic(q, bd_disk, GPCMD_START_STOP_UNIT, data);
 }
 
-#ifdef CONFIG_COMPAT
-struct compat_sg_io_hdr {
-	compat_int_t interface_id;	/* [i] 'S' for SCSI generic (required) */
-	compat_int_t dxfer_direction;	/* [i] data transfer direction  */
-	unsigned char cmd_len;		/* [i] SCSI command length ( <= 16 bytes) */
-	unsigned char mx_sb_len;	/* [i] max length to write to sbp */
-	unsigned short iovec_count;	/* [i] 0 implies no scatter gather */
-	compat_uint_t dxfer_len;	/* [i] byte count of data transfer */
-	compat_uint_t dxferp;		/* [i], [*io] points to data transfer memory
-						or scatter gather list */
-	compat_uptr_t cmdp;		/* [i], [*i] points to command to perform */
-	compat_uptr_t sbp;		/* [i], [*o] points to sense_buffer memory */
-	compat_uint_t timeout;		/* [i] MAX_UINT->no timeout (unit: millisec) */
-	compat_uint_t flags;		/* [i] 0 -> default, see SG_FLAG... */
-	compat_int_t pack_id;		/* [i->o] unused internally (normally) */
-	compat_uptr_t usr_ptr;		/* [i->o] unused internally */
-	unsigned char status;		/* [o] scsi status */
-	unsigned char masked_status;	/* [o] shifted, masked scsi status */
-	unsigned char msg_status;	/* [o] messaging level data (optional) */
-	unsigned char sb_len_wr;	/* [o] byte count actually written to sbp */
-	unsigned short host_status;	/* [o] errors from host adapter */
-	unsigned short driver_status;	/* [o] errors from software driver */
-	compat_int_t resid;		/* [o] dxfer_len - actual_transferred */
-	compat_uint_t duration;		/* [o] time taken by cmd (unit: millisec) */
-	compat_uint_t info;		/* [o] auxiliary information */
-};
-#endif
-
 int put_sg_io_hdr(const struct sg_io_hdr *hdr, void __user *argp)
 {
 #ifdef CONFIG_COMPAT
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 160748ad9c0f..985546aac236 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -198,6 +198,7 @@ static void sg_device_destroy(struct kref *kref);
 
 #define SZ_SG_HEADER sizeof(struct sg_header)
 #define SZ_SG_IO_HDR sizeof(sg_io_hdr_t)
+#define SZ_COMPAT_SG_IO_HDR sizeof(struct compat_sg_io_hdr)
 #define SZ_SG_IOVEC sizeof(sg_iovec_t)
 #define SZ_SG_REQ_INFO sizeof(sg_req_info_t)
 
@@ -405,6 +406,36 @@ sg_release(struct inode *inode, struct file *filp)
 	return 0;
 }
 
+static int get_sg_io_pack_id(int *pack_id, void __user *buf, size_t count)
+{
+	struct sg_header __user *old_hdr = buf;
+	int reply_len;
+
+	if (count < SZ_SG_HEADER)
+		goto unknown_id;
+
+	/* negative reply_len means v3 format, otherwise v1/v2 */
+	if (get_user(reply_len, &old_hdr->reply_len))
+		return -EFAULT;
+	if (reply_len >= 0)
+		return get_user(*pack_id, &old_hdr->pack_id);
+
+	if (in_compat_syscall() && count >= SZ_COMPAT_SG_IO_HDR) {
+		struct compat_sg_io_hdr __user *hp = buf;
+		return get_user(*pack_id, &hp->pack_id);
+	}
+
+	if (count >= SZ_SG_IO_HDR) {
+		struct sg_io_hdr __user *hp = buf;
+		return get_user(*pack_id, &hp->pack_id);
+	}
+
+unknown_id:
+	/* no valid header was passed, so ignore the pack_id */
+	*pack_id = -1;
+	return 0;
+}
+
 static ssize_t
 sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
 {
@@ -413,8 +444,8 @@ sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
 	Sg_request *srp;
 	int req_pack_id = -1;
 	sg_io_hdr_t *hp;
-	struct sg_header *old_hdr = NULL;
-	int retval = 0;
+	struct sg_header *old_hdr;
+	int retval;
 
 	/*
 	 * This could cause a response to be stranded. Close the associated
@@ -429,79 +460,34 @@ sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
 	SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp,
 				      "sg_read: count=%d\n", (int) count));
 
-	if (sfp->force_packid && (count >= SZ_SG_HEADER)) {
-		old_hdr = memdup_user(buf, SZ_SG_HEADER);
-		if (IS_ERR(old_hdr))
-			return PTR_ERR(old_hdr);
-		if (old_hdr->reply_len < 0) {
-			if (count >= SZ_SG_IO_HDR) {
-				/*
-				 * This is stupid.
-				 *
-				 * We're copying the whole sg_io_hdr_t from user
-				 * space just to get the 'pack_id' field. But the
-				 * field is at different offsets for the compat
-				 * case, so we'll use "get_sg_io_hdr()" to copy
-				 * the whole thing and convert it.
-				 *
-				 * We could do something like just calculating the
-				 * offset based of 'in_compat_syscall()', but the
-				 * 'compat_sg_io_hdr' definition is in the wrong
-				 * place for that.
-				 */
-				sg_io_hdr_t *new_hdr;
-				new_hdr = kmalloc(SZ_SG_IO_HDR, GFP_KERNEL);
-				if (!new_hdr) {
-					retval = -ENOMEM;
-					goto free_old_hdr;
-				}
-				retval = get_sg_io_hdr(new_hdr, buf);
-				req_pack_id = new_hdr->pack_id;
-				kfree(new_hdr);
-				if (retval) {
-					retval = -EFAULT;
-					goto free_old_hdr;
-				}
-			}
-		} else
-			req_pack_id = old_hdr->pack_id;
-	}
+	if (sfp->force_packid)
+		retval = get_sg_io_pack_id(&req_pack_id, buf, count);
+	if (retval)
+		return retval;
+
 	srp = sg_get_rq_mark(sfp, req_pack_id);
 	if (!srp) {		/* now wait on packet to arrive */
-		if (atomic_read(&sdp->detaching)) {
-			retval = -ENODEV;
-			goto free_old_hdr;
-		}
-		if (filp->f_flags & O_NONBLOCK) {
-			retval = -EAGAIN;
-			goto free_old_hdr;
-		}
+		if (atomic_read(&sdp->detaching))
+			return -ENODEV;
+		if (filp->f_flags & O_NONBLOCK)
+			return -EAGAIN;
 		retval = wait_event_interruptible(sfp->read_wait,
 			(atomic_read(&sdp->detaching) ||
 			(srp = sg_get_rq_mark(sfp, req_pack_id))));
-		if (atomic_read(&sdp->detaching)) {
-			retval = -ENODEV;
-			goto free_old_hdr;
-		}
-		if (retval) {
+		if (atomic_read(&sdp->detaching))
+			return -ENODEV;
+		if (retval)
 			/* -ERESTARTSYS as signal hit process */
-			goto free_old_hdr;
-		}
-	}
-	if (srp->header.interface_id != '\0') {
-		retval = sg_new_read(sfp, buf, count, srp);
-		goto free_old_hdr;
+			return retval;
 	}
+	if (srp->header.interface_id != '\0')
+		return sg_new_read(sfp, buf, count, srp);
 
 	hp = &srp->header;
-	if (old_hdr == NULL) {
-		old_hdr = kmalloc(SZ_SG_HEADER, GFP_KERNEL);
-		if (! old_hdr) {
-			retval = -ENOMEM;
-			goto free_old_hdr;
-		}
-	}
-	memset(old_hdr, 0, SZ_SG_HEADER);
+	old_hdr = kzalloc(SZ_SG_HEADER, GFP_KERNEL);
+	if (!old_hdr)
+		return -ENOMEM;
+
 	old_hdr->reply_len = (int) hp->timeout;
 	old_hdr->pack_len = old_hdr->reply_len; /* old, strange behaviour */
 	old_hdr->pack_id = hp->pack_id;
@@ -575,7 +561,12 @@ sg_new_read(Sg_fd * sfp, char __user *buf, size_t count, Sg_request * srp)
 	int err = 0, err2;
 	int len;
 
-	if (count < SZ_SG_IO_HDR) {
+	if (in_compat_syscall()) {
+		if (count < SZ_COMPAT_SG_IO_HDR) {
+			err = -EINVAL;
+			goto err_out;
+		}
+	} else if (count < SZ_SG_IO_HDR) {
 		err = -EINVAL;
 		goto err_out;
 	}
diff --git a/include/scsi/sg.h b/include/scsi/sg.h
index f91bcca604e4..29c7ad04d2e2 100644
--- a/include/scsi/sg.h
+++ b/include/scsi/sg.h
@@ -68,6 +68,36 @@ typedef struct sg_io_hdr
     unsigned int info;          /* [o] auxiliary information */
 } sg_io_hdr_t;  /* 64 bytes long (on i386) */
 
+#if defined(__KERNEL__)
+#include <linux/compat.h>
+
+struct compat_sg_io_hdr {
+	compat_int_t interface_id;	/* [i] 'S' for SCSI generic (required) */
+	compat_int_t dxfer_direction;	/* [i] data transfer direction  */
+	unsigned char cmd_len;		/* [i] SCSI command length ( <= 16 bytes) */
+	unsigned char mx_sb_len;	/* [i] max length to write to sbp */
+	unsigned short iovec_count;	/* [i] 0 implies no scatter gather */
+	compat_uint_t dxfer_len;	/* [i] byte count of data transfer */
+	compat_uint_t dxferp;		/* [i], [*io] points to data transfer memory
+						or scatter gather list */
+	compat_uptr_t cmdp;		/* [i], [*i] points to command to perform */
+	compat_uptr_t sbp;		/* [i], [*o] points to sense_buffer memory */
+	compat_uint_t timeout;		/* [i] MAX_UINT->no timeout (unit: millisec) */
+	compat_uint_t flags;		/* [i] 0 -> default, see SG_FLAG... */
+	compat_int_t pack_id;		/* [i->o] unused internally (normally) */
+	compat_uptr_t usr_ptr;		/* [i->o] unused internally */
+	unsigned char status;		/* [o] scsi status */
+	unsigned char masked_status;	/* [o] shifted, masked scsi status */
+	unsigned char msg_status;	/* [o] messaging level data (optional) */
+	unsigned char sb_len_wr;	/* [o] byte count actually written to sbp */
+	unsigned short host_status;	/* [o] errors from host adapter */
+	unsigned short driver_status;	/* [o] errors from software driver */
+	compat_int_t resid;		/* [o] dxfer_len - actual_transferred */
+	compat_uint_t duration;		/* [o] time taken by cmd (unit: millisec) */
+	compat_uint_t info;		/* [o] auxiliary information */
+};
+#endif
+
 #define SG_INTERFACE_ID_ORIG 'S'
 
 /* Use negative values to flag difference from original sg_header structure */
-- 
2.20.0

[PATCH 12/24] compat_ioctl: add scsi_compat_ioctl

[Arnd Bergmann]

In order to move the compat handling for SCSI ioctl commands out of
fs/compat_ioctl.c into the individual drivers, we need a helper function
first to match the native ioctl handler called by sd, sr, st, etc.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/scsi_ioctl.c | 54 +++++++++++++++++++++++++++++----------
 include/scsi/scsi_ioctl.h |  1 +
 2 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index 57bcd05605bf..8f3af87b6bb0 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -189,17 +189,7 @@ static int scsi_ioctl_get_pci(struct scsi_device *sdev, void __user *arg)
 }
 
 
-/**
- * scsi_ioctl - Dispatch ioctl to scsi device
- * @sdev: scsi device receiving ioctl
- * @cmd: which ioctl is it
- * @arg: data associated with ioctl
- *
- * Description: The scsi_ioctl() function differs from most ioctls in that it
- * does not take a major/minor number as the dev field.  Rather, it takes
- * a pointer to a &struct scsi_device.
- */
-int scsi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int scsi_ioctl_common(struct scsi_device *sdev, int cmd, void __user *arg)
 {
 	char scsi_cmd[MAX_COMMAND_SIZE];
 	struct scsi_sense_hdr sense_hdr;
@@ -266,14 +256,50 @@ int scsi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
                 return scsi_ioctl_get_pci(sdev, arg);
 	case SG_SCSI_RESET:
 		return scsi_ioctl_reset(sdev, arg);
-	default:
-		if (sdev->host->hostt->ioctl)
-			return sdev->host->hostt->ioctl(sdev, cmd, arg);
 	}
+	return -ENOIOCTLCMD;
+}
+
+/**
+ * scsi_ioctl - Dispatch ioctl to scsi device
+ * @sdev: scsi device receiving ioctl
+ * @cmd: which ioctl is it
+ * @arg: data associated with ioctl
+ *
+ * Description: The scsi_ioctl() function differs from most ioctls in that it
+ * does not take a major/minor number as the dev field.  Rather, it takes
+ * a pointer to a &struct scsi_device.
+ */
+int scsi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+{
+	int ret = scsi_ioctl_common(sdev, cmd, arg);
+
+	if (ret != -ENOIOCTLCMD)
+		return ret;
+
+	if (sdev->host->hostt->ioctl)
+		return sdev->host->hostt->ioctl(sdev, cmd, arg);
+
 	return -EINVAL;
 }
 EXPORT_SYMBOL(scsi_ioctl);
 
+#ifdef CONFIG_COMPAT
+int scsi_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+{
+	int ret = scsi_ioctl_common(sdev, cmd, arg);
+
+	if (ret != -ENOIOCTLCMD)
+		return ret;
+
+	if (sdev->host->hostt->compat_ioctl)
+		return sdev->host->hostt->compat_ioctl(sdev, cmd, arg);
+
+	return ret;
+}
+EXPORT_SYMBOL(scsi_compat_ioctl);
+#endif
+
 /*
  * We can process a reset even when a device isn't fully operable.
  */
diff --git a/include/scsi/scsi_ioctl.h b/include/scsi/scsi_ioctl.h
index 5101e987c0ef..4fe69d863b5d 100644
--- a/include/scsi/scsi_ioctl.h
+++ b/include/scsi/scsi_ioctl.h
@@ -44,6 +44,7 @@ typedef struct scsi_fctargaddress {
 int scsi_ioctl_block_when_processing_errors(struct scsi_device *sdev,
 		int cmd, bool ndelay);
 extern int scsi_ioctl(struct scsi_device *, int, void __user *);
+extern int scsi_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
 
 #endif /* __KERNEL__ */
 #endif /* _SCSI_IOCTL_H */
-- 
2.20.0

[PATCH 13/24] compat_ioctl: bsg: add handler

[Arnd Bergmann]

bsg_ioctl() calls into scsi_cmd_ioctl() for a couple of generic commands
and relies on fs/compat_ioctl.c to handle it correctly in compat mode.

Adding a private compat_ioctl() handler avoids that round-trip and lets
us get rid of the generic emulation once this is done.

Note that bsg implements an SG_IO command that is different from the
other drivers and does not need emulation.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 block/bsg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/bsg.c b/block/bsg.c
index 833c44b3d458..d7bae94b64d9 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -382,6 +382,7 @@ static const struct file_operations bsg_fops = {
 	.open		=	bsg_open,
 	.release	=	bsg_release,
 	.unlocked_ioctl	=	bsg_ioctl,
+	.compat_ioctl	=	compat_ptr_ioctl,
 	.owner		=	THIS_MODULE,
 	.llseek		=	default_llseek,
 };
-- 
2.20.0

[PATCH 15/24] compat_ioctl: scsi: move ioctl handling into drivers

[Arnd Bergmann]

Each driver calling scsi_ioctl() gets an equivalent compat_ioctl()
handler that implements the same commands by calling scsi_compat_ioctl().

The scsi_cmd_ioctl() and scsi_cmd_blk_ioctl() functions are compatible
at this point, so any driver that calls those can do so for both native
and compat mode, with the argument passed through compat_ptr().

With this, we can remove the entries from fs/compat_ioctl.c.  The new
code is larger, but should be easier to maintain and keep updated with
newly added commands.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/block/virtio_blk.c |   3 +
 drivers/scsi/ch.c          |   9 ++-
 drivers/scsi/sd.c          |  50 ++++++--------
 drivers/scsi/sg.c          |  44 ++++++++-----
 drivers/scsi/sr.c          |  57 ++++++++++++++--
 drivers/scsi/st.c          |  51 ++++++++------
 fs/compat_ioctl.c          | 132 +------------------------------------
 7 files changed, 142 insertions(+), 204 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 7ffd719d89de..fbbf18ac1d5d 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -405,6 +405,9 @@ static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
 
 static const struct block_device_operations virtblk_fops = {
 	.ioctl  = virtblk_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = blkdev_compat_ptr_ioctl,
+#endif
 	.owner  = THIS_MODULE,
 	.getgeo = virtblk_getgeo,
 };
diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index 76751d6c7f0d..ed5f4a6ae270 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -872,6 +872,10 @@ static long ch_ioctl_compat(struct file * file,
 			    unsigned int cmd, unsigned long arg)
 {
 	scsi_changer *ch = file->private_data;
+	int retval = scsi_ioctl_block_when_processing_errors(ch->device, cmd,
+							file->f_flags & O_NDELAY);
+	if (retval)
+		return retval;
 
 	switch (cmd) {
 	case CHIOGPARAMS:
@@ -883,7 +887,7 @@ static long ch_ioctl_compat(struct file * file,
 	case CHIOINITELEM:
 	case CHIOSVOLTAG:
 		/* compatible */
-		return ch_ioctl(file, cmd, arg);
+		return ch_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
 	case CHIOGSTATUS32:
 	{
 		struct changer_element_status32 ces32;
@@ -898,8 +902,7 @@ static long ch_ioctl_compat(struct file * file,
 		return ch_gstatus(ch, ces32.ces_type, data);
 	}
 	default:
-		// return scsi_ioctl_compat(ch->device, cmd, (void*)arg);
-		return -ENOIOCTLCMD;
+		return scsi_compat_ioctl(ch->device, cmd, compat_ptr(arg));
 
 	}
 }
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index cea625906440..5afb0046b12a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1465,13 +1465,12 @@ static int sd_getgeo(struct block_device *bdev, struct hd_geometry *geo)
  *	Note: most ioctls are forward onto the block subsystem or further
  *	down in the scsi subsystem.
  **/
-static int sd_ioctl(struct block_device *bdev, fmode_t mode,
-		    unsigned int cmd, unsigned long arg)
+static int sd_ioctl_common(struct block_device *bdev, fmode_t mode,
+			   unsigned int cmd, void __user *p)
 {
 	struct gendisk *disk = bdev->bd_disk;
 	struct scsi_disk *sdkp = scsi_disk(disk);
 	struct scsi_device *sdp = sdkp->device;
-	void __user *p = (void __user *)arg;
 	int error;
     
 	SCSI_LOG_IOCTL(1, sd_printk(KERN_INFO, sdkp, "sd_ioctl: disk=%s, "
@@ -1507,9 +1506,6 @@ static int sd_ioctl(struct block_device *bdev, fmode_t mode,
 			break;
 		default:
 			error = scsi_cmd_blk_ioctl(bdev, mode, cmd, p);
-			if (error != -ENOTTY)
-				break;
-			error = scsi_ioctl(sdp, cmd, p);
 			break;
 	}
 out:
@@ -1691,39 +1687,31 @@ static void sd_rescan(struct device *dev)
 	revalidate_disk(sdkp->disk);
 }
 
+static int sd_ioctl(struct block_device *bdev, fmode_t mode,
+		    unsigned int cmd, unsigned long arg)
+{
+	void __user *p = (void __user *)arg;
+	int ret;
+
+	ret = sd_ioctl_common(bdev, mode, cmd, p);
+	if (ret != -ENOTTY)
+		return ret;
+
+	return scsi_ioctl(scsi_disk(bdev->bd_disk)->device, cmd, p);
+}
 
 #ifdef CONFIG_COMPAT
-/* 
- * This gets directly called from VFS. When the ioctl 
- * is not recognized we go back to the other translation paths. 
- */
 static int sd_compat_ioctl(struct block_device *bdev, fmode_t mode,
 			   unsigned int cmd, unsigned long arg)
 {
-	struct gendisk *disk = bdev->bd_disk;
-	struct scsi_disk *sdkp = scsi_disk(disk);
-	struct scsi_device *sdev = sdkp->device;
 	void __user *p = compat_ptr(arg);
-	int error;
-
-	error = scsi_verify_blk_ioctl(bdev, cmd);
-	if (error < 0)
-		return error;
+	int ret;
 
-	error = scsi_ioctl_block_when_processing_errors(sdev, cmd,
-			(mode & FMODE_NDELAY) != 0);
-	if (error)
-		return error;
+	ret = sd_ioctl_common(bdev, mode, cmd, p);
+	if (ret != -ENOTTY)
+		return ret;
 
-	if (is_sed_ioctl(cmd))
-		return sed_ioctl(sdkp->opal_dev, cmd, p);
-	       
-	/* 
-	 * Let the static ioctl translation table take care of it.
-	 */
-	if (!sdev->host->hostt->compat_ioctl)
-		return -ENOIOCTLCMD; 
-	return sdev->host->hostt->compat_ioctl(sdev, cmd, p);
+	return scsi_compat_ioctl(scsi_disk(bdev->bd_disk)->device, cmd, p);
 }
 #endif
 
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 985546aac236..08efcee7a34d 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -910,19 +910,14 @@ static int put_compat_request_table(struct compat_sg_req_info __user *o,
 #endif
 
 static long
-sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
+sg_ioctl_common(struct file *filp, Sg_device *sdp, Sg_fd *sfp,
+		unsigned int cmd_in, void __user *p)
 {
-	void __user *p = (void __user *)arg;
 	int __user *ip = p;
 	int result, val, read_only;
-	Sg_device *sdp;
-	Sg_fd *sfp;
 	Sg_request *srp;
 	unsigned long iflags;
 
-	if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
-		return -ENXIO;
-
 	SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp,
 				   "sg_ioctl: cmd=0x%x\n", (int) cmd_in));
 	read_only = (O_RDWR != (filp->f_flags & O_ACCMODE));
@@ -1145,29 +1140,44 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
 			cmd_in, filp->f_flags & O_NDELAY);
 	if (result)
 		return result;
+
+	return -ENOIOCTLCMD;
+}
+
+static long
+sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
+{
+	void __user *p = (void __user *)arg;
+	Sg_device *sdp;
+	Sg_fd *sfp;
+	int ret;
+
+	if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
+		return -ENXIO;
+
+	ret = sg_ioctl_common(filp, sdp, sfp, cmd_in, p);
+	if (ret != -ENOIOCTLCMD)
+		return ret;
+
 	return scsi_ioctl(sdp->device, cmd_in, p);
 }
 
 #ifdef CONFIG_COMPAT
 static long sg_compat_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
 {
+	void __user *p = compat_ptr(arg);
 	Sg_device *sdp;
 	Sg_fd *sfp;
-	struct scsi_device *sdev;
+	int ret;
 
 	if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
 		return -ENXIO;
 
-	sdev = sdp->device;
-	if (sdev->host->hostt->compat_ioctl) { 
-		int ret;
-
-		ret = sdev->host->hostt->compat_ioctl(sdev, cmd_in, (void __user *)arg);
-
+	ret = sg_ioctl_common(filp, sdp, sfp, cmd_in, p);
+	if (ret != -ENOIOCTLCMD)
 		return ret;
-	}
-	
-	return -ENOIOCTLCMD;
+
+	return scsi_compat_ioctl(sdp->device, cmd_in, p);
 }
 #endif
 
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 4664fdf75c0f..6033a886c42c 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -38,6 +38,7 @@
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/bio.h>
+#include <linux/compat.h>
 #include <linux/string.h>
 #include <linux/errno.h>
 #include <linux/cdrom.h>
@@ -598,6 +599,55 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 	return ret;
 }
 
+#ifdef CONFIG_COMPAT
+static int sr_block_compat_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
+			  unsigned long arg)
+{
+	struct scsi_cd *cd = scsi_cd(bdev->bd_disk);
+	struct scsi_device *sdev = cd->device;
+	void __user *argp = compat_ptr(arg);
+	int ret;
+
+	mutex_lock(&sr_mutex);
+
+	ret = scsi_ioctl_block_when_processing_errors(sdev, cmd,
+			(mode & FMODE_NDELAY) != 0);
+	if (ret)
+		goto out;
+
+	scsi_autopm_get_device(sdev);
+
+	/*
+	 * Send SCSI addressing ioctls directly to mid level, send other
+	 * ioctls to cdrom/block level.
+	 */
+	switch (cmd) {
+	case SCSI_IOCTL_GET_IDLUN:
+	case SCSI_IOCTL_GET_BUS_NUMBER:
+		ret = scsi_compat_ioctl(sdev, cmd, argp);
+		goto put;
+	}
+
+	/*
+	 * CDROM ioctls are handled in the block layer, but
+	 * do the scsi blk ioctls here.
+	 */
+	ret = scsi_cmd_blk_ioctl(bdev, mode, cmd, argp);
+	if (ret != -ENOTTY)
+		return ret;
+
+	ret = scsi_compat_ioctl(sdev, cmd, argp);
+
+put:
+	scsi_autopm_put_device(sdev);
+
+out:
+	mutex_unlock(&sr_mutex);
+	return ret;
+
+}
+#endif
+
 static unsigned int sr_block_check_events(struct gendisk *disk,
 					  unsigned int clearing)
 {
@@ -641,12 +691,11 @@ static const struct block_device_operations sr_bdops =
 	.open		= sr_block_open,
 	.release	= sr_block_release,
 	.ioctl		= sr_block_ioctl,
+#ifdef CONFIG_COMPAT
+	.ioctl		= sr_block_compat_ioctl,
+#endif
 	.check_events	= sr_block_check_events,
 	.revalidate_disk = sr_block_revalidate_disk,
-	/* 
-	 * No compat_ioctl for now because sr_block_ioctl never
-	 * seems to pass arbitrary ioctls down to host drivers.
-	 */
 };
 
 static int sr_open(struct cdrom_device_info *cdi, int purpose)
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 9e3fff2de83e..393f3019ccac 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -3501,7 +3501,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)
+static long st_ioctl_common(struct file *file, unsigned int cmd_in, void __user *p)
 {
 	int i, cmd_nr, cmd_type, bt;
 	int retval = 0;
@@ -3509,7 +3509,6 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 	struct scsi_tape *STp = file->private_data;
 	struct st_modedef *STm;
 	struct st_partstat *STps;
-	void __user *p = (void __user *)arg;
 
 	if (mutex_lock_interruptible(&STp->lock))
 		return -ERESTARTSYS;
@@ -3824,9 +3823,19 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 	}
 	mutex_unlock(&STp->lock);
 	switch (cmd_in) {
+		case SCSI_IOCTL_STOP_UNIT:
+			/* unload */
+			retval = scsi_ioctl(STp->device, cmd_in, p);
+			if (!retval) {
+				STp->rew_at_close = 0;
+				STp->ready = ST_NO_TAPE;
+			}
+			return retval;
+
 		case SCSI_IOCTL_GET_IDLUN:
 		case SCSI_IOCTL_GET_BUS_NUMBER:
 			break;
+
 		default:
 			if ((cmd_in == SG_IO ||
 			     cmd_in == SCSI_IOCTL_SEND_COMMAND ||
@@ -3840,42 +3849,46 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 				return i;
 			break;
 	}
-	retval = scsi_ioctl(STp->device, cmd_in, p);
-	if (!retval && cmd_in == SCSI_IOCTL_STOP_UNIT) { /* unload */
-		STp->rew_at_close = 0;
-		STp->ready = ST_NO_TAPE;
-	}
-	return retval;
+	return -ENOTTY;
 
  out:
 	mutex_unlock(&STp->lock);
 	return retval;
 }
 
+static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
+{
+	void __user *p = (void __user *)arg;
+	struct scsi_tape *STp = file->private_data;
+	int ret;
+
+	ret = st_ioctl_common(file, cmd_in, p);
+	if (ret != -ENOTTY)
+		return ret;
+
+	return scsi_ioctl(STp->device, cmd_in, p);
+}
+
 #ifdef CONFIG_COMPAT
 static long st_compat_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 {
 	void __user *p = compat_ptr(arg);
 	struct scsi_tape *STp = file->private_data;
-	struct scsi_device *sdev = STp->device;
-	int ret = -ENOIOCTLCMD;
+	int ret;
 
 	/* argument conversion is handled using put_user_mtpos/put_user_mtget */
 	switch (cmd_in) {
-	case MTIOCTOP:
-		return st_ioctl(file, MTIOCTOP, (unsigned long)p);
 	case MTIOCPOS32:
-		return st_ioctl(file, MTIOCPOS, (unsigned long)p);
+		return st_ioctl_common(file, MTIOCPOS, p);
 	case MTIOCGET32:
-		return st_ioctl(file, MTIOCGET, (unsigned long)p);
+		return st_ioctl_common(file, MTIOCGET, p);
 	}
 
-	if (sdev->host->hostt->compat_ioctl) { 
+	ret = st_ioctl_common(file, cmd_in, p);
+	if (ret != -ENOTTY)
+		return ret;
 
-		ret = sdev->host->hostt->compat_ioctl(sdev, cmd_in, (void __user *)arg);
-
-	}
-	return ret;
+	return scsi_compat_ioctl(STp->device, cmd_in, p);
 }
 #endif
 
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 358ea2ecf36b..ab4471f469e6 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -36,109 +36,11 @@
 
 #include "internal.h"
 
-#ifdef CONFIG_BLOCK
-#include <linux/cdrom.h>
-#include <linux/fd.h>
-#include <scsi/scsi.h>
-#include <scsi/scsi_ioctl.h>
-#include <scsi/sg.h>
-#endif
-
 #include <linux/uaccess.h>
 #include <linux/watchdog.h>
 
 #include <linux/hiddev.h>
 
-
-#include <linux/sort.h>
-
-/*
- * simple reversible transform to make our table more evenly
- * distributed after sorting.
- */
-#define XFORM(i) (((i) ^ ((i) << 27) ^ ((i) << 17)) & 0xffffffff)
-
-#define COMPATIBLE_IOCTL(cmd) XFORM((u32)cmd),
-static unsigned int ioctl_pointer[] = {
-#ifdef CONFIG_BLOCK
-/* Big S */
-COMPATIBLE_IOCTL(SCSI_IOCTL_GET_IDLUN)
-COMPATIBLE_IOCTL(SCSI_IOCTL_DOORLOCK)
-COMPATIBLE_IOCTL(SCSI_IOCTL_DOORUNLOCK)
-COMPATIBLE_IOCTL(SCSI_IOCTL_TEST_UNIT_READY)
-COMPATIBLE_IOCTL(SCSI_IOCTL_GET_BUS_NUMBER)
-COMPATIBLE_IOCTL(SCSI_IOCTL_SEND_COMMAND)
-COMPATIBLE_IOCTL(SCSI_IOCTL_PROBE_HOST)
-COMPATIBLE_IOCTL(SCSI_IOCTL_GET_PCI)
-#endif
-#ifdef CONFIG_BLOCK
-/* SG stuff */
-COMPATIBLE_IOCTL(SG_IO)
-COMPATIBLE_IOCTL(SG_GET_REQUEST_TABLE)
-COMPATIBLE_IOCTL(SG_SET_TIMEOUT)
-COMPATIBLE_IOCTL(SG_GET_TIMEOUT)
-COMPATIBLE_IOCTL(SG_EMULATED_HOST)
-COMPATIBLE_IOCTL(SG_GET_TRANSFORM)
-COMPATIBLE_IOCTL(SG_SET_RESERVED_SIZE)
-COMPATIBLE_IOCTL(SG_GET_RESERVED_SIZE)
-COMPATIBLE_IOCTL(SG_GET_SCSI_ID)
-COMPATIBLE_IOCTL(SG_SET_FORCE_LOW_DMA)
-COMPATIBLE_IOCTL(SG_GET_LOW_DMA)
-COMPATIBLE_IOCTL(SG_SET_FORCE_PACK_ID)
-COMPATIBLE_IOCTL(SG_GET_PACK_ID)
-COMPATIBLE_IOCTL(SG_GET_NUM_WAITING)
-COMPATIBLE_IOCTL(SG_SET_DEBUG)
-COMPATIBLE_IOCTL(SG_GET_SG_TABLESIZE)
-COMPATIBLE_IOCTL(SG_GET_COMMAND_Q)
-COMPATIBLE_IOCTL(SG_SET_COMMAND_Q)
-COMPATIBLE_IOCTL(SG_GET_VERSION_NUM)
-COMPATIBLE_IOCTL(SG_NEXT_CMD_LEN)
-COMPATIBLE_IOCTL(SG_SCSI_RESET)
-COMPATIBLE_IOCTL(SG_GET_REQUEST_TABLE)
-COMPATIBLE_IOCTL(SG_SET_KEEP_ORPHAN)
-COMPATIBLE_IOCTL(SG_GET_KEEP_ORPHAN)
-#endif
-};
-
-/*
- * Convert common ioctl arguments based on their command number
- *
- * Please do not add any code in here. Instead, implement
- * a compat_ioctl operation in the place that handleѕ the
- * ioctl for the native case.
- */
-static long do_ioctl_trans(unsigned int cmd,
-		 unsigned long arg, struct file *file)
-{
-	return -ENOIOCTLCMD;
-}
-
-static int compat_ioctl_check_table(unsigned int xcmd)
-{
-#ifdef CONFIG_BLOCK
-	int i;
-	const int max = ARRAY_SIZE(ioctl_pointer) - 1;
-
-	BUILD_BUG_ON(max >= (1 << 16));
-
-	/* guess initial offset into table, assuming a
-	   normalized distribution */
-	i = ((xcmd >> 16) * max) >> 16;
-
-	/* do linear search up first, until greater or equal */
-	while (ioctl_pointer[i] < xcmd && i < max)
-		i++;
-
-	/* then do linear search down */
-	while (ioctl_pointer[i] > xcmd && i > 0)
-		i--;
-
-	return ioctl_pointer[i] == xcmd;
-#else
-	return 0;
-#endif
-}
-
 COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
 		       compat_ulong_t, arg32)
 {
@@ -216,19 +118,9 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
 				goto out_fput;
 		}
 
-		if (!f.file->f_op->unlocked_ioctl)
-			goto do_ioctl;
-		break;
-	}
-
-	if (compat_ioctl_check_table(XFORM(cmd)))
-		goto found_handler;
-
-	error = do_ioctl_trans(cmd, arg, f.file);
-	if (error == -ENOIOCTLCMD)
 		error = -ENOTTY;
-
-	goto out_fput;
+		goto out_fput;
+	}
 
  found_handler:
 	arg = (unsigned long)compat_ptr(arg);
@@ -239,23 +131,3 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
  out:
 	return error;
 }
-
-static int __init init_sys32_ioctl_cmp(const void *p, const void *q)
-{
-	unsigned int a, b;
-	a = *(unsigned int *)p;
-	b = *(unsigned int *)q;
-	if (a > b)
-		return 1;
-	if (a < b)
-		return -1;
-	return 0;
-}
-
-static int __init init_sys32_ioctl(void)
-{
-	sort(ioctl_pointer, ARRAY_SIZE(ioctl_pointer), sizeof(*ioctl_pointer),
-		init_sys32_ioctl_cmp, NULL);
-	return 0;
-}
-__initcall(init_sys32_ioctl);
-- 
2.20.0

[PATCH 18/24] compat_ioctl: move cdrom commands into cdrom.c

[Arnd Bergmann]

There is no need for the special cases for the cdrom ioctls any more now,
so make sure that each cdrom driver has a .compat_ioctl() callback and
calls cdrom_compat_ioctl() directly there.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 block/compat_ioctl.c       | 45 --------------------------------------
 drivers/block/paride/pcd.c |  3 +++
 drivers/cdrom/gdrom.c      |  3 +++
 drivers/ide/ide-cd.c       | 36 ++++++++++++++++++++++++++++++
 drivers/scsi/sr.c          | 10 +++------
 5 files changed, 45 insertions(+), 52 deletions(-)

diff --git a/block/compat_ioctl.c b/block/compat_ioctl.c
index cf136bc2c9fc..7cb534d6e767 100644
--- a/block/compat_ioctl.c
+++ b/block/compat_ioctl.c
@@ -159,42 +159,6 @@ static int compat_blkdev_driver_ioctl(struct block_device *bdev, fmode_t mode,
 	case HDIO_DRIVE_CMD:
 	/* 0x330 is reserved -- it used to be HDIO_GETGEO_BIG */
 	case 0x330:
-	/* CDROM stuff */
-	case CDROMPAUSE:
-	case CDROMRESUME:
-	case CDROMPLAYMSF:
-	case CDROMPLAYTRKIND:
-	case CDROMREADTOCHDR:
-	case CDROMREADTOCENTRY:
-	case CDROMSTOP:
-	case CDROMSTART:
-	case CDROMEJECT:
-	case CDROMVOLCTRL:
-	case CDROMSUBCHNL:
-	case CDROMMULTISESSION:
-	case CDROM_GET_MCN:
-	case CDROMRESET:
-	case CDROMVOLREAD:
-	case CDROMSEEK:
-	case CDROMPLAYBLK:
-	case CDROMCLOSETRAY:
-	case CDROM_DISC_STATUS:
-	case CDROM_CHANGER_NSLOTS:
-	case CDROM_GET_CAPABILITY:
-	case CDROM_SEND_PACKET:
-	/* Ignore cdrom.h about these next 5 ioctls, they absolutely do
-	 * not take a struct cdrom_read, instead they take a struct cdrom_msf
-	 * which is compatible.
-	 */
-	case CDROMREADMODE2:
-	case CDROMREADMODE1:
-	case CDROMREADRAW:
-	case CDROMREADCOOKED:
-	case CDROMREADALL:
-	/* DVD ioctls */
-	case DVD_READ_STRUCT:
-	case DVD_WRITE_STRUCT:
-	case DVD_AUTH:
 		arg = (unsigned long)compat_ptr(arg);
 	/* These intepret arg as an unsigned long, not as a pointer,
 	 * so we must not do compat_ptr() conversion. */
@@ -210,15 +174,6 @@ static int compat_blkdev_driver_ioctl(struct block_device *bdev, fmode_t mode,
 	case HDIO_SET_ACOUSTIC:
 	case HDIO_SET_BUSSTATE:
 	case HDIO_SET_ADDRESS:
-	case CDROMEJECT_SW:
-	case CDROM_SET_OPTIONS:
-	case CDROM_CLEAR_OPTIONS:
-	case CDROM_SELECT_SPEED:
-	case CDROM_SELECT_DISC:
-	case CDROM_MEDIA_CHANGED:
-	case CDROM_DRIVE_STATUS:
-	case CDROM_LOCKDOOR:
-	case CDROM_DEBUG:
 		break;
 	default:
 		/* unknown ioctl number */
diff --git a/drivers/block/paride/pcd.c b/drivers/block/paride/pcd.c
index 636bfea2de6f..117cfc8cd05a 100644
--- a/drivers/block/paride/pcd.c
+++ b/drivers/block/paride/pcd.c
@@ -275,6 +275,9 @@ static const struct block_device_operations pcd_bdops = {
 	.open		= pcd_block_open,
 	.release	= pcd_block_release,
 	.ioctl		= pcd_block_ioctl,
+#ifdef CONFIG_COMPAT
+	.ioctl		= blkdev_compat_ptr_ioctl,
+#endif
 	.check_events	= pcd_block_check_events,
 };
 
diff --git a/drivers/cdrom/gdrom.c b/drivers/cdrom/gdrom.c
index 5b21dc421c94..886b2638c730 100644
--- a/drivers/cdrom/gdrom.c
+++ b/drivers/cdrom/gdrom.c
@@ -518,6 +518,9 @@ static const struct block_device_operations gdrom_bdops = {
 	.release		= gdrom_bdops_release,
 	.check_events		= gdrom_bdops_check_events,
 	.ioctl			= gdrom_bdops_ioctl,
+#ifdef CONFIG_COMPAT
+	.ioctl			= blkdev_compat_ptr_ioctl,
+#endif
 };
 
 static irqreturn_t gdrom_command_interrupt(int irq, void *dev_id)
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 9d117936bee1..2de6e8ace957 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -25,6 +25,7 @@
 
 #define IDECD_VERSION "5.00"
 
+#include <linux/compat.h>
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
@@ -1710,6 +1711,38 @@ static int idecd_ioctl(struct block_device *bdev, fmode_t mode,
 	return ret;
 }
 
+#ifdef CONFIG_COMPAT
+static int idecd_locked_compat_ioctl(struct block_device *bdev, fmode_t mode,
+			unsigned int cmd, unsigned long arg)
+{
+	struct cdrom_info *info = ide_drv_g(bdev->bd_disk, cdrom_info);
+	int err;
+
+	switch (cmd) {
+	case CDROMSETSPINDOWN:
+		return idecd_set_spindown(&info->devinfo, arg);
+	case CDROMGETSPINDOWN:
+		return idecd_get_spindown(&info->devinfo, arg);
+	default:
+		break;
+	}
+
+	return cdrom_ioctl(&info->devinfo, bdev, mode, cmd,
+			   (unsigned long)compat_ptr(arg));
+}
+
+static int idecd_compat_ioctl(struct block_device *bdev, fmode_t mode,
+			     unsigned int cmd, unsigned long arg)
+{
+	int ret;
+
+	mutex_lock(&ide_cd_mutex);
+	ret = idecd_locked_compat_ioctl(bdev, mode, cmd, arg);
+	mutex_unlock(&ide_cd_mutex);
+
+	return ret;
+}
+#endif
 
 static unsigned int idecd_check_events(struct gendisk *disk,
 				       unsigned int clearing)
@@ -1732,6 +1765,9 @@ static const struct block_device_operations idecd_ops = {
 	.open			= idecd_open,
 	.release		= idecd_release,
 	.ioctl			= idecd_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl		= idecd_compat_ioctl,
+#endif
 	.check_events		= idecd_check_events,
 	.revalidate_disk	= idecd_revalidate_disk
 };
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 6033a886c42c..0fbb8fe6e521 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -628,13 +628,9 @@ static int sr_block_compat_ioctl(struct block_device *bdev, fmode_t mode, unsign
 		goto put;
 	}
 
-	/*
-	 * CDROM ioctls are handled in the block layer, but
-	 * do the scsi blk ioctls here.
-	 */
-	ret = scsi_cmd_blk_ioctl(bdev, mode, cmd, argp);
-	if (ret != -ENOTTY)
-		return ret;
+	ret = cdrom_ioctl(&cd->cdi, bdev, mode, cmd, (unsigned long)argp);
+	if (ret != -ENOSYS)
+		goto put;
 
 	ret = scsi_compat_ioctl(sdev, cmd, argp);
 
-- 
2.20.0

[PATCH 19/24] compat_ioctl: scsi: handle HDIO commands from drivers

[Arnd Bergmann]

The ata_sas_scsi_ioctl() function implements a number of HDIO_* commands
for SCSI devices, it is used by all libata drivers as well as a few
drivers that support SAS attached SATA drives.

The only command that is not safe for compat ioctls here is
HDIO_GET_32BIT. Change the implementation to check for in_compat_syscall()
in order to do both cases correctly, and change all callers to use it
as both native and compat callback pointers, including the indirect
callers through sas_ioctl and ata_scsi_ioctl.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/ata/libata-scsi.c              | 9 +++++++++
 drivers/scsi/aic94xx/aic94xx_init.c    | 3 +++
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 3 +++
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 3 +++
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +++
 drivers/scsi/ipr.c                     | 3 +++
 drivers/scsi/isci/init.c               | 3 +++
 drivers/scsi/mvsas/mv_init.c           | 3 +++
 drivers/scsi/pm8001/pm8001_init.c      | 3 +++
 include/linux/libata.h                 | 6 ++++++
 10 files changed, 39 insertions(+)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 58e09ffe8b9c..eb2eb599e602 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -17,6 +17,7 @@
  *  - http://www.t13.org/
  */
 
+#include <linux/compat.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
 #include <linux/blkdev.h>
@@ -761,6 +762,10 @@ static int ata_ioc32(struct ata_port *ap)
 	return 0;
 }
 
+/*
+ * This handles both native and compat commands, so anything added
+ * here must have a compatible argument, or check in_compat_syscall()
+ */
 int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
 		     unsigned int cmd, void __user *arg)
 {
@@ -773,6 +778,10 @@ int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
 		spin_lock_irqsave(ap->lock, flags);
 		val = ata_ioc32(ap);
 		spin_unlock_irqrestore(ap->lock, flags);
+#ifdef CONFIG_COMPAT
+		if (in_compat_syscall())
+			return put_user(val, (compat_ulong_t __user *)arg);
+#endif
 		return put_user(val, (unsigned long __user *)arg);
 
 	case HDIO_SET_32BIT:
diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index f5781e31f57c..d022407e5645 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -54,6 +54,9 @@ static struct scsi_host_template aic94xx_sht = {
 	.eh_target_reset_handler	= sas_eh_target_reset_handler,
 	.target_destroy		= sas_target_destroy,
 	.ioctl			= sas_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl		= sas_ioctl,
+#endif
 	.track_queue_depth	= 1,
 };
 
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 3af53cc42bd6..fa25766502a2 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1772,6 +1772,9 @@ static struct scsi_host_template sht_v1_hw = {
 	.eh_target_reset_handler = sas_eh_target_reset_handler,
 	.target_destroy		= sas_target_destroy,
 	.ioctl			= sas_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl		= sas_ioctl,
+#endif
 	.shost_attrs		= host_attrs_v1_hw,
 	.host_reset             = hisi_sas_host_reset,
 };
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 61b1e2693b08..545eaff5f3ee 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3551,6 +3551,9 @@ static struct scsi_host_template sht_v2_hw = {
 	.eh_target_reset_handler = sas_eh_target_reset_handler,
 	.target_destroy		= sas_target_destroy,
 	.ioctl			= sas_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl		= sas_ioctl,
+#endif
 	.shost_attrs		= host_attrs_v2_hw,
 	.host_reset		= hisi_sas_host_reset,
 };
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index bf5d5f138437..fa05e612d85a 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -3075,6 +3075,9 @@ static struct scsi_host_template sht_v3_hw = {
 	.eh_target_reset_handler = sas_eh_target_reset_handler,
 	.target_destroy		= sas_target_destroy,
 	.ioctl			= sas_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl		= sas_ioctl,
+#endif
 	.shost_attrs		= host_attrs_v3_hw,
 	.tag_alloc_policy	= BLK_TAG_ALLOC_RR,
 	.host_reset             = hisi_sas_host_reset,
diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index 079c04bc448a..ae45cbe98ae2 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -6727,6 +6727,9 @@ static struct scsi_host_template driver_template = {
 	.name = "IPR",
 	.info = ipr_ioa_info,
 	.ioctl = ipr_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = ipr_ioctl,
+#endif
 	.queuecommand = ipr_queuecommand,
 	.eh_abort_handler = ipr_eh_abort,
 	.eh_device_reset_handler = ipr_eh_dev_reset,
diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index 1727d0c71b12..b48aac8dfcb8 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -168,6 +168,9 @@ static struct scsi_host_template isci_sht = {
 	.eh_target_reset_handler        = sas_eh_target_reset_handler,
 	.target_destroy			= sas_target_destroy,
 	.ioctl				= sas_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl			= sas_ioctl,
+#endif
 	.shost_attrs			= isci_host_attrs,
 	.track_queue_depth		= 1,
 };
diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index da719b0694dc..7af9173c4925 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -47,6 +47,9 @@ static struct scsi_host_template mvs_sht = {
 	.eh_target_reset_handler = sas_eh_target_reset_handler,
 	.target_destroy		= sas_target_destroy,
 	.ioctl			= sas_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl		= sas_ioctl,
+#endif
 	.shost_attrs		= mvst_host_attrs,
 	.track_queue_depth	= 1,
 };
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index ff618ad80ebd..3c6076e4c6d2 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -101,6 +101,9 @@ static struct scsi_host_template pm8001_sht = {
 	.eh_target_reset_handler = sas_eh_target_reset_handler,
 	.target_destroy		= sas_target_destroy,
 	.ioctl			= sas_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl		= sas_ioctl,
+#endif
 	.shost_attrs		= pm8001_host_attrs,
 	.track_queue_depth	= 1,
 };
diff --git a/include/linux/libata.h b/include/linux/libata.h
index d3bbfddf616a..e68d05febe5a 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1109,6 +1109,11 @@ extern void ata_host_init(struct ata_host *, struct device *, struct ata_port_op
 extern int ata_scsi_detect(struct scsi_host_template *sht);
 extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
 			  void __user *arg);
+#ifdef CONFIG_COMPAT
+#define ATA_SCSI_COMPAT_IOCTL .compat_ioctl = ata_scsi_ioctl,
+#else
+#define ATA_SCSI_COMPAT_IOCTL /* empty */
+#endif
 extern int ata_scsi_queuecmd(struct Scsi_Host *h, struct scsi_cmnd *cmd);
 extern int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *dev,
 			    unsigned int cmd, void __user *arg);
@@ -1340,6 +1345,7 @@ extern struct device_attribute *ata_common_sdev_attrs[];
 	.module			= THIS_MODULE,			\
 	.name			= drv_name,			\
 	.ioctl			= ata_scsi_ioctl,		\
+	ATA_SCSI_COMPAT_IOCTL					\
 	.queuecommand		= ata_scsi_queuecmd,		\
 	.can_queue		= ATA_DEF_QUEUE,		\
 	.tag_alloc_policy	= BLK_TAG_ALLOC_RR,		\
-- 
2.20.0

Re: [PATCH 15/24] compat_ioctl: scsi: move ioctl handling into drivers

[Michael S. Tsirkin]

On Wed, Dec 11, 2019 at 09:42:49PM +0100, Arnd Bergmann wrote:
> Each driver calling scsi_ioctl() gets an equivalent compat_ioctl()
> handler that implements the same commands by calling scsi_compat_ioctl().
> 
> The scsi_cmd_ioctl() and scsi_cmd_blk_ioctl() functions are compatible
> at this point, so any driver that calls those can do so for both native
> and compat mode, with the argument passed through compat_ptr().
> 
> With this, we can remove the entries from fs/compat_ioctl.c.  The new
> code is larger, but should be easier to maintain and keep updated with
> newly added commands.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/block/virtio_blk.c |   3 +
>  drivers/scsi/ch.c          |   9 ++-
>  drivers/scsi/sd.c          |  50 ++++++--------
>  drivers/scsi/sg.c          |  44 ++++++++-----
>  drivers/scsi/sr.c          |  57 ++++++++++++++--
>  drivers/scsi/st.c          |  51 ++++++++------
>  fs/compat_ioctl.c          | 132 +------------------------------------
>  7 files changed, 142 insertions(+), 204 deletions(-)
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 7ffd719d89de..fbbf18ac1d5d 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -405,6 +405,9 @@ static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
>  
>  static const struct block_device_operations virtblk_fops = {
>  	.ioctl  = virtblk_ioctl,
> +#ifdef CONFIG_COMPAT
> +	.compat_ioctl = blkdev_compat_ptr_ioctl,
> +#endif
>  	.owner  = THIS_MODULE,
>  	.getgeo = virtblk_getgeo,
>  };

Hmm - is virtio blk lumped in with scsi things intentionally?

-- 
MST

Re: [PATCH 15/24] compat_ioctl: scsi: move ioctl handling into drivers

[Paolo Bonzini]

On 12/12/19 00:05, Michael S. Tsirkin wrote:
>> @@ -405,6 +405,9 @@ static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
>>  
>>  static const struct block_device_operations virtblk_fops = {
>>  	.ioctl  = virtblk_ioctl,
>> +#ifdef CONFIG_COMPAT
>> +	.compat_ioctl = blkdev_compat_ptr_ioctl,
>> +#endif
>>  	.owner  = THIS_MODULE,
>>  	.getgeo = virtblk_getgeo,
>>  };
> Hmm - is virtio blk lumped in with scsi things intentionally?

I think it's because the only ioctl for virtio-blk is SG_IO.  It makes
sense to lump it in with scsi, but I wouldn't mind getting rid of
CONFIG_VIRTIO_BLK_SCSI altogether.

Paolo

Re: [PATCH 15/24] compat_ioctl: scsi: move ioctl handling into drivers

[Arnd Bergmann]

On Thu, Dec 12, 2019 at 1:28 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 12/12/19 00:05, Michael S. Tsirkin wrote:
> >> @@ -405,6 +405,9 @@ static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
> >>
> >>  static const struct block_device_operations virtblk_fops = {
> >>      .ioctl  = virtblk_ioctl,
> >> +#ifdef CONFIG_COMPAT
> >> +    .compat_ioctl = blkdev_compat_ptr_ioctl,
> >> +#endif
> >>      .owner  = THIS_MODULE,
> >>      .getgeo = virtblk_getgeo,
> >>  };
> > Hmm - is virtio blk lumped in with scsi things intentionally?
>
> I think it's because the only ioctl for virtio-blk is SG_IO.  It makes
> sense to lump it in with scsi, but I wouldn't mind getting rid of
> CONFIG_VIRTIO_BLK_SCSI altogether.

It currently calls scsi_cmd_blk_ioctl(), which implements a bunch of ioctl
commands, including some that are unrelated to SG_IO:

                case SG_GET_VERSION_NUM:
                case SCSI_IOCTL_GET_IDLUN:
                case SCSI_IOCTL_GET_BUS_NUMBER:
                case SG_SET_TIMEOUT:
                case SG_GET_TIMEOUT:
                case SG_GET_RESERVED_SIZE:
                case SG_SET_RESERVED_SIZE:
                case SG_EMULATED_HOST:
                case SG_IO: {
                case CDROM_SEND_PACKET:
                case SCSI_IOCTL_SEND_COMMAND:
                case CDROMCLOSETRAY:
                case CDROMEJECT:

My patch changes all callers of this function, and the idea is
to preserve the existing behavior through my series, so I think
it makes sense to keep my patch as is.

I would assume that calling scsi_cmd_blk_ioctl() is harmless
here, but if you want to remove it or limit the set of supported
commands, that should be independent of my change.

       Arnd

Re: [PATCH 15/24] compat_ioctl: scsi: move ioctl handling into drivers

[Michael S. Tsirkin]

On Wed, Dec 11, 2019 at 09:42:49PM +0100, Arnd Bergmann wrote:
> Each driver calling scsi_ioctl() gets an equivalent compat_ioctl()
> handler that implements the same commands by calling scsi_compat_ioctl().
> 
> The scsi_cmd_ioctl() and scsi_cmd_blk_ioctl() functions are compatible
> at this point, so any driver that calls those can do so for both native
> and compat mode, with the argument passed through compat_ptr().
> 
> With this, we can remove the entries from fs/compat_ioctl.c.  The new
> code is larger, but should be easier to maintain and keep updated with
> newly added commands.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/block/virtio_blk.c |   3 +
>  drivers/scsi/ch.c          |   9 ++-
>  drivers/scsi/sd.c          |  50 ++++++--------
>  drivers/scsi/sg.c          |  44 ++++++++-----
>  drivers/scsi/sr.c          |  57 ++++++++++++++--
>  drivers/scsi/st.c          |  51 ++++++++------
>  fs/compat_ioctl.c          | 132 +------------------------------------
>  7 files changed, 142 insertions(+), 204 deletions(-)
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 7ffd719d89de..fbbf18ac1d5d 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -405,6 +405,9 @@ static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
>  
>  static const struct block_device_operations virtblk_fops = {
>  	.ioctl  = virtblk_ioctl,
> +#ifdef CONFIG_COMPAT
> +	.compat_ioctl = blkdev_compat_ptr_ioctl,
> +#endif
>  	.owner  = THIS_MODULE,
>  	.getgeo = virtblk_getgeo,
>  };


virtio part:

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
> index 76751d6c7f0d..ed5f4a6ae270 100644
> --- a/drivers/scsi/ch.c
> +++ b/drivers/scsi/ch.c
> @@ -872,6 +872,10 @@ static long ch_ioctl_compat(struct file * file,
>  			    unsigned int cmd, unsigned long arg)
>  {
>  	scsi_changer *ch = file->private_data;
> +	int retval = scsi_ioctl_block_when_processing_errors(ch->device, cmd,
> +							file->f_flags & O_NDELAY);
> +	if (retval)
> +		return retval;
>  
>  	switch (cmd) {
>  	case CHIOGPARAMS:
> @@ -883,7 +887,7 @@ static long ch_ioctl_compat(struct file * file,
>  	case CHIOINITELEM:
>  	case CHIOSVOLTAG:
>  		/* compatible */
> -		return ch_ioctl(file, cmd, arg);
> +		return ch_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
>  	case CHIOGSTATUS32:
>  	{
>  		struct changer_element_status32 ces32;
> @@ -898,8 +902,7 @@ static long ch_ioctl_compat(struct file * file,
>  		return ch_gstatus(ch, ces32.ces_type, data);
>  	}
>  	default:
> -		// return scsi_ioctl_compat(ch->device, cmd, (void*)arg);
> -		return -ENOIOCTLCMD;
> +		return scsi_compat_ioctl(ch->device, cmd, compat_ptr(arg));
>  
>  	}
>  }
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index cea625906440..5afb0046b12a 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1465,13 +1465,12 @@ static int sd_getgeo(struct block_device *bdev, struct hd_geometry *geo)
>   *	Note: most ioctls are forward onto the block subsystem or further
>   *	down in the scsi subsystem.
>   **/
> -static int sd_ioctl(struct block_device *bdev, fmode_t mode,
> -		    unsigned int cmd, unsigned long arg)
> +static int sd_ioctl_common(struct block_device *bdev, fmode_t mode,
> +			   unsigned int cmd, void __user *p)
>  {
>  	struct gendisk *disk = bdev->bd_disk;
>  	struct scsi_disk *sdkp = scsi_disk(disk);
>  	struct scsi_device *sdp = sdkp->device;
> -	void __user *p = (void __user *)arg;
>  	int error;
>      
>  	SCSI_LOG_IOCTL(1, sd_printk(KERN_INFO, sdkp, "sd_ioctl: disk=%s, "
> @@ -1507,9 +1506,6 @@ static int sd_ioctl(struct block_device *bdev, fmode_t mode,
>  			break;
>  		default:
>  			error = scsi_cmd_blk_ioctl(bdev, mode, cmd, p);
> -			if (error != -ENOTTY)
> -				break;
> -			error = scsi_ioctl(sdp, cmd, p);
>  			break;
>  	}
>  out:
> @@ -1691,39 +1687,31 @@ static void sd_rescan(struct device *dev)
>  	revalidate_disk(sdkp->disk);
>  }
>  
> +static int sd_ioctl(struct block_device *bdev, fmode_t mode,
> +		    unsigned int cmd, unsigned long arg)
> +{
> +	void __user *p = (void __user *)arg;
> +	int ret;
> +
> +	ret = sd_ioctl_common(bdev, mode, cmd, p);
> +	if (ret != -ENOTTY)
> +		return ret;
> +
> +	return scsi_ioctl(scsi_disk(bdev->bd_disk)->device, cmd, p);
> +}
>  
>  #ifdef CONFIG_COMPAT
> -/* 
> - * This gets directly called from VFS. When the ioctl 
> - * is not recognized we go back to the other translation paths. 
> - */
>  static int sd_compat_ioctl(struct block_device *bdev, fmode_t mode,
>  			   unsigned int cmd, unsigned long arg)
>  {
> -	struct gendisk *disk = bdev->bd_disk;
> -	struct scsi_disk *sdkp = scsi_disk(disk);
> -	struct scsi_device *sdev = sdkp->device;
>  	void __user *p = compat_ptr(arg);
> -	int error;
> -
> -	error = scsi_verify_blk_ioctl(bdev, cmd);
> -	if (error < 0)
> -		return error;
> +	int ret;
>  
> -	error = scsi_ioctl_block_when_processing_errors(sdev, cmd,
> -			(mode & FMODE_NDELAY) != 0);
> -	if (error)
> -		return error;
> +	ret = sd_ioctl_common(bdev, mode, cmd, p);
> +	if (ret != -ENOTTY)
> +		return ret;
>  
> -	if (is_sed_ioctl(cmd))
> -		return sed_ioctl(sdkp->opal_dev, cmd, p);
> -	       
> -	/* 
> -	 * Let the static ioctl translation table take care of it.
> -	 */
> -	if (!sdev->host->hostt->compat_ioctl)
> -		return -ENOIOCTLCMD; 
> -	return sdev->host->hostt->compat_ioctl(sdev, cmd, p);
> +	return scsi_compat_ioctl(scsi_disk(bdev->bd_disk)->device, cmd, p);
>  }
>  #endif
>  
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 985546aac236..08efcee7a34d 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -910,19 +910,14 @@ static int put_compat_request_table(struct compat_sg_req_info __user *o,
>  #endif
>  
>  static long
> -sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
> +sg_ioctl_common(struct file *filp, Sg_device *sdp, Sg_fd *sfp,
> +		unsigned int cmd_in, void __user *p)
>  {
> -	void __user *p = (void __user *)arg;
>  	int __user *ip = p;
>  	int result, val, read_only;
> -	Sg_device *sdp;
> -	Sg_fd *sfp;
>  	Sg_request *srp;
>  	unsigned long iflags;
>  
> -	if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
> -		return -ENXIO;
> -
>  	SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp,
>  				   "sg_ioctl: cmd=0x%x\n", (int) cmd_in));
>  	read_only = (O_RDWR != (filp->f_flags & O_ACCMODE));
> @@ -1145,29 +1140,44 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
>  			cmd_in, filp->f_flags & O_NDELAY);
>  	if (result)
>  		return result;
> +
> +	return -ENOIOCTLCMD;
> +}
> +
> +static long
> +sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
> +{
> +	void __user *p = (void __user *)arg;
> +	Sg_device *sdp;
> +	Sg_fd *sfp;
> +	int ret;
> +
> +	if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
> +		return -ENXIO;
> +
> +	ret = sg_ioctl_common(filp, sdp, sfp, cmd_in, p);
> +	if (ret != -ENOIOCTLCMD)
> +		return ret;
> +
>  	return scsi_ioctl(sdp->device, cmd_in, p);
>  }
>  
>  #ifdef CONFIG_COMPAT
>  static long sg_compat_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
>  {
> +	void __user *p = compat_ptr(arg);
>  	Sg_device *sdp;
>  	Sg_fd *sfp;
> -	struct scsi_device *sdev;
> +	int ret;
>  
>  	if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
>  		return -ENXIO;
>  
> -	sdev = sdp->device;
> -	if (sdev->host->hostt->compat_ioctl) { 
> -		int ret;
> -
> -		ret = sdev->host->hostt->compat_ioctl(sdev, cmd_in, (void __user *)arg);
> -
> +	ret = sg_ioctl_common(filp, sdp, sfp, cmd_in, p);
> +	if (ret != -ENOIOCTLCMD)
>  		return ret;
> -	}
> -	
> -	return -ENOIOCTLCMD;
> +
> +	return scsi_compat_ioctl(sdp->device, cmd_in, p);
>  }
>  #endif
>  
> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index 4664fdf75c0f..6033a886c42c 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
> @@ -38,6 +38,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mm.h>
>  #include <linux/bio.h>
> +#include <linux/compat.h>
>  #include <linux/string.h>
>  #include <linux/errno.h>
>  #include <linux/cdrom.h>
> @@ -598,6 +599,55 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
>  	return ret;
>  }
>  
> +#ifdef CONFIG_COMPAT
> +static int sr_block_compat_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
> +			  unsigned long arg)
> +{
> +	struct scsi_cd *cd = scsi_cd(bdev->bd_disk);
> +	struct scsi_device *sdev = cd->device;
> +	void __user *argp = compat_ptr(arg);
> +	int ret;
> +
> +	mutex_lock(&sr_mutex);
> +
> +	ret = scsi_ioctl_block_when_processing_errors(sdev, cmd,
> +			(mode & FMODE_NDELAY) != 0);
> +	if (ret)
> +		goto out;
> +
> +	scsi_autopm_get_device(sdev);
> +
> +	/*
> +	 * Send SCSI addressing ioctls directly to mid level, send other
> +	 * ioctls to cdrom/block level.
> +	 */
> +	switch (cmd) {
> +	case SCSI_IOCTL_GET_IDLUN:
> +	case SCSI_IOCTL_GET_BUS_NUMBER:
> +		ret = scsi_compat_ioctl(sdev, cmd, argp);
> +		goto put;
> +	}
> +
> +	/*
> +	 * CDROM ioctls are handled in the block layer, but
> +	 * do the scsi blk ioctls here.
> +	 */
> +	ret = scsi_cmd_blk_ioctl(bdev, mode, cmd, argp);
> +	if (ret != -ENOTTY)
> +		return ret;
> +
> +	ret = scsi_compat_ioctl(sdev, cmd, argp);
> +
> +put:
> +	scsi_autopm_put_device(sdev);
> +
> +out:
> +	mutex_unlock(&sr_mutex);
> +	return ret;
> +
> +}
> +#endif
> +
>  static unsigned int sr_block_check_events(struct gendisk *disk,
>  					  unsigned int clearing)
>  {
> @@ -641,12 +691,11 @@ static const struct block_device_operations sr_bdops =
>  	.open		= sr_block_open,
>  	.release	= sr_block_release,
>  	.ioctl		= sr_block_ioctl,
> +#ifdef CONFIG_COMPAT
> +	.ioctl		= sr_block_compat_ioctl,
> +#endif
>  	.check_events	= sr_block_check_events,
>  	.revalidate_disk = sr_block_revalidate_disk,
> -	/* 
> -	 * No compat_ioctl for now because sr_block_ioctl never
> -	 * seems to pass arbitrary ioctls down to host drivers.
> -	 */
>  };
>  
>  static int sr_open(struct cdrom_device_info *cdi, int purpose)
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index 9e3fff2de83e..393f3019ccac 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -3501,7 +3501,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)
> +static long st_ioctl_common(struct file *file, unsigned int cmd_in, void __user *p)
>  {
>  	int i, cmd_nr, cmd_type, bt;
>  	int retval = 0;
> @@ -3509,7 +3509,6 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
>  	struct scsi_tape *STp = file->private_data;
>  	struct st_modedef *STm;
>  	struct st_partstat *STps;
> -	void __user *p = (void __user *)arg;
>  
>  	if (mutex_lock_interruptible(&STp->lock))
>  		return -ERESTARTSYS;
> @@ -3824,9 +3823,19 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
>  	}
>  	mutex_unlock(&STp->lock);
>  	switch (cmd_in) {
> +		case SCSI_IOCTL_STOP_UNIT:
> +			/* unload */
> +			retval = scsi_ioctl(STp->device, cmd_in, p);
> +			if (!retval) {
> +				STp->rew_at_close = 0;
> +				STp->ready = ST_NO_TAPE;
> +			}
> +			return retval;
> +
>  		case SCSI_IOCTL_GET_IDLUN:
>  		case SCSI_IOCTL_GET_BUS_NUMBER:
>  			break;
> +
>  		default:
>  			if ((cmd_in == SG_IO ||
>  			     cmd_in == SCSI_IOCTL_SEND_COMMAND ||
> @@ -3840,42 +3849,46 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
>  				return i;
>  			break;
>  	}
> -	retval = scsi_ioctl(STp->device, cmd_in, p);
> -	if (!retval && cmd_in == SCSI_IOCTL_STOP_UNIT) { /* unload */
> -		STp->rew_at_close = 0;
> -		STp->ready = ST_NO_TAPE;
> -	}
> -	return retval;
> +	return -ENOTTY;
>  
>   out:
>  	mutex_unlock(&STp->lock);
>  	return retval;
>  }
>  
> +static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
> +{
> +	void __user *p = (void __user *)arg;
> +	struct scsi_tape *STp = file->private_data;
> +	int ret;
> +
> +	ret = st_ioctl_common(file, cmd_in, p);
> +	if (ret != -ENOTTY)
> +		return ret;
> +
> +	return scsi_ioctl(STp->device, cmd_in, p);
> +}
> +
>  #ifdef CONFIG_COMPAT
>  static long st_compat_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
>  {
>  	void __user *p = compat_ptr(arg);
>  	struct scsi_tape *STp = file->private_data;
> -	struct scsi_device *sdev = STp->device;
> -	int ret = -ENOIOCTLCMD;
> +	int ret;
>  
>  	/* argument conversion is handled using put_user_mtpos/put_user_mtget */
>  	switch (cmd_in) {
> -	case MTIOCTOP:
> -		return st_ioctl(file, MTIOCTOP, (unsigned long)p);
>  	case MTIOCPOS32:
> -		return st_ioctl(file, MTIOCPOS, (unsigned long)p);
> +		return st_ioctl_common(file, MTIOCPOS, p);
>  	case MTIOCGET32:
> -		return st_ioctl(file, MTIOCGET, (unsigned long)p);
> +		return st_ioctl_common(file, MTIOCGET, p);
>  	}
>  
> -	if (sdev->host->hostt->compat_ioctl) { 
> +	ret = st_ioctl_common(file, cmd_in, p);
> +	if (ret != -ENOTTY)
> +		return ret;
>  
> -		ret = sdev->host->hostt->compat_ioctl(sdev, cmd_in, (void __user *)arg);
> -
> -	}
> -	return ret;
> +	return scsi_compat_ioctl(STp->device, cmd_in, p);
>  }
>  #endif
>  
> diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
> index 358ea2ecf36b..ab4471f469e6 100644
> --- a/fs/compat_ioctl.c
> +++ b/fs/compat_ioctl.c
> @@ -36,109 +36,11 @@
>  
>  #include "internal.h"
>  
> -#ifdef CONFIG_BLOCK
> -#include <linux/cdrom.h>
> -#include <linux/fd.h>
> -#include <scsi/scsi.h>
> -#include <scsi/scsi_ioctl.h>
> -#include <scsi/sg.h>
> -#endif
> -
>  #include <linux/uaccess.h>
>  #include <linux/watchdog.h>
>  
>  #include <linux/hiddev.h>
>  
> -
> -#include <linux/sort.h>
> -
> -/*
> - * simple reversible transform to make our table more evenly
> - * distributed after sorting.
> - */
> -#define XFORM(i) (((i) ^ ((i) << 27) ^ ((i) << 17)) & 0xffffffff)
> -
> -#define COMPATIBLE_IOCTL(cmd) XFORM((u32)cmd),
> -static unsigned int ioctl_pointer[] = {
> -#ifdef CONFIG_BLOCK
> -/* Big S */
> -COMPATIBLE_IOCTL(SCSI_IOCTL_GET_IDLUN)
> -COMPATIBLE_IOCTL(SCSI_IOCTL_DOORLOCK)
> -COMPATIBLE_IOCTL(SCSI_IOCTL_DOORUNLOCK)
> -COMPATIBLE_IOCTL(SCSI_IOCTL_TEST_UNIT_READY)
> -COMPATIBLE_IOCTL(SCSI_IOCTL_GET_BUS_NUMBER)
> -COMPATIBLE_IOCTL(SCSI_IOCTL_SEND_COMMAND)
> -COMPATIBLE_IOCTL(SCSI_IOCTL_PROBE_HOST)
> -COMPATIBLE_IOCTL(SCSI_IOCTL_GET_PCI)
> -#endif
> -#ifdef CONFIG_BLOCK
> -/* SG stuff */
> -COMPATIBLE_IOCTL(SG_IO)
> -COMPATIBLE_IOCTL(SG_GET_REQUEST_TABLE)
> -COMPATIBLE_IOCTL(SG_SET_TIMEOUT)
> -COMPATIBLE_IOCTL(SG_GET_TIMEOUT)
> -COMPATIBLE_IOCTL(SG_EMULATED_HOST)
> -COMPATIBLE_IOCTL(SG_GET_TRANSFORM)
> -COMPATIBLE_IOCTL(SG_SET_RESERVED_SIZE)
> -COMPATIBLE_IOCTL(SG_GET_RESERVED_SIZE)
> -COMPATIBLE_IOCTL(SG_GET_SCSI_ID)
> -COMPATIBLE_IOCTL(SG_SET_FORCE_LOW_DMA)
> -COMPATIBLE_IOCTL(SG_GET_LOW_DMA)
> -COMPATIBLE_IOCTL(SG_SET_FORCE_PACK_ID)
> -COMPATIBLE_IOCTL(SG_GET_PACK_ID)
> -COMPATIBLE_IOCTL(SG_GET_NUM_WAITING)
> -COMPATIBLE_IOCTL(SG_SET_DEBUG)
> -COMPATIBLE_IOCTL(SG_GET_SG_TABLESIZE)
> -COMPATIBLE_IOCTL(SG_GET_COMMAND_Q)
> -COMPATIBLE_IOCTL(SG_SET_COMMAND_Q)
> -COMPATIBLE_IOCTL(SG_GET_VERSION_NUM)
> -COMPATIBLE_IOCTL(SG_NEXT_CMD_LEN)
> -COMPATIBLE_IOCTL(SG_SCSI_RESET)
> -COMPATIBLE_IOCTL(SG_GET_REQUEST_TABLE)
> -COMPATIBLE_IOCTL(SG_SET_KEEP_ORPHAN)
> -COMPATIBLE_IOCTL(SG_GET_KEEP_ORPHAN)
> -#endif
> -};
> -
> -/*
> - * Convert common ioctl arguments based on their command number
> - *
> - * Please do not add any code in here. Instead, implement
> - * a compat_ioctl operation in the place that handleѕ the
> - * ioctl for the native case.
> - */
> -static long do_ioctl_trans(unsigned int cmd,
> -		 unsigned long arg, struct file *file)
> -{
> -	return -ENOIOCTLCMD;
> -}
> -
> -static int compat_ioctl_check_table(unsigned int xcmd)
> -{
> -#ifdef CONFIG_BLOCK
> -	int i;
> -	const int max = ARRAY_SIZE(ioctl_pointer) - 1;
> -
> -	BUILD_BUG_ON(max >= (1 << 16));
> -
> -	/* guess initial offset into table, assuming a
> -	   normalized distribution */
> -	i = ((xcmd >> 16) * max) >> 16;
> -
> -	/* do linear search up first, until greater or equal */
> -	while (ioctl_pointer[i] < xcmd && i < max)
> -		i++;
> -
> -	/* then do linear search down */
> -	while (ioctl_pointer[i] > xcmd && i > 0)
> -		i--;
> -
> -	return ioctl_pointer[i] == xcmd;
> -#else
> -	return 0;
> -#endif
> -}
> -
>  COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
>  		       compat_ulong_t, arg32)
>  {
> @@ -216,19 +118,9 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
>  				goto out_fput;
>  		}
>  
> -		if (!f.file->f_op->unlocked_ioctl)
> -			goto do_ioctl;
> -		break;
> -	}
> -
> -	if (compat_ioctl_check_table(XFORM(cmd)))
> -		goto found_handler;
> -
> -	error = do_ioctl_trans(cmd, arg, f.file);
> -	if (error == -ENOIOCTLCMD)
>  		error = -ENOTTY;
> -
> -	goto out_fput;
> +		goto out_fput;
> +	}
>  
>   found_handler:
>  	arg = (unsigned long)compat_ptr(arg);
> @@ -239,23 +131,3 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
>   out:
>  	return error;
>  }
> -
> -static int __init init_sys32_ioctl_cmp(const void *p, const void *q)
> -{
> -	unsigned int a, b;
> -	a = *(unsigned int *)p;
> -	b = *(unsigned int *)q;
> -	if (a > b)
> -		return 1;
> -	if (a < b)
> -		return -1;
> -	return 0;
> -}
> -
> -static int __init init_sys32_ioctl(void)
> -{
> -	sort(ioctl_pointer, ARRAY_SIZE(ioctl_pointer), sizeof(*ioctl_pointer),
> -		init_sys32_ioctl_cmp, NULL);
> -	return 0;
> -}
> -__initcall(init_sys32_ioctl);
> -- 
> 2.20.0

Re: [PATCH 15/24] compat_ioctl: scsi: move ioctl handling into drivers

[Michael S. Tsirkin]

On Thu, Dec 12, 2019 at 01:28:08AM +0100, Paolo Bonzini wrote:
> On 12/12/19 00:05, Michael S. Tsirkin wrote:
> >> @@ -405,6 +405,9 @@ static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
> >>  
> >>  static const struct block_device_operations virtblk_fops = {
> >>  	.ioctl  = virtblk_ioctl,
> >> +#ifdef CONFIG_COMPAT
> >> +	.compat_ioctl = blkdev_compat_ptr_ioctl,
> >> +#endif
> >>  	.owner  = THIS_MODULE,
> >>  	.getgeo = virtblk_getgeo,
> >>  };
> > Hmm - is virtio blk lumped in with scsi things intentionally?
> 
> I think it's because the only ioctl for virtio-blk is SG_IO.

Oh right, I forgot about that one ...

>  It makes
> sense to lump it in with scsi, but I wouldn't mind getting rid of
> CONFIG_VIRTIO_BLK_SCSI altogether.
> 
> Paolo

Re: [PATCH 02/24] compat: scsi: sg: fix v3 compat read/write interface

[Christoph Hellwig]

On Wed, Dec 11, 2019 at 09:42:36PM +0100, Arnd Bergmann wrote:
> In the v5.4 merge window, a cleanup patch from Al Viro conflicted
> with my rework of the compat handling for sg.c read(). Linus Torvalds
> did a correct merge but pointed out that the resulting code is still
> unsatisfactory.
> 
> I later noticed that the sg_new_read() function still gets the compat
> mode wrong, when the 'count' argument is large enough to pass a
> compat_sg_io_hdr object, but not a nativ sg_io_hdr.
> 
> To address both of these, move the definition of compat_sg_io_hdr
> into a scsi/sg.h to make it visible to sg.c and rewrite the logic
> for reading req_pack_id as well as the size check to a simpler
> version that gets the expected results.
> 
> Fixes: c35a5cfb4150 ("scsi: sg: sg_read(): simplify reading ->pack_id of userland sg_io_hdr_t")
> Fixes: 98aaaec4a150 ("compat_ioctl: reimplement SG_IO handling")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  block/scsi_ioctl.c |  29 +----------
>  drivers/scsi/sg.c  | 125 +++++++++++++++++++++------------------------
>  include/scsi/sg.h  |  30 +++++++++++
>  3 files changed, 89 insertions(+), 95 deletions(-)
> 
> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
> index 650bade5ea5a..b61dbf4d8443 100644
> --- a/block/scsi_ioctl.c
> +++ b/block/scsi_ioctl.c
> @@ -20,6 +20,7 @@
>  #include <scsi/scsi.h>
>  #include <scsi/scsi_ioctl.h>
>  #include <scsi/scsi_cmnd.h>
> +#include <scsi/sg.h>
>  
>  struct blk_cmd_filter {
>  	unsigned long read_ok[BLK_SCSI_CMD_PER_LONG];
> @@ -550,34 +551,6 @@ static inline int blk_send_start_stop(struct request_queue *q,
>  	return __blk_send_generic(q, bd_disk, GPCMD_START_STOP_UNIT, data);
>  }
>  
> -#ifdef CONFIG_COMPAT
> -struct compat_sg_io_hdr {
> -	compat_int_t interface_id;	/* [i] 'S' for SCSI generic (required) */
> -	compat_int_t dxfer_direction;	/* [i] data transfer direction  */
> -	unsigned char cmd_len;		/* [i] SCSI command length ( <= 16 bytes) */
> -	unsigned char mx_sb_len;	/* [i] max length to write to sbp */
> -	unsigned short iovec_count;	/* [i] 0 implies no scatter gather */
> -	compat_uint_t dxfer_len;	/* [i] byte count of data transfer */
> -	compat_uint_t dxferp;		/* [i], [*io] points to data transfer memory
> -						or scatter gather list */
> -	compat_uptr_t cmdp;		/* [i], [*i] points to command to perform */
> -	compat_uptr_t sbp;		/* [i], [*o] points to sense_buffer memory */
> -	compat_uint_t timeout;		/* [i] MAX_UINT->no timeout (unit: millisec) */
> -	compat_uint_t flags;		/* [i] 0 -> default, see SG_FLAG... */
> -	compat_int_t pack_id;		/* [i->o] unused internally (normally) */
> -	compat_uptr_t usr_ptr;		/* [i->o] unused internally */
> -	unsigned char status;		/* [o] scsi status */
> -	unsigned char masked_status;	/* [o] shifted, masked scsi status */
> -	unsigned char msg_status;	/* [o] messaging level data (optional) */
> -	unsigned char sb_len_wr;	/* [o] byte count actually written to sbp */
> -	unsigned short host_status;	/* [o] errors from host adapter */
> -	unsigned short driver_status;	/* [o] errors from software driver */
> -	compat_int_t resid;		/* [o] dxfer_len - actual_transferred */
> -	compat_uint_t duration;		/* [o] time taken by cmd (unit: millisec) */
> -	compat_uint_t info;		/* [o] auxiliary information */
> -};
> -#endif
> -
>  int put_sg_io_hdr(const struct sg_io_hdr *hdr, void __user *argp)
>  {
>  #ifdef CONFIG_COMPAT
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 160748ad9c0f..985546aac236 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -198,6 +198,7 @@ static void sg_device_destroy(struct kref *kref);
>  
>  #define SZ_SG_HEADER sizeof(struct sg_header)
>  #define SZ_SG_IO_HDR sizeof(sg_io_hdr_t)
> +#define SZ_COMPAT_SG_IO_HDR sizeof(struct compat_sg_io_hdr)

I'd rather not add more defines like this.  The raw sizeof is
much more readable and obvious.

>  
> +	if (count < SZ_SG_HEADER)
> +		goto unknown_id;
> +
> +	/* negative reply_len means v3 format, otherwise v1/v2 */
> +	if (get_user(reply_len, &old_hdr->reply_len))
> +		return -EFAULT;
> +	if (reply_len >= 0)
> +		return get_user(*pack_id, &old_hdr->pack_id);
> +
> +	if (in_compat_syscall() && count >= SZ_COMPAT_SG_IO_HDR) {
> +		struct compat_sg_io_hdr __user *hp = buf;
> +		return get_user(*pack_id, &hp->pack_id);
> +	}
> +
> +	if (count >= SZ_SG_IO_HDR) {
> +		struct sg_io_hdr __user *hp = buf;
> +		return get_user(*pack_id, &hp->pack_id);
> +	}
> +
> +unknown_id:
> +	/* no valid header was passed, so ignore the pack_id */
> +	*pack_id = -1;
> +	return 0;
> +}

I find the structure here a little confusing, as it doesn't follow
the normal flow.  What do you think of:

	if (count >= SZ_SG_HEADER) {
		if (get_user(reply_len, &old_hdr->reply_len))
			return -EFAULT;

		/* negative reply_len means v3 format, otherwise v1/v2 */
		if (reply_len >= 0)
			return get_user(*pack_id, &old_hdr->pack_id);

		if (in_compat_syscall() {
			if (count >= SZ_COMPAT_SG_IO_HDR) {
				struct compat_sg_io_hdr __user *hp = buf;

				return get_user(*pack_id, &hp->pack_id);
			}
		} else {
			if (count >= SZ_SG_IO_HDR) {
				struct sg_io_hdr __user *hp = buf;

				return get_user(*pack_id, &hp->pack_id);
			}
		}
	}

	/* no valid header was passed, so ignore the pack_id */
	*pack_id = -1;
	return 0;
}

Re: [PATCH 15/24] compat_ioctl: scsi: move ioctl handling into drivers

[Christoph Hellwig]

On Thu, Dec 12, 2019 at 01:28:08AM +0100, Paolo Bonzini wrote:
> I think it's because the only ioctl for virtio-blk is SG_IO.  It makes
> sense to lump it in with scsi, but I wouldn't mind getting rid of
> CONFIG_VIRTIO_BLK_SCSI altogether.

CONFIG_VIRTIO_BLK_SCSI has been broken for about two years, as it
never set the QUEUE_FLAG_SCSI_PASSTHROUGH flag after that was
introduced.  I actually have a patch that I plan to send to remove
this support as it was a really idea to start with (speaking as
the person who had that idea back in the day).

Re: [PATCH 15/24] compat_ioctl: scsi: move ioctl handling into drivers

[Jens Axboe]

On 12/12/19 9:27 AM, Christoph Hellwig wrote:
> On Thu, Dec 12, 2019 at 01:28:08AM +0100, Paolo Bonzini wrote:
>> I think it's because the only ioctl for virtio-blk is SG_IO.  It makes
>> sense to lump it in with scsi, but I wouldn't mind getting rid of
>> CONFIG_VIRTIO_BLK_SCSI altogether.
> 
> CONFIG_VIRTIO_BLK_SCSI has been broken for about two years, as it
> never set the QUEUE_FLAG_SCSI_PASSTHROUGH flag after that was
> introduced.  I actually have a patch that I plan to send to remove
> this support as it was a really idea to start with (speaking as
> the person who had that idea back in the day).

We had this very discussion two years ago, and we never got it removed.
Let's please get it done.

-- 
Jens Axboe

Re: [PATCH 02/24] compat: scsi: sg: fix v3 compat read/write interface

[Arnd Bergmann]

On Thu, Dec 12, 2019 at 5:25 PM Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Dec 11, 2019 at 09:42:36PM +0100, Arnd Bergmann wrote:

> > --- a/drivers/scsi/sg.c
> > +++ b/drivers/scsi/sg.c
> > @@ -198,6 +198,7 @@ static void sg_device_destroy(struct kref *kref);
> >
> >  #define SZ_SG_HEADER sizeof(struct sg_header)
> >  #define SZ_SG_IO_HDR sizeof(sg_io_hdr_t)
> > +#define SZ_COMPAT_SG_IO_HDR sizeof(struct compat_sg_io_hdr)
>
> I'd rather not add more defines like this.  The raw sizeof is
> much more readable and obvious.

Done. I actually had it that way in the previous submission and then changed
it for consistency. I considered removing SZ_SG_IO_HDR as well,
but decided not to make Doug's life harder than necessary -- he has nother
50 or so patches on top of this that he needs to rebase.

> I find the structure here a little confusing, as it doesn't follow
> the normal flow.  What do you think of:
>
>         if (count >= SZ_SG_HEADER) {
>                 if (get_user(reply_len, &old_hdr->reply_len))
>                         return -EFAULT;

I don't see much benefit either way. Changed it now it as you suggested.

Thanks for the review!

      Arnd