staging: comedi: comedi_fops: introduce __comedi_get_user_chanlist()

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

Introduce a helper function to handle the memdup_user() and the sanity
checking.

Also, remove the unnecessary dev_dbg() when the memdup_user() or
comedi_check_chanlist() fail.

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 bf5265a083f60bf933b8eda3b5a456e1532783b0..ea6dc36d753b0d5b7344011576a6eeae93735737 100644 (file)
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1455,6 +1455,35 @@ static int __comedi_get_user_cmd(struct comedi_device *dev,
        return 0;
 }
 
+static int __comedi_get_user_chanlist(struct comedi_device *dev,
+                                     struct comedi_subdevice *s,
+                                     unsigned int __user *user_chanlist,
+                                     struct comedi_cmd *cmd)
+{
+       unsigned int *chanlist;
+       int ret;
+
+       /* user_chanlist could be NULL for do_cmdtest ioctls */
+       if (!user_chanlist)
+               return 0;
+
+       chanlist = memdup_user(user_chanlist,
+                              cmd->chanlist_len * sizeof(unsigned int));
+       if (IS_ERR(chanlist))
+               return PTR_ERR(chanlist);
+
+       /* make sure each element in channel/gain list is valid */
+       ret = comedi_check_chanlist(s, cmd->chanlist_len, chanlist);
+       if (ret < 0) {
+               kfree(chanlist);
+               return ret;
+       }
+
+       cmd->chanlist = chanlist;
+
+       return 0;
+}
+
 static int do_cmd_ioctl(struct comedi_device *dev,
                        struct comedi_cmd __user *arg, void *file)
 {
@@ -1496,26 +1525,11 @@ static int do_cmd_ioctl(struct comedi_device *dev,
 
        async->cmd = cmd;
        async->cmd.data = NULL;
-       /* load channel/gain list */
-       async->cmd.chanlist = memdup_user(user_chanlist,
-                                         async->cmd.chanlist_len *
-                                         sizeof(int));
-       if (IS_ERR(async->cmd.chanlist)) {
-               ret = PTR_ERR(async->cmd.chanlist);
-               async->cmd.chanlist = NULL;
-               dev_dbg(dev->class_dev, "memdup_user failed with code %d\n",
-                       ret);
-               goto cleanup;
-       }
 
-       /* make sure each element in channel/gain list is valid */
-       ret = comedi_check_chanlist(s,
-                                   async->cmd.chanlist_len,
-                                   async->cmd.chanlist);
-       if (ret < 0) {
-               dev_dbg(dev->class_dev, "bad chanlist\n");
+       /* load channel/gain list */
+       ret = __comedi_get_user_chanlist(dev, s, user_chanlist, &async->cmd);
+       if (ret)
                goto cleanup;
-       }
 
        ret = s->do_cmdtest(dev, s, &async->cmd);
 
@@ -1598,26 +1612,9 @@ static int do_cmdtest_ioctl(struct comedi_device *dev,
        s = &dev->subdevices[cmd.subdev];
 
        /* load channel/gain list */
-       if (cmd.chanlist) {
-               chanlist = memdup_user(user_chanlist,
-                                      cmd.chanlist_len * sizeof(int));
-               if (IS_ERR(chanlist)) {
-                       ret = PTR_ERR(chanlist);
-                       chanlist = NULL;
-                       dev_dbg(dev->class_dev,
-                               "memdup_user exited with code %d", ret);
-                       goto cleanup;
-               }
-
-               /* make sure each element in channel/gain list is valid */
-               ret = comedi_check_chanlist(s, cmd.chanlist_len, chanlist);
-               if (ret < 0) {
-                       dev_dbg(dev->class_dev, "bad chanlist\n");
-                       goto cleanup;
-               }
-
-               cmd.chanlist = chanlist;
-       }
+       ret = __comedi_get_user_chanlist(dev, s, user_chanlist, &cmd);
+       if (ret)
+               return ret;
 
        ret = s->do_cmdtest(dev, s, &cmd);
 
@@ -1627,9 +1624,8 @@ static int do_cmdtest_ioctl(struct comedi_device *dev,
        if (copy_to_user(arg, &cmd, sizeof(cmd))) {
                dev_dbg(dev->class_dev, "bad cmd address\n");
                ret = -EFAULT;
-               goto cleanup;
        }
-cleanup:
+
        kfree(chanlist);
 
        return ret;