diff mbox series

[v2,17/27] compat_ioctl: ide: floppy: add handler

Message ID 20191217221708.3730997-18-arnd@arndb.de (mailing list archive)
State New, archived
Headers show
Series [v2,01/27] pktcdvd: fix regression on 64-bit architectures | expand

Commit Message

Arnd Bergmann Dec. 17, 2019, 10:16 p.m. UTC 
Rather than relying on fs/compat_ioctl.c, this adds support
for a compat_ioctl() callback in the ide-floppy driver directly,
which lets it translate the scsi commands.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/ide/ide-floppy.c       |  4 ++++
 drivers/ide/ide-floppy.h       |  2 ++
 drivers/ide/ide-floppy_ioctl.c | 36 ++++++++++++++++++++++++++++++++++
 drivers/ide/ide-gd.c           | 14 +++++++++++++
 include/linux/ide.h            |  2 ++
 5 files changed, 58 insertions(+)

Comments

Ben Hutchings Dec. 18, 2019, 6:45 p.m. UTC | #1 
On Tue, 2019-12-17 at 23:16 +0100, Arnd Bergmann wrote:
> Rather than relying on fs/compat_ioctl.c, this adds support
> for a compat_ioctl() callback in the ide-floppy driver directly,
> which lets it translate the scsi commands.
[...]

After this, and before "compat_ioctl: move HDIO ioctl handling into
drivers/ide", compat ioctls on an IDE hard drive will result in a null
pointer dereference in ide_gd_compat_ioctl().  Not sure how much that
really matters though.

Ben.
Arnd Bergmann Dec. 18, 2019, 7:28 p.m. UTC | #2 
On Wed, Dec 18, 2019 at 7:45 PM Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
>
> On Tue, 2019-12-17 at 23:16 +0100, Arnd Bergmann wrote:
> > Rather than relying on fs/compat_ioctl.c, this adds support
> > for a compat_ioctl() callback in the ide-floppy driver directly,
> > which lets it translate the scsi commands.
> [...]
>
> After this, and before "compat_ioctl: move HDIO ioctl handling into
> drivers/ide", compat ioctls on an IDE hard drive will result in a null
> pointer dereference in ide_gd_compat_ioctl().  Not sure how much that
> really matters though.

I'm sure it makes no difference in the end, but you are of course right that
this is a bug. I've folded in a check now, and leaving that in place
even after it is no longer needed:

--- a/drivers/ide/ide-gd.c
+++ b/drivers/ide/ide-gd.c
@@ -348,6 +348,9 @@ static int ide_gd_compat_ioctl(struct block_device
*bdev, fmode_t mode,
        struct ide_disk_obj *idkp = ide_drv_g(bdev->bd_disk, ide_disk_obj);
        ide_drive_t *drive = idkp->drive;

+       if (!drive->disk_ops->compat_ioctl)
+               return -ENOIOCTLCMD;
+
        return drive->disk_ops->compat_ioctl(drive, bdev, mode, cmd, arg);
 }
 #endif

I pushed out an updated signed tag with this change.

Thanks for the continued careful review!

       Arnd
diff mbox series

Patch

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 1ea2f9e82bf8..1fe1f9d37a51 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -19,6 +19,7 @@ 
 #include <linux/types.h>
 #include <linux/string.h>
 #include <linux/kernel.h>
+#include <linux/compat.h>
 #include <linux/delay.h>
 #include <linux/timer.h>
 #include <linux/mm.h>
@@ -546,4 +547,7 @@  const struct ide_disk_ops ide_atapi_disk_ops = {
 	.set_doorlock	= ide_set_media_lock,
 	.do_request	= ide_floppy_do_request,
 	.ioctl		= ide_floppy_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= ide_floppy_compat_ioctl,
+#endif
 };
diff --git a/drivers/ide/ide-floppy.h b/drivers/ide/ide-floppy.h
index 13c9b4b6d75e..8505a5f58f4e 100644
--- a/drivers/ide/ide-floppy.h
+++ b/drivers/ide/ide-floppy.h
@@ -26,6 +26,8 @@  void ide_floppy_create_read_capacity_cmd(struct ide_atapi_pc *);
 /* ide-floppy_ioctl.c */
 int ide_floppy_ioctl(ide_drive_t *, struct block_device *, fmode_t,
 		     unsigned int, unsigned long);
