Re: [PATCH 1/4] add compat_ioctl methods to dasd

[Christoph Hellwig <hch@lst.de>]

On Tue, Nov 15, 2005 at 06:24:38PM +0100, Christoph Hellwig wrote:
> > and doesn't work after fixing the compile problem. It's a
> > problem with the bdev->bd_disk->private_data which is NULL at the time
> > the partition detection code calls the BIODASDINFO and HDIO_GETGEO ioctl
> > with ioctl_by_bdev. I don't see an easy way to fix this right now.
> 
> my patch doesn't change anything related to dereferencing those fields.
> 
> I see the problem that you're probably having: ioctl_by_bdev calls
> ->ioctl without ensuring ->open has been called previously.  But I don't
> see why this couldn't have happened previously.

So looking at it again I found a bug:  we return EINVAL on an invalid
ioctl, but the compat layer expects ENOIOCTLCMD so it returns using the
generic compat bits.  Returning ENOIOCTLCMD from unlocked_ioctl is fine
aswell as the ioctl layer turns it back.  The patch below fix this issue
and the missing semicolon after lock_kernel()


Index: linux-2.6/drivers/s390/block/dasd_ioctl.c
===================================================================
--- linux-2.6.orig/drivers/s390/block/dasd_ioctl.c	2005-11-16 00:59:04.000000000 +0100
+++ linux-2.6/drivers/s390/block/dasd_ioctl.c	2005-11-16 01:02:36.000000000 +0100
@@ -86,11 +86,11 @@
 	struct dasd_device *device = bdev->bd_disk->private_data;
 	struct dasd_ioctl *ioctl;
 	const char *dir;
-	int rc = -EINVAL;
+	int rc = -ENOIOCTLCMD;
 
 	if ((_IOC_DIR(no) != _IOC_NONE) && (data == 0)) {
 		PRINT_DEBUG("empty data ptr");
-		return -EINVAL;
+		goto out;
 	}
 	dir = _IOC_DIR (no) == _IOC_NONE ? "0" :
 		_IOC_DIR (no) == _IOC_READ ? "r" :
@@ -100,7 +100,7 @@
 		      "ioctl 0x%08x %s'0x%x'%d(%d) with data %8lx", no,
 		      dir, _IOC_TYPE(no), _IOC_NR(no), _IOC_SIZE(no), data);
 
-	lock_kernel()
+	lock_kernel();
 	/* Search for ioctl no in the ioctl list. */
 	list_for_each_entry(ioctl, &dasd_ioctl_list, list) {
 		if (ioctl->no == no) {
@@ -109,15 +109,16 @@
 				continue;
 			rc = ioctl->handler(bdev, no, data);
 			module_put(ioctl->owner);
-			goto out;
+			break;
 		}
 	}
 	/* No ioctl with number no. */
 	DBF_DEV_EVENT(DBF_INFO, device,
 		      "unknown ioctl 0x%08x=%s'0x%x'%d(%d) data %8lx", no,
 		      dir, _IOC_TYPE(no), _IOC_NR(no), _IOC_SIZE(no), data);
- out:
+ out_unlock:
 	unlock_kernel();
+ out:
 	return rc;
 }