vfs: fix up ENOIOCTLCMD error handling

We're doing some odd things there, which already messes up various users
(see the net/socket.c code that this removes), and it was going to add
yet more crud to the block layer because of the incorrect error code
translation.

ENOIOCTLCMD is not an error return that should be returned to user mode
from the "ioctl()" system call, but it should *not* be translated as
EINVAL ("Invalid argument").  It should be translated as ENOTTY
("Inappropriate ioctl for device").

That EINVAL confusion has apparently so permeated some code that the
block layer actually checks for it, which is sad.  We continue to do so
for now, but add a big comment about how wrong that is, and we should
remove it entirely eventually.  In the meantime, this tries to keep the
changes localized to just the EINVAL -> ENOTTY fix, and removing code
that makes it harder to do the right thing.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/block/ioctl.c b/block/ioctl.c
index ca939fc1030f29489f8a5f2c5a142c4e6a6f2264..d510c2a4eff8d42eb02c2da4f0a814c0ad3f6bec 100644 (file)
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -179,6 +179,26 @@ int __blkdev_driver_ioctl(struct block_device *bdev, fmode_t mode,
  */
 EXPORT_SYMBOL_GPL(__blkdev_driver_ioctl);
 
+/*
+ * Is it an unrecognized ioctl? The correct returns are either
+ * ENOTTY (final) or ENOIOCTLCMD ("I don't know this one, try a
+ * fallback"). ENOIOCTLCMD gets turned into ENOTTY by the ioctl
+ * code before returning.
+ *
+ * Confused drivers sometimes return EINVAL, which is wrong. It
+ * means "I understood the ioctl command, but the parameters to
+ * it were wrong".
+ *
+ * We should aim to just fix the broken drivers, the EINVAL case
+ * should go away.
+ */
+static inline int is_unrecognized_ioctl(int ret)
+{
+       return  ret == -EINVAL ||
+               ret == -ENOTTY ||
+               ret == -ENOIOCTLCMD;
+}
+
 /*
  * always keep this in sync with compat_blkdev_ioctl()
  */
@@ -196,8 +216,7 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
                        return -EACCES;
 
                ret = __blkdev_driver_ioctl(bdev, mode, cmd, arg);
-               /* -EINVAL to handle old uncorrected drivers */
-               if (ret != -EINVAL && ret != -ENOTTY)
+               if (!is_unrecognized_ioctl(ret))
                        return ret;
 
                fsync_bdev(bdev);
@@ -206,8 +225,7 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 
        case BLKROSET:
                ret = __blkdev_driver_ioctl(bdev, mode, cmd, arg);
-               /* -EINVAL to handle old uncorrected drivers */
-               if (ret != -EINVAL && ret != -ENOTTY)
+               if (!is_unrecognized_ioctl(ret))
                        return ret;
                if (!capable(CAP_SYS_ADMIN))
                        return -EACCES;

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 51352de88ef10ea41583661f39625f93b475aa34..a10e428b32b4a62dfc22d515d29c423b7e130a01 100644 (file)
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -1506,35 +1506,6 @@ static long do_ioctl_trans(int fd, unsigned int cmd,
        return -ENOIOCTLCMD;
 }
 
-static void compat_ioctl_error(struct file *filp, unsigned int fd,
-               unsigned int cmd, unsigned long arg)
-{
-       char buf[10];
-       char *fn = "?";
-       char *path;
-
-       /* find the name of the device. */
-       path = (char *)__get_free_page(GFP_KERNEL);
-       if (path) {
-               fn = d_path(&filp->f_path, path, PAGE_SIZE);
-               if (IS_ERR(fn))
-                       fn = "?";
-       }
-
-        sprintf(buf,"'%c'", (cmd>>_IOC_TYPESHIFT) & _IOC_TYPEMASK);
-       if (!isprint(buf[1]))
-               sprintf(buf, "%02x", buf[1]);
-       compat_printk("ioctl32(%s:%d): Unknown cmd fd(%d) "
-                       "cmd(%08x){t:%s;sz:%u} arg(%08x) on %s\n",
-                       current->comm, current->pid,
-                       (int)fd, (unsigned int)cmd, buf,
-                       (cmd >> _IOC_SIZESHIFT) & _IOC_SIZEMASK,
-                       (unsigned int)arg, fn);
-
-       if (path)
-               free_page((unsigned long)path);
-}
-
 static int compat_ioctl_check_table(unsigned int xcmd)
 {
        int i;
@@ -1621,13 +1592,8 @@ asmlinkage long compat_sys_ioctl(unsigned int fd, unsigned int cmd,
                goto found_handler;
 
        error = do_ioctl_trans(fd, cmd, arg, filp);
-       if (error == -ENOIOCTLCMD) {
-               static int count;
-
-               if (++count <= 50)
-                       compat_ioctl_error(filp, fd, cmd, arg);
-               error = -EINVAL;
-       }
+       if (error == -ENOIOCTLCMD)
+               error = -ENOTTY;
 
        goto out_fput;
 

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 1d9b9fcb2db48ec682b4e696bbab9dbb79fd26b9..066836e818481fb2240d3cc78062fea3a2cee592 100644 (file)
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -42,7 +42,7 @@ static long vfs_ioctl(struct file *filp, unsigned int cmd,
 
        error = filp->f_op->unlocked_ioctl(filp, cmd, arg);
        if (error == -ENOIOCTLCMD)
-               error = -EINVAL;
+               error = -ENOTTY;
  out:
        return error;
 }

diff --git a/net/socket.c b/net/socket.c
index 2877647f347b06e75aaaa7fd355799a43cd65bf3..a0053750e37a5ee5137ff8b72a88c71ec07a34a5 100644 (file)
--- a/net/socket.c
+++ b/net/socket.c
@@ -2883,7 +2883,7 @@ static int bond_ioctl(struct net *net, unsigned int cmd,
 
                return dev_ioctl(net, cmd, uifr);
        default:
-               return -EINVAL;
+               return -ENOIOCTLCMD;
        }
 }
 
@@ -3210,20 +3210,6 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
                return sock_do_ioctl(net, sock, cmd, arg);
        }
 
-       /* Prevent warning from compat_sys_ioctl, these always
-        * result in -EINVAL in the native case anyway. */
-       switch (cmd) {
-       case SIOCRTMSG:
-       case SIOCGIFCOUNT:
-       case SIOCSRARP:
-       case SIOCGRARP:
-       case SIOCDRARP:
-       case SIOCSIFLINK:
-       case SIOCGIFSLAVE:
-       case SIOCSIFSLAVE:
-               return -EINVAL;
-       }
-
        return -ENOIOCTLCMD;
 }