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 <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
drivers/staging/comedi/comedi_fops.c | 95 +++++++++++++++++-------------------
1 file changed, 45 insertions(+), 50 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index a819e54..bf5265a 100644
--- 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) {
--
1.8.5.2
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