+int ide_floppy_compat_ioctl(ide_drive_t *, struct block_device *, fmode_t,
+			    unsigned int, unsigned long);
 
 #ifdef CONFIG_IDE_PROC_FS
 /* ide-floppy_proc.c */
diff --git a/drivers/ide/ide-floppy_ioctl.c b/drivers/ide/ide-floppy_ioctl.c
index 40a2ebe34e1d..4fd70f804d6f 100644
--- a/drivers/ide/ide-floppy_ioctl.c
+++ b/drivers/ide/ide-floppy_ioctl.c
@@ -5,6 +5,7 @@ 
 
 #include <linux/kernel.h>
 #include <linux/ide.h>
+#include <linux/compat.h>
 #include <linux/cdrom.h>
 #include <linux/mutex.h>
 
@@ -302,3 +303,38 @@  int ide_floppy_ioctl(ide_drive_t *drive, struct block_device *bdev,
 	mutex_unlock(&ide_floppy_ioctl_mutex);
 	return err;
 }
+
+#ifdef CONFIG_COMPAT
+int ide_floppy_compat_ioctl(ide_drive_t *drive, struct block_device *bdev,
+			    fmode_t mode, unsigned int cmd, unsigned long arg)
+{
+	struct ide_atapi_pc pc;
+	void __user *argp = compat_ptr(arg);
+	int err;
+
+	mutex_lock(&ide_floppy_ioctl_mutex);
+	if (cmd == CDROMEJECT || cmd == CDROM_LOCKDOOR) {
+		err = ide_floppy_lockdoor(drive, &pc, arg, cmd);
+		goto out;
+	}
+
+	err = ide_floppy_format_ioctl(drive, &pc, mode, cmd, argp);
+	if (err != -ENOTTY)
+		goto out;
+
+	/*
+	 * skip SCSI_IOCTL_SEND_COMMAND (deprecated)
+	 * and CDROM_SEND_PACKET (legacy) ioctls
+	 */
+	if (cmd != CDROM_SEND_PACKET && cmd != SCSI_IOCTL_SEND_COMMAND)
+		err = scsi_cmd_blk_ioctl(bdev, mode, cmd, argp);
+
+	/*
+	 * there is no generic_ide_compat_ioctl(), that is handled
+	 * through compat_blkdev_ioctl().
+	 */
+out:
+	mutex_unlock(&ide_floppy_ioctl_mutex);
+	return err;
+}
+#endif
diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c
index dba9ad5c97b3..1b0270efcce2 100644
--- a/drivers/ide/ide-gd.c
+++ b/drivers/ide/ide-gd.c
@@ -341,11 +341,25 @@  static int ide_gd_ioctl(struct block_device *bdev, fmode_t mode,
 	return drive->disk_ops->ioctl(drive, bdev, mode, cmd, arg);
 }
 
+#ifdef CONFIG_COMPAT
+static int ide_gd_compat_ioctl(struct block_device *bdev, fmode_t mode,
+			       unsigned int cmd, unsigned long arg)
+{
+	struct ide_disk_obj *idkp = ide_drv_g(bdev->bd_disk, ide_disk_obj);
+	ide_drive_t *drive = idkp->drive;
+
+	return drive->disk_ops->compat_ioctl(drive, bdev, mode, cmd, arg);
+}
+#endif
+
 static const struct block_device_operations ide_gd_ops = {
 	.owner			= THIS_MODULE,
 	.open			= ide_gd_unlocked_open,
 	.release		= ide_gd_release,
 	.ioctl			= ide_gd_ioctl,
+#ifdef CONFIG_COMPAT
+	.ioctl			= ide_gd_compat_ioctl,
+#endif
 	.getgeo			= ide_gd_getgeo,
 	.check_events		= ide_gd_check_events,
 	.unlock_native_capacity	= ide_gd_unlock_native_capacity,
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 46b771d6999e..06dae6438557 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -413,6 +413,8 @@  struct ide_disk_ops {
 				      sector_t);
 	int		(*ioctl)(struct ide_drive_s *, struct block_device *,
 				 fmode_t, unsigned int, unsigned long);
+	int		(*compat_ioctl)(struct ide_drive_s *, struct block_device *,
+					fmode_t, unsigned int, unsigned long);
 };
 
 /* ATAPI device flags */