staging: comedi: comedi_fops: introduce __comedi_get_user_cmd()

The COMEDI_CMD and COMEDI_CMDTEST ioctl functions both copy the
comedi_cmd passed by the user from __user memory space to kernel
memory space. They then do some basic sanity checking of the cmd
before the subdevice (*do_cmdtest) and (*do_cmd) operations are
called.

Introduce a helper function to handle the copy_from_user() and
do the basic sanity checking.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index a819e543ed30292dc5fba66e505c25976126afd1..bf5265a083f60bf933b8eda3b5a456e1532783b0 100644 (file)
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1416,41 +1416,65 @@ error:
        return ret;
 }
 
-static int do_cmd_ioctl(struct comedi_device *dev,
-                       struct comedi_cmd __user *arg, void *file)
+static int __comedi_get_user_cmd(struct comedi_device *dev,
+                                struct comedi_cmd __user *arg,
+                                struct comedi_cmd *cmd)
 {
-       struct comedi_cmd cmd;
        struct comedi_subdevice *s;
-       struct comedi_async *async;
-       int ret = 0;
-       unsigned int __user *user_chanlist;
 
-       if (copy_from_user(&cmd, arg, sizeof(cmd))) {
+       if (copy_from_user(cmd, arg, sizeof(*cmd))) {
                dev_dbg(dev->class_dev, "bad cmd address\n");
                return -EFAULT;
        }
-       /* save user's chanlist pointer so it can be restored later */
-       user_chanlist = (unsigned int __user *)cmd.chanlist;
 
-       if (cmd.subdev >= dev->n_subdevices) {
-               dev_dbg(dev->class_dev, "%d no such subdevice\n", cmd.subdev);
+       if (cmd->subdev >= dev->n_subdevices) {
+               dev_dbg(dev->class_dev, "%d no such subdevice\n", cmd->subdev);
                return -ENODEV;
        }
 
-       s = &dev->subdevices[cmd.subdev];
-       async = s->async;
+       s = &dev->subdevices[cmd->subdev];
 
        if (s->type == COMEDI_SUBD_UNUSED) {
-               dev_dbg(dev->class_dev, "%d not valid subdevice\n", cmd.subdev);
+               dev_dbg(dev->class_dev, "%d not valid subdevice\n", cmd->subdev);
                return -EIO;
        }
 
        if (!s->do_cmd || !s->do_cmdtest || !s->async) {
                dev_dbg(dev->class_dev,
-                       "subdevice %i does not support commands\n", cmd.subdev);
+                       "subdevice %d does not support commands\n", cmd->subdev);
                return -EIO;
        }
 
+       /* make sure channel/gain list isn't too long */
+       if (cmd->chanlist_len > s->len_chanlist) {
+               dev_dbg(dev->class_dev, "channel/gain list too long %d > %d\n",
+                       cmd->chanlist_len, s->len_chanlist);
+               return -EINVAL;
+       }
+
+       return 0;
+}
+
+static int do_cmd_ioctl(struct comedi_device *dev,
+                       struct comedi_cmd __user *arg, void *file)
+{
+       struct comedi_cmd cmd;
+       struct comedi_subdevice *s;
+       struct comedi_async *async;
+       unsigned int __user *user_chanlist;
+       int ret;
+
+       /* get the user's cmd and do some simple validation */
+       ret = __comedi_get_user_cmd(dev, arg, &cmd);
+       if (ret)
+               return ret;
+
+       /* save user's chanlist pointer so it can be restored later */
+       user_chanlist = (unsigned int __user *)cmd.chanlist;
+
+       s = &dev->subdevices[cmd.subdev];
+       async = s->async;
+
        /* are we locked? (ioctl lock) */
        if (s->lock && s->lock != file) {
                dev_dbg(dev->class_dev, "subdevice locked\n");
@@ -1463,13 +1487,6 @@ static int do_cmd_ioctl(struct comedi_device *dev,
                return -EBUSY;
        }
 
-       /* make sure channel/gain list isn't too long */
-       if (cmd.chanlist_len > s->len_chanlist) {
-               dev_dbg(dev->class_dev, "channel/gain list too long %u > %d\n",
-                       cmd.chanlist_len, s->len_chanlist);
-               return -EINVAL;
-       }
-
        /* make sure channel/gain list isn't too short */
        if (cmd.chanlist_len < 1) {
                dev_dbg(dev->class_dev, "channel/gain list too short %u < 1\n",
@@ -1566,41 +1583,19 @@ static int do_cmdtest_ioctl(struct comedi_device *dev,
 {
        struct comedi_cmd cmd;
        struct comedi_subdevice *s;
-       int ret = 0;
        unsigned int *chanlist = NULL;
        unsigned int __user *user_chanlist;
+       int ret;
+
+       /* get the user's cmd and do some simple validation */
+       ret = __comedi_get_user_cmd(dev, arg, &cmd);
+       if (ret)
+               return ret;
 
-       if (copy_from_user(&cmd, arg, sizeof(cmd))) {
-               dev_dbg(dev->class_dev, "bad cmd address\n");
-               return -EFAULT;
-       }
        /* save user's chanlist pointer so it can be restored later */
        user_chanlist = (unsigned int __user *)cmd.chanlist;
 
-       if (cmd.subdev >= dev->n_subdevices) {
-               dev_dbg(dev->class_dev, "%d no such subdevice\n", cmd.subdev);
-               return -ENODEV;
-       }
-
        s = &dev->subdevices[cmd.subdev];
-       if (s->type == COMEDI_SUBD_UNUSED) {
-               dev_dbg(dev->class_dev, "%d not valid subdevice\n", cmd.subdev);
-               return -EIO;
-       }
-
-       if (!s->do_cmd || !s->do_cmdtest) {
-               dev_dbg(dev->class_dev,
-                       "subdevice %i does not support commands\n", cmd.subdev);
-               return -EIO;
-       }
-
-       /* make sure channel/gain list isn't too long */
-       if (cmd.chanlist_len > s->len_chanlist) {
-               dev_dbg(dev->class_dev, "channel/gain list too long %d > %d\n",
-                       cmd.chanlist_len, s->len_chanlist);
-               ret = -EINVAL;
-               goto cleanup;
-       }
 
        /* load channel/gain list */
        if (cmd.chanlist) {