Skip to contentSkip to conversation

compat_ioctl: introduce block/compat_ioctl.c

PATCH Superseded Arnd Bergmann 5 participants 19 messages Oct 6, 2007 – Oct 9, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover compat_ioctl: introduce block/compat_ioctl.c

This is my block ioctl series split up into managable chunks. I'm not really sure about the last two of these, I'd prefer to get a second opinion on those.

Please apply once your tests have gone though.

	Arnd <><
#1 lore raw

PATCH 1/9 compat_ioctl: move common block ioctls to compat_blkdev_ioctl

Make compat_blkdev_ioctl and blkdev_ioctl reflect the respective native versions. This is somewhat more efficient and makes it easier to keep the two in sync.

block/Makefile +1 -0
Index: linux-2.6/block/Makefile
===================================================================
@@ -11,3 +11,4 @@ obj-$(CONFIG_IOSCHED_DEADLINE)	+= deadli
 obj-$(CONFIG_IOSCHED_CFQ)	+= cfq-iosched.o
 
 obj-$(CONFIG_BLK_DEV_IO_TRACE)	+= blktrace.o
+obj-$(CONFIG_COMPAT)		+= compat_ioctl.o
block/compat_ioctl.c new file +123 -0

123 lines not on this page · raw message

block/ioctl.c +4 -17
Index: linux-2.6/block/ioctl.c
===================================================================
@@ -217,6 +217,10 @@ int blkdev_driver_ioctl(struct inode *in
 }
 EXPORT_SYMBOL_GPL(blkdev_driver_ioctl);
 
