Re: [PATCH v3 13/22] compat_ioctl: scsi: move ioctl handling into drivers

2020-02-12 Thread Arnd Bergmann
On Wed, Feb 12, 2020 at 10:15 PM Johannes Hirte
 wrote:
>
> On 2020 Jan 02, Arnd Bergmann wrote:

>
> Error in getting drive hardware properties
> Error in getting drive reading properties
> Error in getting drive writing properties
> __
>
> Disc mode is listed as: CD-DA
> ++ WARN: error in ioctl CDROMREADTOCHDR: Bad address
>
> cd-info: Can't get first track number. I give up.

Right, there was also a report about breaking the Fedora installer,
see https://bugzilla.redhat.com/show_bug.cgi?id=1801353

There is a preliminary patch that should fix this, I'll post a
version with more references tomorrow:
https://www.happyassassin.net/temp/0001-Replace-.ioctl-with-.compat_ioctl-in-three-appropria.patch

  Arnd
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 13/22] compat_ioctl: scsi: move ioctl handling into drivers

2020-01-02 Thread Stefan Hajnoczi
On Thu, Jan 02, 2020 at 03:55:31PM +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 
> ---
>  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(-)

virtio_blk.c changes:

Acked-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v3 13/22] compat_ioctl: scsi: move ioctl handling into drivers

2020-01-02 Thread 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 
---
 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