+/*
+ * always keep this in sync with compat_blkdev_ioctl() and
+ * compat_blkdev_locked_ioctl()
+ */
 int blkdev_ioctl(struct inode *inode, struct file *file, unsigned cmd,
 			unsigned long arg)
 {
@@ -284,21 +288,4 @@ int blkdev_ioctl(struct inode *inode, st
 
 	return blkdev_driver_ioctl(inode, file, disk, cmd, arg);
 }
-
-/* Most of the generic ioctls are handled in the normal fallback path.
-   This assumes the blkdev's low level compat_ioctl always returns
-   ENOIOCTLCMD for unknown ioctls. */
-long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
-{
-	struct block_device *bdev = file->f_path.dentry->d_inode->i_bdev;
-	struct gendisk *disk = bdev->bd_disk;
-	int ret = -ENOIOCTLCMD;
-	if (disk->fops->compat_ioctl) {
-		lock_kernel();
-		ret = disk->fops->compat_ioctl(file, cmd, arg);
-		unlock_kernel();
-	}
-	return ret;
-}
-
 EXPORT_SYMBOL_GPL(blkdev_ioctl);
fs/compat_ioctl.c +0 -45

80 lines not on this page · raw message

#2 lore raw

Re: [patch 1/9] compat_ioctl: move common block ioctls to compat_blkdev_ioctl

Quoted 14 lines · Arnd Bergmann
block/compat_ioctl.c new file +14 -0
+static int compat_put_ushort(unsigned long arg, unsigned short val)
+{
+	return put_user(val, (unsigned short __user *)compat_ptr(arg));
+}
+
+static int compat_put_int(unsigned long arg, int val)
+{
+	return put_user(val, (compat_int_t __user *)compat_ptr(arg));
+}
+
+static int compat_put_long(unsigned long arg, long val)
+{
+	return put_user(val, (compat_long_t __user *)compat_ptr(arg));
+}

block/compat_ioctl.c line 37

Quoted 10 lines
block/compat_ioctl.c new file +10 -0
+
+static int compat_put_ulong(unsigned long arg, compat_ulong_t val)
+{
+	return put_user(val, (compat_ulong_t __user *)compat_ptr(arg));
+}
+
+static int compat_put_u64(unsigned long arg, u64 val)
+{
+	return put_user(val, (compat_u64 __user *)compat_ptr(arg));
+}

These should probably be in compat.h

#3 lore raw

Re: [patch 1/9] compat_ioctl: move common block ioctls to compat_blkdev_ioctl

Quoted 6 lines · Christoph Hellwig

block/compat_ioctl.c line 37

Quoted 4 lines
block/compat_ioctl.c new file +4 -0
+static int compat_put_u64(unsigned long arg, u64 val)
+{
+     return put_user(val, (compat_u64 __user *)compat_ptr(arg));
+}

These should probably be in compat.h

Hmm, I'm not sure. They are modeled after the put_{short,int,uint,...} functions that only exist in block/ioctl.c and I put them in so I could use almost identical code in the native and compat path.

If we find these generally useful, I guess we could have both versions of each of these functions in global headers, e.g. in linux/uaccess.h and linux/compat.h, but right now I would recommend using plain put_user in all compat code except for block ioctl.

	Arnd <><
#4 lore raw

PATCH 2/9 compat_ioctl: add compat_blkdev_driver_ioctl()

In reply to Arnd Bergmann

Handle those blockdev ioctl calls that are compatible directly from the compat_blkdev_ioctl() function, instead of having to go through the compat_ioctl hash lookup.

block/compat_ioctl.c +120 -1
Index: linux-2.6/block/compat_ioctl.c
===================================================================
@@ -40,6 +40,122 @@ static int compat_put_u64(unsigned long 
 #define BLKBSZSET_32		_IOW(0x12, 113, int)
 #define BLKGETSIZE64_32		_IOR(0x12, 114, int)
 
+static int compat_blkdev_driver_ioctl(struct inode *inode, struct file *file,
+			struct gendisk *disk, unsigned cmd, unsigned long arg)
+{
+	int ret;
+
+	switch (arg) {
+	/*
+	 * No handler required for the ones below, we just need to
+	 * convert arg to a 64 bit pointer.
+	 */
+	case BLKSECTSET:
+	/*
+	 * 0x03 -- HD/IDE ioctl's used by hdparm and friends.
+	 *         Some need translations, these do not.
+	 */
+	case HDIO_GET_IDENTITY:
+	case HDIO_DRIVE_TASK:
+	case HDIO_DRIVE_CMD:
+	case HDIO_SCAN_HWIF:
+	/* 0x330 is reserved -- it used to be HDIO_GETGEO_BIG */
+	case 0x330:
+	/* 0x02 -- Floppy ioctls */
+	case FDMSGON:
+	case FDMSGOFF:
+	case FDSETEMSGTRESH:
+	case FDFLUSH:
+	case FDWERRORCLR:
+	case FDSETMAXERRS:
+	case FDGETMAXERRS:
+	case FDGETDRVTYP:
+	case FDEJECT:
+	case FDCLRPRM:
+	case FDFMTBEG:
+	case FDFMTEND:
+	case FDRESET:
+	case FDTWADDLE:
+	case FDFMTTRK:
+	case FDRAWCMD:
+	/* 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:
+	/* 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. */
+	case HDIO_SET_MULTCOUNT:
+	case HDIO_SET_UNMASKINTR:
+	case HDIO_SET_KEEPSETTINGS:
+	case HDIO_SET_32BIT:
+	case HDIO_SET_NOWERR:
+	case HDIO_SET_DMA:
+	case HDIO_SET_PIO_MODE:
+	case HDIO_SET_NICE:
+	case HDIO_SET_WCACHE:
+	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 */
+		return -ENOIOCTLCMD;
+	}
+
+	if (disk->fops->unlocked_ioctl)
+		return disk->fops->unlocked_ioctl(file, cmd, arg);
+
+	if (disk->fops->ioctl) {
+		lock_kernel();
+		ret = disk->fops->ioctl(inode, file, cmd, arg);
+		unlock_kernel();
+		return ret;
+	}
+
+	return -ENOTTY;
+}
+
 static int compat_blkdev_locked_ioctl(struct inode *inode, struct file *file,
 				struct block_device *bdev,
 				unsigned cmd, unsigned long arg)
@@ -119,5 +235,8 @@ long compat_blkdev_ioctl(struct file *fi
 		ret = disk->fops->compat_ioctl(file, cmd, arg);
 	unlock_kernel();
 
-	return ret;
+	if (ret != -ENOIOCTLCMD)
+		return ret;
+
+	return compat_blkdev_driver_ioctl(inode, file, disk, cmd, arg);
 }
fs/compat_ioctl.c +0 -83

97 lines not on this page · raw message

#5 lore raw

PATCH 3/9 compat_ioctl: handle blk_trace ioctls

In reply to Arnd Bergmann

blk_trace_setup is broken on x86_64 compat systems, this makes the code work correctly on all 64 bit architectures in compat mode.

block/blktrace.c +34 -20
Index: linux-2.6/block/blktrace.c
===================================================================
@@ -312,33 +312,26 @@ static struct rchan_callbacks blk_relay_
 /*
  * Setup everything required to start tracing
  */
-static int blk_trace_setup(struct request_queue *q, struct block_device *bdev,
-			   char __user *arg)
+int do_blk_trace_setup(struct request_queue *q, struct block_device *bdev,
+			struct blk_user_trace_setup *buts)
 {
-	struct blk_user_trace_setup buts;
 	struct blk_trace *old_bt, *bt = NULL;
 	struct dentry *dir = NULL;
 	char b[BDEVNAME_SIZE];
 	int ret, i;
 
-	if (copy_from_user(&buts, arg, sizeof(buts)))
-		return -EFAULT;
-
-	if (!buts.buf_size || !buts.buf_nr)
+	if (!buts->buf_size || !buts->buf_nr)
 		return -EINVAL;
 
-	strcpy(buts.name, bdevname(bdev, b));
+	strcpy(buts->name, bdevname(bdev, b));
 
 	/*
 	 * some device names have larger paths - convert the slashes
 	 * to underscores for this to work as expected
 	 */
-	for (i = 0; i < strlen(buts.name); i++)
-		if (buts.name[i] == '/')
-			buts.name[i] = '_';
-
-	if (copy_to_user(arg, &buts, sizeof(buts)))
-		return -EFAULT;
+	for (i = 0; i < strlen(buts->name); i++)
+		if (buts->name[i] == '/')
+			buts->name[i] = '_';
 
 	ret = -ENOMEM;
 	bt = kzalloc(sizeof(*bt), GFP_KERNEL);
@@ -350,7 +343,7 @@ static int blk_trace_setup(struct reques
 		goto err;
 
 	ret = -ENOENT;
-	dir = blk_create_tree(buts.name);
+	dir = blk_create_tree(buts->name);
 	if (!dir)
 		goto err;
 
@@ -363,20 +356,21 @@ static int blk_trace_setup(struct reques
 	if (!bt->dropped_file)
 		goto err;
 
-	bt->rchan = relay_open("trace", dir, buts.buf_size, buts.buf_nr, &blk_relay_callbacks, bt);
+	bt->rchan = relay_open("trace", dir, buts->buf_size,
+				buts->buf_nr, &blk_relay_callbacks, bt);
 	if (!bt->rchan)
 		goto err;
 
-	bt->act_mask = buts.act_mask;
+	bt->act_mask = buts->act_mask;
 	if (!bt->act_mask)
 		bt->act_mask = (u16) -1;
 
-	bt->start_lba = buts.start_lba;
-	bt->end_lba = buts.end_lba;
+	bt->start_lba = buts->start_lba;
+	bt->end_lba = buts->end_lba;
 	if (!bt->end_lba)
 		bt->end_lba = -1ULL;
 
-	bt->pid = buts.pid;
+	bt->pid = buts->pid;
 	bt->trace_state = Blktrace_setup;
 
 	ret = -EBUSY;
@@ -401,6 +395,26 @@ err:
 	return ret;
 }
 
+static int blk_trace_setup(struct request_queue *q, struct block_device *bdev,
+			   char __user *arg)
+{
+	struct blk_user_trace_setup buts;
+	int ret;
+
+	ret = copy_from_user(&buts, arg, sizeof(buts));
+	if (ret)
+		return -EFAULT;
+
+	ret = do_blk_trace_setup(q, bdev, &buts);
+	if (ret)
+		return ret;
+
+	if (copy_to_user(arg, &buts, sizeof(buts)))
+		return -EFAULT;
+
+	return 0;
+}
+
 static int blk_trace_startstop(struct request_queue *q, int start)
 {
 	struct blk_trace *bt;
block/compat_ioctl.c +54 -0

66 lines not on this page · raw message

fs/compat_ioctl.c +0 -8
Index: linux-2.6/fs/compat_ioctl.c
===================================================================
@@ -62,7 +62,6 @@
 #include <linux/i2c-dev.h>
 #include <linux/wireless.h>
 #include <linux/atalk.h>
-#include <linux/blktrace_api.h>
 #include <linux/loop.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -2477,13 +2476,6 @@ COMPATIBLE_IOCTL(FIONREAD)  /* This is a
 /* 0x00 */
 COMPATIBLE_IOCTL(FIBMAP)
 COMPATIBLE_IOCTL(FIGETBSZ)
-/* 0x12 */
-#ifdef CONFIG_BLOCK
-COMPATIBLE_IOCTL(BLKTRACESTART)
-COMPATIBLE_IOCTL(BLKTRACESTOP)
-COMPATIBLE_IOCTL(BLKTRACESETUP)
-COMPATIBLE_IOCTL(BLKTRACETEARDOWN)
-#endif
 /* RAID */
 COMPATIBLE_IOCTL(RAID_VERSION)
 COMPATIBLE_IOCTL(GET_ARRAY_INFO)
include/linux/blktrace_api.h +6 -1
Index: linux-2.6/include/linux/blktrace_api.h
===================================================================
@@ -142,10 +142,14 @@ struct blk_user_trace_setup {
 	u32 pid;
 };
 
+#ifdef __KERNEL__
 #if defined(CONFIG_BLK_DEV_IO_TRACE)
 extern int blk_trace_ioctl(struct block_device *, unsigned, char __user *);
 extern void blk_trace_shutdown(struct request_queue *);
 extern void __blk_add_trace(struct blk_trace *, sector_t, int, int, u32, int, int, void *);
+extern int do_blk_trace_setup(struct request_queue *q,
+	struct block_device *bdev, struct blk_user_trace_setup *buts);
+
 
 /**
  * blk_add_trace_rq - Add a trace for a request oriented action
@@ -286,6 +290,7 @@ static inline void blk_add_trace_remap(s
 #define blk_add_trace_generic(q, rq, rw, what)	do { } while (0)
 #define blk_add_trace_pdu_int(q, what, bio, pdu)	do { } while (0)
 #define blk_add_trace_remap(q, bio, dev, f, t)	do {} while (0)
+#define do_blk_trace_setup(q, bdev, buts)	do {} while (0)
 #endif /* CONFIG_BLK_DEV_IO_TRACE */
-
+#endif /* __KERNEL__ */
 #endif
#6 lore raw

PATCH 4/9 compat_ioctl: move hdio calls to block/compat_ioctl.c

In reply to Arnd Bergmann

These are common to multiple block drivers, so they should be handled by the block layer.

block/compat_ioctl.c +70 -0
Index: linux-2.6/block/compat_ioctl.c
===================================================================
@@ -36,6 +36,62 @@ static int compat_put_u64(unsigned long 
 	return put_user(val, (compat_u64 __user *)compat_ptr(arg));
 }
 
+struct compat_hd_geometry {
+	unsigned char heads;
+	unsigned char sectors;
+	unsigned short cylinders;
+	u32 start;
+};
+
+static int compat_hdio_getgeo(struct gendisk *disk, struct block_device *bdev,
+			struct compat_hd_geometry __user *ugeo)
+{
+	struct hd_geometry geo;
+	int ret;
+
+	if (!ugeo)
+		return -EINVAL;
+	if (!disk->fops->getgeo)
+		return -ENOTTY;
+
+	/*
+	 * We need to set the startsect first, the driver may
+	 * want to override it.
+	 */
+	geo.start = get_start_sect(bdev);
+	ret = disk->fops->getgeo(bdev, &geo);
+	if (ret)
+		return ret;
+
+	ret = copy_to_user(ugeo, &geo, 4);
+	ret |= __put_user(geo.start, &ugeo->start);
+	if (ret)
+		ret = -EFAULT;
+
+	return ret;
+}
+
+static int compat_hdio_ioctl(struct inode *inode, struct file *file,
+		struct gendisk *disk, unsigned int cmd, unsigned long arg)
+{
+	mm_segment_t old_fs = get_fs();
+	unsigned long kval;
+	unsigned int __user *uvp;
+	int error;
+
+	set_fs(KERNEL_DS);
+	error = blkdev_driver_ioctl(inode, file, disk,
+				cmd, (unsigned long)(&kval));
+	set_fs(old_fs);
+
+	if (error == 0) {
+		uvp = compat_ptr(arg);
+		if (put_user(kval, uvp))
+			error = -EFAULT;
+	}
+	return error;
+}
+
 #define BLKBSZGET_32		_IOR(0x12, 112, int)
 #define BLKBSZSET_32		_IOW(0x12, 113, int)
 #define BLKGETSIZE64_32		_IOR(0x12, 114, int)
@@ -93,6 +149,18 @@ static int compat_blkdev_driver_ioctl(st
 	int ret;
 
 	switch (arg) {
+	case HDIO_GET_UNMASKINTR:
+	case HDIO_GET_MULTCOUNT:
+	case HDIO_GET_KEEPSETTINGS:
+	case HDIO_GET_32BIT:
+	case HDIO_GET_NOWERR:
+	case HDIO_GET_DMA:
+	case HDIO_GET_NICE:
+	case HDIO_GET_WCACHE:
+	case HDIO_GET_ACOUSTIC:
+	case HDIO_GET_ADDRESS:
+	case HDIO_GET_BUSSTATE:
+		return compat_hdio_ioctl(inode, file, disk, cmd, arg);
 	/*
 	 * No handler required for the ones below, we just need to
 	 * convert arg to a 64 bit pointer.
@@ -268,6 +336,8 @@ long compat_blkdev_ioctl(struct file *fi
 	struct gendisk *disk = bdev->bd_disk;
 
 	switch (cmd) {
+	case HDIO_GETGEO:
+		return compat_hdio_getgeo(disk, bdev, compat_ptr(arg));
 	case BLKFLSBUF:
 	case BLKROSET:
 	/*
fs/compat_ioctl.c +0 -60

79 lines not on this page · raw message

#7 lore raw

PATCH 5/9 compat_ioctl: move BLKPG handling to block/compat_ioctl.c

In reply to Arnd Bergmann

BLKPG is common to all block devices, so it should be handled by common code.

block/compat_ioctl.c +31 -0
Index: linux-2.6/block/compat_ioctl.c
===================================================================
@@ -92,6 +92,35 @@ static int compat_hdio_ioctl(struct inod
 	return error;
 }
 
+struct compat_blkpg_ioctl_arg {
+	compat_int_t op;
+	compat_int_t flags;
+	compat_int_t datalen;
+	compat_caddr_t data;
+};
+
+static int compat_blkpg_ioctl(struct inode *inode, struct file *file,
+		unsigned int cmd, struct compat_blkpg_ioctl_arg __user *ua32)
+{
+	struct blkpg_ioctl_arg __user *a = compat_alloc_user_space(sizeof(*a));
+	compat_caddr_t udata;
+	compat_int_t n;
+	int err;
+
+	err = get_user(n, &ua32->op);
+	err |= put_user(n, &a->op);
+	err |= get_user(n, &ua32->flags);
+	err |= put_user(n, &a->flags);
+	err |= get_user(n, &ua32->datalen);
+	err |= put_user(n, &a->datalen);
+	err |= get_user(udata, &ua32->data);
+	err |= put_user(compat_ptr(udata), &a->data);
+	if (err)
+		return err;
+
+	return blkdev_ioctl(inode, file, cmd, (unsigned long)a);
+}
+
 #define BLKBSZGET_32		_IOR(0x12, 112, int)
 #define BLKBSZSET_32		_IOW(0x12, 113, int)
 #define BLKGETSIZE64_32		_IOR(0x12, 114, int)
@@ -350,6 +379,8 @@ long compat_blkdev_ioctl(struct file *fi
 	case BLKBSZSET_32:
 		return blkdev_ioctl(inode, file, BLKBSZSET,
 				(unsigned long)compat_ptr(arg));
+	case BLKPG:
+		return compat_blkpg_ioctl(inode, file, cmd, compat_ptr(arg));
 	}
 
 	lock_kernel();
fs/compat_ioctl.c +0 -33
Index: linux-2.6/fs/compat_ioctl.c
===================================================================
@@ -47,7 +47,6 @@
 #include <linux/netdevice.h>
 #include <linux/raw.h>
 #include <linux/smb_fs.h>
-#include <linux/blkpg.h>
 #include <linux/blkdev.h>
 #include <linux/elevator.h>
 #include <linux/rtc.h>
@@ -1487,37 +1486,6 @@ ret_einval(unsigned int fd, unsigned int
 	return -EINVAL;
 }
 
-#ifdef CONFIG_BLOCK
-struct blkpg_ioctl_arg32 {
-	compat_int_t op;
-	compat_int_t flags;
-	compat_int_t datalen;
-	compat_caddr_t data;
-};
-
-static int blkpg_ioctl_trans(unsigned int fd, unsigned int cmd, unsigned long arg)
-{
-	struct blkpg_ioctl_arg32 __user *ua32 = compat_ptr(arg);
-	struct blkpg_ioctl_arg __user *a = compat_alloc_user_space(sizeof(*a));
-	compat_caddr_t udata;
-	compat_int_t n;
-	int err;
-	
-	err = get_user(n, &ua32->op);
-	err |= put_user(n, &a->op);
-	err |= get_user(n, &ua32->flags);
-	err |= put_user(n, &a->flags);
-	err |= get_user(n, &ua32->datalen);
-	err |= put_user(n, &a->datalen);
-	err |= get_user(udata, &ua32->data);
-	err |= put_user(compat_ptr(udata), &a->data);
-	if (err)
-		return err;
-
-	return sys_ioctl(fd, cmd, (unsigned long)a);
-}
-#endif
-
 static int ioc_settimeout(unsigned int fd, unsigned int cmd, unsigned long arg)
 {
 	return rw_long(fd, AUTOFS_IOC_SETTIMEOUT, arg);
@@ -3160,7 +3128,6 @@ HANDLE_IOCTL(SIOCGSTAMP, do_siocgstamp)
 HANDLE_IOCTL(SIOCGSTAMPNS, do_siocgstampns)
 #endif
 #ifdef CONFIG_BLOCK
-HANDLE_IOCTL(BLKPG, blkpg_ioctl_trans)
 HANDLE_IOCTL(FDSETPRM32, fd_ioctl_trans)
 HANDLE_IOCTL(FDDEFPRM32, fd_ioctl_trans)
 HANDLE_IOCTL(FDGETPRM32, fd_ioctl_trans)
#8 lore raw

PATCH 6/9 compat_ioctl: move cdrom handlers to block/compat_ioctl.c

In reply to Arnd Bergmann

These are shared by all cd-rom drivers and should have common handlers. Do slight cosmetic cleanups in the process.

block/compat_ioctl.c +83 -0
Index: linux-2.6/block/compat_ioctl.c
===================================================================
@@ -92,6 +92,84 @@ static int compat_hdio_ioctl(struct inod
 	return error;
 }
 
+struct compat_cdrom_read_audio {
+	union cdrom_addr	addr;
+	u8			addr_format;
+	compat_int_t		nframes;
+	compat_caddr_t		buf;
+};
+
+struct compat_cdrom_generic_command {
+	unsigned char	cmd[CDROM_PACKET_SIZE];
+	compat_caddr_t	buffer;
+	compat_uint_t	buflen;
+	compat_int_t	stat;
+	compat_caddr_t	sense;
+	unsigned char	data_direction;
+	compat_int_t	quiet;
+	compat_int_t	timeout;
+	compat_caddr_t	reserved[1];
+};
+
+static int compat_cdrom_read_audio(struct inode *inode, struct file *file,
+		struct gendisk *disk, unsigned int cmd, unsigned long arg)
+{
+	struct cdrom_read_audio __user *cdread_audio;
+	struct compat_cdrom_read_audio __user *cdread_audio32;
+	__u32 data;
+	void __user *datap;
+
+	cdread_audio = compat_alloc_user_space(sizeof(*cdread_audio));
+	cdread_audio32 = compat_ptr(arg);
+
+	if (copy_in_user(&cdread_audio->addr,
+			 &cdread_audio32->addr,
+			 (sizeof(*cdread_audio32) -
+			  sizeof(compat_caddr_t))))
+		return -EFAULT;
+
+	if (get_user(data, &cdread_audio32->buf))
+		return -EFAULT;
+	datap = compat_ptr(data);
+	if (put_user(datap, &cdread_audio->buf))
+		return -EFAULT;
+
+	return blkdev_driver_ioctl(inode, file, disk, cmd,
+			(unsigned long)cdread_audio);
+}
+
+static int compat_cdrom_generic_command(struct inode *inode, struct file *file,
+		struct gendisk *disk, unsigned int cmd, unsigned long arg)
+{
+	struct cdrom_generic_command __user *cgc;
+	struct compat_cdrom_generic_command __user *cgc32;
+	u32 data;
+	unsigned char dir;
+	int itmp;
+
+	cgc = compat_alloc_user_space(sizeof(*cgc));
+	cgc32 = compat_ptr(arg);
+
+	if (copy_in_user(&cgc->cmd, &cgc32->cmd, sizeof(cgc->cmd)) ||
+	    get_user(data, &cgc32->buffer) ||
+	    put_user(compat_ptr(data), &cgc->buffer) ||
+	    copy_in_user(&cgc->buflen, &cgc32->buflen,
+			 (sizeof(unsigned int) + sizeof(int))) ||
+	    get_user(data, &cgc32->sense) ||
+	    put_user(compat_ptr(data), &cgc->sense) ||
+	    get_user(dir, &cgc32->data_direction) ||
+	    put_user(dir, &cgc->data_direction) ||
+	    get_user(itmp, &cgc32->quiet) ||
+	    put_user(itmp, &cgc->quiet) ||
+	    get_user(itmp, &cgc32->timeout) ||
+	    put_user(itmp, &cgc->timeout) ||
+	    get_user(data, &cgc32->reserved[0]) ||
+	    put_user(compat_ptr(data), &cgc->reserved[0]))
+		return -EFAULT;
+
+	return blkdev_driver_ioctl(inode, file, disk, cmd, (unsigned long)cgc);
+}
+
 struct compat_blkpg_ioctl_arg {
 	compat_int_t op;
 	compat_int_t flags;
@@ -190,6 +268,11 @@ static int compat_blkdev_driver_ioctl(st
 	case HDIO_GET_ADDRESS:
 	case HDIO_GET_BUSSTATE:
 		return compat_hdio_ioctl(inode, file, disk, cmd, arg);
+	case CDROMREADAUDIO:
+		return compat_cdrom_read_audio(inode, file, disk, cmd, arg);
+	case CDROM_SEND_PACKET:
+		return compat_cdrom_generic_command(inode, file, disk, cmd, arg);
+
 	/*
 	 * No handler required for the ones below, we just need to
 	 * convert arg to a 64 bit pointer.
fs/compat_ioctl.c +0 -105

123 lines not on this page · raw message

#9 lore raw

PATCH 7/9 compat_ioctl: move floppy handlers to block/compat_ioctl.c

In reply to Arnd Bergmann

The floppy ioctls are used by multiple drivers, so they should be handled in a shared location. Also, add minor cleanups.

block/compat_ioctl.c +336 -0
Index: linux-2.6/block/compat_ioctl.c
===================================================================
@@ -203,6 +203,332 @@ static int compat_blkpg_ioctl(struct ino
 #define BLKBSZSET_32		_IOW(0x12, 113, int)
 #define BLKGETSIZE64_32		_IOR(0x12, 114, int)
 
+struct compat_floppy_struct {
+	compat_uint_t	size;
+	compat_uint_t	sect;
+	compat_uint_t	head;
+	compat_uint_t	track;
+	compat_uint_t	stretch;
+	unsigned char	gap;
+	unsigned char	rate;
+	unsigned char	spec1;
+	unsigned char	fmt_gap;
+	const compat_caddr_t name;
+};
+
+struct compat_floppy_drive_params {
+	char		cmos;
+	compat_ulong_t	max_dtr;
+	compat_ulong_t	hlt;
+	compat_ulong_t	hut;
+	compat_ulong_t	srt;
+	compat_ulong_t	spinup;
+	compat_ulong_t	spindown;
+	unsigned char	spindown_offset;
+	unsigned char	select_delay;
+	unsigned char	rps;
+	unsigned char	tracks;
+	compat_ulong_t	timeout;
+	unsigned char	interleave_sect;
+	struct floppy_max_errors max_errors;
+	char		flags;
+	char		read_track;
+	short		autodetect[8];
+	compat_int_t	checkfreq;
+	compat_int_t	native_format;
+};
+
+struct compat_floppy_drive_struct {
+	signed char	flags;
+	compat_ulong_t	spinup_date;
+	compat_ulong_t	select_date;
+	compat_ulong_t	first_read_date;
+	short		probed_format;
+	short		track;
+	short		maxblock;
+	short		maxtrack;
+	compat_int_t	generation;
+	compat_int_t	keep_data;
+	compat_int_t	fd_ref;
+	compat_int_t	fd_device;
+	compat_int_t	last_checked;
+	compat_caddr_t dmabuf;
+	compat_int_t	bufblocks;
+};
+
+struct compat_floppy_fdc_state {
+	compat_int_t	spec1;
+	compat_int_t	spec2;
+	compat_int_t	dtr;
+	unsigned char	version;
+	unsigned char	dor;
+	compat_ulong_t	address;
+	unsigned int	rawcmd:2;
+	unsigned int	reset:1;
+	unsigned int	need_configure:1;
+	unsigned int	perp_mode:2;
+	unsigned int	has_fifo:1;
+	unsigned int	driver_version;
+	unsigned char	track[4];
+};
+
+struct compat_floppy_write_errors {
+	unsigned int	write_errors;
+	compat_ulong_t	first_error_sector;
+	compat_int_t	first_error_generation;
+	compat_ulong_t	last_error_sector;
+	compat_int_t	last_error_generation;
+	compat_uint_t	badness;
+};
+
+#define FDSETPRM32 _IOW(2, 0x42, struct compat_floppy_struct)
+#define FDDEFPRM32 _IOW(2, 0x43, struct compat_floppy_struct)
+#define FDGETPRM32 _IOR(2, 0x04, struct compat_floppy_struct)
+#define FDSETDRVPRM32 _IOW(2, 0x90, struct compat_floppy_drive_params)
+#define FDGETDRVPRM32 _IOR(2, 0x11, struct compat_floppy_drive_params)
+#define FDGETDRVSTAT32 _IOR(2, 0x12, struct compat_floppy_drive_struct)
+#define FDPOLLDRVSTAT32 _IOR(2, 0x13, struct compat_floppy_drive_struct)
+#define FDGETFDCSTAT32 _IOR(2, 0x15, struct compat_floppy_fdc_state)
+#define FDWERRORGET32  _IOR(2, 0x17, struct compat_floppy_write_errors)
+
+static struct {
+	unsigned int	cmd32;
+	unsigned int	cmd;
+} fd_ioctl_trans_table[] = {
+	{ FDSETPRM32, FDSETPRM },
+	{ FDDEFPRM32, FDDEFPRM },
+	{ FDGETPRM32, FDGETPRM },
+	{ FDSETDRVPRM32, FDSETDRVPRM },
+	{ FDGETDRVPRM32, FDGETDRVPRM },
+	{ FDGETDRVSTAT32, FDGETDRVSTAT },
+	{ FDPOLLDRVSTAT32, FDPOLLDRVSTAT },
+	{ FDGETFDCSTAT32, FDGETFDCSTAT },
+	{ FDWERRORGET32, FDWERRORGET }
+};
+
+#define NR_FD_IOCTL_TRANS ARRAY_SIZE(fd_ioctl_trans_table)
+
+static int compat_fd_ioctl(struct inode *inode, struct file *file,
+		struct gendisk *disk, unsigned int cmd, unsigned long arg)
+{
+	mm_segment_t old_fs = get_fs();
+	void *karg = NULL;
+	unsigned int kcmd = 0;
+	int i, err;
+
+	for (i = 0; i < NR_FD_IOCTL_TRANS; i++)
+		if (cmd == fd_ioctl_trans_table[i].cmd32) {
+			kcmd = fd_ioctl_trans_table[i].cmd;
+			break;
+		}
+	if (!kcmd)
+		return -EINVAL;
+
+	switch (cmd) {
+	case FDSETPRM32:
+	case FDDEFPRM32:
+	case FDGETPRM32:
+	{
+		compat_uptr_t name;
+		struct compat_floppy_struct __user *uf;
+		struct floppy_struct *f;
+
+		uf = compat_ptr(arg);
+		f = karg = kmalloc(sizeof(struct floppy_struct), GFP_KERNEL);
+		if (!karg)
+			return -ENOMEM;
+		if (cmd == FDGETPRM32)
+			break;
+		err = __get_user(f->size, &uf->size);
+		err |= __get_user(f->sect, &uf->sect);
+		err |= __get_user(f->head, &uf->head);
+		err |= __get_user(f->track, &uf->track);
+		err |= __get_user(f->stretch, &uf->stretch);
+		err |= __get_user(f->gap, &uf->gap);
+		err |= __get_user(f->rate, &uf->rate);
+		err |= __get_user(f->spec1, &uf->spec1);
+		err |= __get_user(f->fmt_gap, &uf->fmt_gap);
+		err |= __get_user(name, &uf->name);
+		f->name = compat_ptr(name);
+		if (err) {
+			err = -EFAULT;
+			goto out;
+		}
+		break;
+	}
+	case FDSETDRVPRM32:
+	case FDGETDRVPRM32:
+	{
+		struct compat_floppy_drive_params __user *uf;
+		struct floppy_drive_params *f;
+
+		uf = compat_ptr(arg);
+		f = karg = kmalloc(sizeof(struct floppy_drive_params), GFP_KERNEL);
+		if (!karg)
+			return -ENOMEM;
+		if (cmd == FDGETDRVPRM32)
+			break;
+		err = __get_user(f->cmos, &uf->cmos);
+		err |= __get_user(f->max_dtr, &uf->max_dtr);
+		err |= __get_user(f->hlt, &uf->hlt);
+		err |= __get_user(f->hut, &uf->hut);
+		err |= __get_user(f->srt, &uf->srt);
+		err |= __get_user(f->spinup, &uf->spinup);
+		err |= __get_user(f->spindown, &uf->spindown);
+		err |= __get_user(f->spindown_offset, &uf->spindown_offset);
+		err |= __get_user(f->select_delay, &uf->select_delay);
+		err |= __get_user(f->rps, &uf->rps);
+		err |= __get_user(f->tracks, &uf->tracks);
+		err |= __get_user(f->timeout, &uf->timeout);
+		err |= __get_user(f->interleave_sect, &uf->interleave_sect);
+		err |= __copy_from_user(&f->max_errors, &uf->max_errors, sizeof(f->max_errors));
+		err |= __get_user(f->flags, &uf->flags);
+		err |= __get_user(f->read_track, &uf->read_track);
+		err |= __copy_from_user(f->autodetect, uf->autodetect, sizeof(f->autodetect));
+		err |= __get_user(f->checkfreq, &uf->checkfreq);
+		err |= __get_user(f->native_format, &uf->native_format);
+		if (err) {
+			err = -EFAULT;
+			goto out;
+		}
+		break;
+	}
+	case FDGETDRVSTAT32:
+	case FDPOLLDRVSTAT32:
+		karg = kmalloc(sizeof(struct floppy_drive_struct), GFP_KERNEL);
+		if (!karg)
+			return -ENOMEM;
+		break;
+	case FDGETFDCSTAT32:
+		karg = kmalloc(sizeof(struct floppy_fdc_state), GFP_KERNEL);
+		if (!karg)
+			return -ENOMEM;
+		break;
+	case FDWERRORGET32:
+		karg = kmalloc(sizeof(struct floppy_write_errors), GFP_KERNEL);
+		if (!karg)
+			return -ENOMEM;
+		break;
+	default:
+		return -EINVAL;
+	}
+	set_fs(KERNEL_DS);
+	err = blkdev_driver_ioctl(inode, file, disk, kcmd, (unsigned long)karg);
+	set_fs(old_fs);
+	if (err)
+		goto out;
+	switch (cmd) {
+	case FDGETPRM32:
+	{
+		struct floppy_struct *f = karg;
+		struct compat_floppy_struct __user *uf = compat_ptr(arg);
+
+		err = __put_user(f->size, &uf->size);
+		err |= __put_user(f->sect, &uf->sect);
+		err |= __put_user(f->head, &uf->head);
+		err |= __put_user(f->track, &uf->track);
+		err |= __put_user(f->stretch, &uf->stretch);
+		err |= __put_user(f->gap, &uf->gap);
+		err |= __put_user(f->rate, &uf->rate);
+		err |= __put_user(f->spec1, &uf->spec1);
+		err |= __put_user(f->fmt_gap, &uf->fmt_gap);
+		err |= __put_user((u64)f->name, (compat_caddr_t __user *)&uf->name);
+		break;
+	}
+	case FDGETDRVPRM32:
+	{
+		struct compat_floppy_drive_params __user *uf;
+		struct floppy_drive_params *f = karg;
+
+		uf = compat_ptr(arg);
+		err = __put_user(f->cmos, &uf->cmos);
+		err |= __put_user(f->max_dtr, &uf->max_dtr);
+		err |= __put_user(f->hlt, &uf->hlt);
+		err |= __put_user(f->hut, &uf->hut);
+		err |= __put_user(f->srt, &uf->srt);
+		err |= __put_user(f->spinup, &uf->spinup);
+		err |= __put_user(f->spindown, &uf->spindown);
+		err |= __put_user(f->spindown_offset, &uf->spindown_offset);
+		err |= __put_user(f->select_delay, &uf->select_delay);
+		err |= __put_user(f->rps, &uf->rps);
+		err |= __put_user(f->tracks, &uf->tracks);
+		err |= __put_user(f->timeout, &uf->timeout);
+		err |= __put_user(f->interleave_sect, &uf->interleave_sect);
+		err |= __copy_to_user(&uf->max_errors, &f->max_errors, sizeof(f->max_errors));
+		err |= __put_user(f->flags, &uf->flags);
+		err |= __put_user(f->read_track, &uf->read_track);
+		err |= __copy_to_user(uf->autodetect, f->autodetect, sizeof(f->autodetect));
+		err |= __put_user(f->checkfreq, &uf->checkfreq);
+		err |= __put_user(f->native_format, &uf->native_format);
+		break;
+	}
+	case FDGETDRVSTAT32:
+	case FDPOLLDRVSTAT32:
+	{
+		struct compat_floppy_drive_struct __user *uf;
+		struct floppy_drive_struct *f = karg;
+
+		uf = compat_ptr(arg);
+		err = __put_user(f->flags, &uf->flags);
+		err |= __put_user(f->spinup_date, &uf->spinup_date);
+		err |= __put_user(f->select_date, &uf->select_date);
+		err |= __put_user(f->first_read_date, &uf->first_read_date);
+		err |= __put_user(f->probed_format, &uf->probed_format);
+		err |= __put_user(f->track, &uf->track);
+		err |= __put_user(f->maxblock, &uf->maxblock);
+		err |= __put_user(f->maxtrack, &uf->maxtrack);
+		err |= __put_user(f->generation, &uf->generation);
+		err |= __put_user(f->keep_data, &uf->keep_data);
+		err |= __put_user(f->fd_ref, &uf->fd_ref);
+		err |= __put_user(f->fd_device, &uf->fd_device);
+		err |= __put_user(f->last_checked, &uf->last_checked);
+		err |= __put_user((u64)f->dmabuf, &uf->dmabuf);
+		err |= __put_user((u64)f->bufblocks, &uf->bufblocks);
+		break;
+	}
+	case FDGETFDCSTAT32:
+	{
+		struct compat_floppy_fdc_state __user *uf;
+		struct floppy_fdc_state *f = karg;
+
+		uf = compat_ptr(arg);
+		err = __put_user(f->spec1, &uf->spec1);
+		err |= __put_user(f->spec2, &uf->spec2);
+		err |= __put_user(f->dtr, &uf->dtr);
+		err |= __put_user(f->version, &uf->version);
+		err |= __put_user(f->dor, &uf->dor);
+		err |= __put_user(f->address, &uf->address);
+		err |= __copy_to_user((char __user *)&uf->address + sizeof(uf->address),
+				   (char *)&f->address + sizeof(f->address), sizeof(int));
+		err |= __put_user(f->driver_version, &uf->driver_version);
+		err |= __copy_to_user(uf->track, f->track, sizeof(f->track));
+		break;
+	}
+	case FDWERRORGET32:
+	{
+		struct compat_floppy_write_errors __user *uf;
+		struct floppy_write_errors *f = karg;
+
+		uf = compat_ptr(arg);
+		err = __put_user(f->write_errors, &uf->write_errors);
+		err |= __put_user(f->first_error_sector, &uf->first_error_sector);
+		err |= __put_user(f->first_error_generation, &uf->first_error_generation);
+		err |= __put_user(f->last_error_sector, &uf->last_error_sector);
+		err |= __put_user(f->last_error_generation, &uf->last_error_generation);
+		err |= __put_user(f->badness, &uf->badness);
+		break;
+	}
+	default:
+		break;
+	}
+	if (err)
+		err = -EFAULT;
+
+out:
+	kfree(karg);
+	return err;
+}
+
 struct compat_blk_user_trace_setup {
 	char name[32];
 	u16 act_mask;
@@ -268,6 +594,16 @@ static int compat_blkdev_driver_ioctl(st
 	case HDIO_GET_ADDRESS:
 	case HDIO_GET_BUSSTATE:
 		return compat_hdio_ioctl(inode, file, disk, cmd, arg);
+	case FDSETPRM32:
+	case FDDEFPRM32:
+	case FDGETPRM32:
+	case FDSETDRVPRM32:
+	case FDGETDRVPRM32:
+	case FDGETDRVSTAT32:
+	case FDPOLLDRVSTAT32:
+	case FDGETFDCSTAT32:
+	case FDWERRORGET32:
+		return compat_fd_ioctl(inode, file, disk, cmd, arg);
 	case CDROMREADAUDIO:
 		return compat_cdrom_read_audio(inode, file, disk, cmd, arg);
 	case CDROM_SEND_PACKET:
fs/compat_ioctl.c +0 -337

355 lines not on this page · raw message

#10 lore raw

PATCH 8/9 compat_ioctl: call disk->fops->compat_ioctl without BKL

In reply to Arnd Bergmann

Normally, all compat_ioctl operations are called without the BKL, the block device operations are an exception to this rule.

Make this work the same as the other handlers.

---
It would be good to find out whether it has been using the BKL on purpose or by accident, before this patch gets applied.

block/compat_ioctl.c +2 -2
Index: linux-2.6/block/compat_ioctl.c
===================================================================
@@ -804,10 +804,10 @@ long compat_blkdev_ioctl(struct file *fi
 
 	lock_kernel();
 	ret = compat_blkdev_locked_ioctl(inode, file, bdev, cmd, arg);
-	/* FIXME: why do we assume -> compat_ioctl needs the BKL? */
+	unlock_kernel();
+
 	if (ret == -ENOIOCTLCMD && disk->fops->compat_ioctl)
 		ret = disk->fops->compat_ioctl(file, cmd, arg);
-	unlock_kernel();
 
 	if (ret != -ENOIOCTLCMD)
 		return ret;
#11 lore raw

Re: [patch 8/9] compat_ioctl: call disk->fops->compat_ioctl without BKL

Quoted 10 lines · Arnd Bergmann

Normally, all compat_ioctl operations are called without the BKL, the block device operations are an exception to this rule.

Make this work the same as the other handlers.

---
It would be good to find out whether it has been using the BKL on purpose or by accident, before this patch gets applied.

Leave it alone, for now - I'm rebasing my series on top of your 1--7 and it will both makes ->ioctl() and ->compat_ioctl() called unlocked and switches ->compat_ioctl() to passing bdev instead of struct file (->unlocked_ioctl() simply dies).

#12 lore raw

Re: [patch 8/9] compat_ioctl: call disk->fops->compat_ioctl without BKL

Quoted 9 lines · Al Viro
Quoted 2 lines · Arnd Bergmann

It would be good to find out whether it has been using the BKL on purpose or by accident, before this patch gets applied.

Leave it alone, for now - I'm rebasing my series on top of your 1--7 and it will both makes ->ioctl() and ->compat_ioctl() called unlocked and switches ->compat_ioctl() to passing bdev instead of struct file (->unlocked_ioctl() simply dies).

ok, excellent.

	Arnd <><
#13 lore raw

PATCH 9/9 compat_ioctl: fix compat_fd_ioctl pointer access

In reply to Arnd Bergmann

As found by sparse, a user space pointer is assigned to a kernel data structure while calling other code with set_fs(KERNEL_DS), which could lead to leaking kernel data if that pointer is ever accessed.

I could not find any place in the floppy drivers that actually uses that pointer, but assigning it to an empty string is a safer choice and gets rid of the sparse warning.

block/compat_ioctl.c +1 -1
Index: linux-2.6/block/compat_ioctl.c
===================================================================
@@ -349,7 +349,7 @@ static int compat_fd_ioctl(struct inode 
 		err |= __get_user(f->spec1, &uf->spec1);
 		err |= __get_user(f->fmt_gap, &uf->fmt_gap);
 		err |= __get_user(name, &uf->name);
-		f->name = compat_ptr(name);
+		f->name = "";
 		if (err) {
 			err = -EFAULT;
 			goto out;
#14 lore raw

Re: [patch 9/9] compat_ioctl: fix compat_fd_ioctl pointer access

Quoted 8 lines · Arnd Bergmann

As found by sparse, a user space pointer is assigned to a kernel data structure while calling other code with set_fs(KERNEL_DS), which could lead to leaking kernel data if that pointer is ever accessed.

I could not find any place in the floppy drivers that actually uses that pointer, but assigning it to an empty string is a safer choice and gets rid of the sparse warning.

FWIW, I'd kill kmalloc(), switched to compat_alloc_user_space() and copy_in_user() / get_user()+put_user(). And kill set_fs() around that sys_ioctl()... Separate from the rest of this series, though.

#15 lore raw
In reply to Arnd Bergmann
Quoted 5 lines · Arnd Bergmann

This is my block ioctl series split up into managable chunks. I'm not really sure about the last two of these, I'd prefer to get a second opinion on those.

Please apply once your tests have gone though.

BTW, one note: 0x1260 thing (

       /* The mkswap binary hard codes it to Intel value :-((( */
       case 0x1260:
       case BLKGETSIZE:

) is an ancient piece of BS. It had appeared in 2.1.115-pre3 and AFAICS it's a misunderstanding.

What happens here:
* mkswap(8) used to hardcode BLKGETSIZE as 0x1260 (instead of
_IO(0x12,0x60)). If _IO is undefined, it still does. Several old architectures (including i386) have the right value still equal to 0x1260 due to the way their _IO() is defined.
* 0x1260 does *NOT* work on sparc32, exactly because its _IO()
is different. _IO(0x12,0x60) works.
* sparc64 for some insane reason (I suspect misreading the mkswap
source) tries to support 0x1260 for 32bit tasks. Note that binary that tries to pull that off will _not_ work on native sparc32.

As the matter of fact, all biarch targets old enough to have 32bit counterpart present prior to 1.3.45 (when BLKGETSIZE went _IO(0x12,60)) either have it still equal to 0x1260 on both 32bit and 64bit (i386/amd64) or have it _not_ equal to 0x1260 on either (ppc/ppc64 and sparc/sparc64). In the latter case 0x1260 doesn't work on 32bit, so having it accepted from 32bit tasks on 64bit is insane.

IOW, that stuff is useless and always had been. In the current form it's simply invalid C on e.g. amd64 - you get duplicate case there, which breaks the build.

I'd simply kill that.

#16 lore raw

From: Al Viro <[email protected]>
Date: Mon, 8 Oct 2007 06:12:18 +0100

Quoted 13 lines · Arnd Bergmann
Quoted 5 lines

This is my block ioctl series split up into managable chunks. I'm not really sure about the last two of these, I'd prefer to get a second opinion on those.

Please apply once your tests have gone though.

BTW, one note: 0x1260 thing (

       /* The mkswap binary hard codes it to Intel value :-((( */
       case 0x1260:
       case BLKGETSIZE:

) is an ancient piece of BS. It had appeared in 2.1.115-pre3 and AFAICS it's a misunderstanding.

...

Quoted 5 lines

IOW, that stuff is useless and always had been. In the current form it's simply invalid C on e.g. amd64 - you get duplicate case there, which breaks the build.

I'd simply kill that.

Agreed.

#17 lore raw

PATCH v2 1/9 compat_ioctl: move common block ioctls to compat_blkdev_ioctl

Make compat_blkdev_ioctl and blkdev_ioctl reflect the respective native versions. This is somewhat more efficient and makes it easier to keep the two in sync.

Also get rid of the bogus handling for broken_blkgetsize and the duplicate entry for BLKRASET.

---

Quoted 4 lines · you

From: Al Viro <[email protected]>

Quoted 1 line

I'd simply kill that.

Agreed.

ok.

block/Makefile +1 -0
Index: linux-2.6/block/Makefile
===================================================================
@@ -11,3 +11,4 @@ obj-$(CONFIG_IOSCHED_DEADLINE)	+= deadli
 obj-$(CONFIG_IOSCHED_CFQ)	+= cfq-iosched.o
 
 obj-$(CONFIG_BLK_DEV_IO_TRACE)	+= blktrace.o
+obj-$(CONFIG_COMPAT)		+= compat_ioctl.o
block/compat_ioctl.c new file +121 -0

121 lines not on this page · raw message

block/ioctl.c +4 -17
Index: linux-2.6/block/ioctl.c
===================================================================
@@ -217,6 +217,10 @@ int blkdev_driver_ioctl(struct inode *in
 }
 EXPORT_SYMBOL_GPL(blkdev_driver_ioctl);
 
+/*
+ * always keep this in sync with compat_blkdev_ioctl() and
+ * compat_blkdev_locked_ioctl()
+ */
 int blkdev_ioctl(struct inode *inode, struct file *file, unsigned cmd,
 			unsigned long arg)
 {
@@ -284,21 +288,4 @@ int blkdev_ioctl(struct inode *inode, st
 
 	return blkdev_driver_ioctl(inode, file, disk, cmd, arg);
 }
-
-/* Most of the generic ioctls are handled in the normal fallback path.
-   This assumes the blkdev's low level compat_ioctl always returns
-   ENOIOCTLCMD for unknown ioctls. */
-long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
-{
-	struct block_device *bdev = file->f_path.dentry->d_inode->i_bdev;
-	struct gendisk *disk = bdev->bd_disk;
-	int ret = -ENOIOCTLCMD;
-	if (disk->fops->compat_ioctl) {
-		lock_kernel();
-		ret = disk->fops->compat_ioctl(file, cmd, arg);
-		unlock_kernel();
-	}
-	return ret;
-}
-
 EXPORT_SYMBOL_GPL(blkdev_ioctl);
fs/compat_ioctl.c +0 -45

80 lines not on this page · raw message

#18 lore raw
In reply to Arnd Bergmann
Quoted 5 lines · Arnd Bergmann

This is my block ioctl series split up into managable chunks. I'm not really sure about the last two of these, I'd prefer to get a second opinion on those.

Please apply once your tests have gone though.

Goodness, thanks a lot Arnd! Applied 1-7 (with v2 of patch #1).

Signature · 1 line
Jens Axboe