The `struct comedi_cmd` type includes a `chanlist` pointer tagged as
`__user` (and also an unused `data` pointer tagged as `__user`). The
`COMEDI_CMD` and `COMEDI_CMDTEST` ioctls treat their argument as a
pointer to this type, where the `chanlist` pointer point to a channel
list in user space. The same type is used elsewhere in comedi (mostly
in the low-level drivers) but in this case the `chanlist` pointer points
to a channel list in kernel space. This results in various "sparse"
warnings about differing address spaces being used.
We can declare a `struct comedi_kcmd` type identical to `struct
comedi_cmd` except that the `chanlist` (and unused `data`) pointer
points to kernel memory. Prepare for this change by temporarily
defining a macro `comedi_kcmd` that expands to `comedi_cmd` and change
the interface between the comedi core and the low-level drivers to use
`struct comedi_kcmd` instead of `struct comedi_cmd`. While were at it,
rename some variables in `do_cmd_ioctl()` and `do_cmdtest_ioctl()` to
avoid confusing user-space stuff with kernel-space stuff, and copy the
user-space `chanlist` separately from the whole of the `struct
comedi_cmd` to avoid some "sparse" warnings after `struct comedi_kcmd`
has been declared properly later.
Once the low-level drivers have been updated, the `struct comedi_kcmd`
type can be defined for real. Once that is done, a bunch of sparse
warnings involving the `chanllist` pointer in this type will be
eliminated.
Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
---
drivers/staging/comedi/comedi_fops.c | 83 +++++++++++++++++-------------------
drivers/staging/comedi/comedidev.h | 9 +++-
2 files changed, 46 insertions(+), 46 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index 00d8d1f..7d18325 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1133,35 +1133,34 @@ static void comedi_set_subdevice_runflags(struct comedi_subdevice *s,
static int do_cmd_ioctl(struct comedi_device *dev,
struct comedi_cmd __user *cmd, void *file)
{
- struct comedi_cmd user_cmd;
+ struct comedi_kcmd kcmd;
struct comedi_subdevice *s;
struct comedi_async *async;
int ret = 0;
- unsigned int __user *chanlist_saver = NULL;
+ unsigned int __user *uchanlist = NULL;
- if (copy_from_user(&user_cmd, cmd, sizeof(struct comedi_cmd))) {
+ if (copy_from_user(&kcmd, cmd, sizeof(struct comedi_kcmd)) ||
+ __get_user(uchanlist, &cmd->chanlist)) {
DPRINTK("bad cmd address\n");
return -EFAULT;
}
- /* save user's chanlist pointer so it can be restored later */
- chanlist_saver = user_cmd.chanlist;
- if (user_cmd.subdev >= dev->n_subdevices) {
- DPRINTK("%d no such subdevice\n", user_cmd.subdev);
+ if (kcmd.subdev >= dev->n_subdevices) {
+ DPRINTK("%d no such subdevice\n", kcmd.subdev);
return -ENODEV;
}
- s = &dev->subdevices[user_cmd.subdev];
+ s = &dev->subdevices[kcmd.subdev];
async = s->async;
if (s->type == COMEDI_SUBD_UNUSED) {
- DPRINTK("%d not valid subdevice\n", user_cmd.subdev);
+ DPRINTK("%d not valid subdevice\n", kcmd.subdev);
return -EIO;
}
if (!s->do_cmd || !s->do_cmdtest || !s->async) {
DPRINTK("subdevice %i does not support commands\n",
- user_cmd.subdev);
+ kcmd.subdev);
return -EIO;
}
@@ -1179,23 +1178,23 @@ static int do_cmd_ioctl(struct comedi_device *dev,
s->busy = file;
/* make sure channel/gain list isn't too long */
- if (user_cmd.chanlist_len > s->len_chanlist) {
+ if (kcmd.chanlist_len > s->len_chanlist) {
DPRINTK("channel/gain list too long %u > %d\n",
- user_cmd.chanlist_len, s->len_chanlist);
+ kcmd.chanlist_len, s->len_chanlist);
ret = -EINVAL;
goto cleanup;
}
/* make sure channel/gain list isn't too short */
- if (user_cmd.chanlist_len < 1) {
+ if (kcmd.chanlist_len < 1) {
DPRINTK("channel/gain list too short %u < 1\n",
- user_cmd.chanlist_len);
+ kcmd.chanlist_len);
ret = -EINVAL;
goto cleanup;
}
kfree(async->cmd.chanlist);
- async->cmd = user_cmd;
+ async->cmd = kcmd;
async->cmd.data = NULL;
/* load channel/gain list */
async->cmd.chanlist =
@@ -1206,7 +1205,7 @@ static int do_cmd_ioctl(struct comedi_device *dev,
goto cleanup;
}
- if (copy_from_user(async->cmd.chanlist, user_cmd.chanlist,
+ if (copy_from_user(async->cmd.chanlist, uchanlist,
async->cmd.chanlist_len * sizeof(int))) {
DPRINTK("fault reading chanlist\n");
ret = -EFAULT;
@@ -1226,11 +1225,10 @@ static int do_cmd_ioctl(struct comedi_device *dev,
if (async->cmd.flags & TRIG_BOGUS || ret) {
DPRINTK("test returned %d\n", ret);
- user_cmd = async->cmd;
- /* restore chanlist pointer before copying back */
- user_cmd.chanlist = chanlist_saver;
- user_cmd.data = NULL;
- if (copy_to_user(cmd, &user_cmd, sizeof(struct comedi_cmd))) {
+ kcmd = async->cmd;
+ kcmd.data = NULL;
+ if (copy_to_user(cmd, &kcmd, sizeof(struct comedi_cmd)) ||
+ __put_user(uchanlist, &cmd->chanlist)) {
DPRINTK("fault writing cmd\n");
ret = -EFAULT;
goto cleanup;
@@ -1283,77 +1281,74 @@ cleanup:
static int do_cmdtest_ioctl(struct comedi_device *dev,
struct comedi_cmd __user *arg, void *file)
{
- struct comedi_cmd user_cmd;
+ struct comedi_kcmd kcmd;
struct comedi_subdevice *s;
int ret = 0;
unsigned int *chanlist = NULL;
- unsigned int __user *chanlist_saver = NULL;
+ unsigned int __user *uchanlist = NULL;
- if (copy_from_user(&user_cmd, arg, sizeof(struct comedi_cmd))) {
+ if (copy_from_user(&kcmd, arg, sizeof(struct comedi_kcmd)) ||
+ __get_user(uchanlist, &arg->chanlist)) {
DPRINTK("bad cmd address\n");
return -EFAULT;
}
- /* save user's chanlist pointer so it can be restored later */
- chanlist_saver = user_cmd.chanlist;
- if (user_cmd.subdev >= dev->n_subdevices) {
- DPRINTK("%d no such subdevice\n", user_cmd.subdev);
+ if (kcmd.subdev >= dev->n_subdevices) {
+ DPRINTK("%d no such subdevice\n", kcmd.subdev);
return -ENODEV;
}
- s = &dev->subdevices[user_cmd.subdev];
+ s = &dev->subdevices[kcmd.subdev];
if (s->type == COMEDI_SUBD_UNUSED) {
- DPRINTK("%d not valid subdevice\n", user_cmd.subdev);
+ DPRINTK("%d not valid subdevice\n", kcmd.subdev);
return -EIO;
}
if (!s->do_cmd || !s->do_cmdtest) {
DPRINTK("subdevice %i does not support commands\n",
- user_cmd.subdev);
+ kcmd.subdev);
return -EIO;
}
/* make sure channel/gain list isn't too long */
- if (user_cmd.chanlist_len > s->len_chanlist) {
+ if (kcmd.chanlist_len > s->len_chanlist) {
DPRINTK("channel/gain list too long %d > %d\n",
- user_cmd.chanlist_len, s->len_chanlist);
+ kcmd.chanlist_len, s->len_chanlist);
ret = -EINVAL;
goto cleanup;
}
/* load channel/gain list */
- if (user_cmd.chanlist) {
+ if (uchanlist) {
chanlist =
- kmalloc(user_cmd.chanlist_len * sizeof(int), GFP_KERNEL);
+ kmalloc(kcmd.chanlist_len * sizeof(int), GFP_KERNEL);
if (!chanlist) {
DPRINTK("allocation failed\n");
ret = -ENOMEM;
goto cleanup;
}
- if (copy_from_user(chanlist, user_cmd.chanlist,
- user_cmd.chanlist_len * sizeof(int))) {
+ if (copy_from_user(chanlist, uchanlist,
+ kcmd.chanlist_len * sizeof(int))) {
DPRINTK("fault reading chanlist\n");
ret = -EFAULT;
goto cleanup;
}
/* make sure each element in channel/gain list is valid */
- ret = comedi_check_chanlist(s, user_cmd.chanlist_len, chanlist);
+ ret = comedi_check_chanlist(s, kcmd.chanlist_len, chanlist);
if (ret < 0) {
DPRINTK("bad chanlist\n");
goto cleanup;
}
- user_cmd.chanlist = chanlist;
+ kcmd.chanlist = chanlist;
}
- ret = s->do_cmdtest(dev, s, &user_cmd);
+ ret = s->do_cmdtest(dev, s, &kcmd);
- /* restore chanlist pointer before copying back */
- user_cmd.chanlist = chanlist_saver;
-
- if (copy_to_user(arg, &user_cmd, sizeof(struct comedi_cmd))) {
+ if (copy_to_user(arg, &kcmd, sizeof(struct comedi_cmd)) ||
+ __put_user(uchanlist, &arg->chanlist)) {
DPRINTK("bad cmd address\n");
ret = -EFAULT;
goto cleanup;
diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
index cb67a5c..708bba6 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -62,6 +62,11 @@
#define COMEDI_NUM_BOARD_MINORS 0x30
#define COMEDI_FIRST_SUBDEVICE_MINOR COMEDI_NUM_BOARD_MINORS
+/* Define kernel versions of some ioctl user API structures. */
+/* FIXME: use macros for now. Define proper structures once the kernel
+ * level code has been changed to use the kernel versions of the structures. */
+#define comedi_kcmd comedi_cmd
+
struct comedi_subdevice {
struct comedi_device *device;
int type;
@@ -104,7 +109,7 @@ struct comedi_subdevice {
int (*do_cmd) (struct comedi_device *, struct comedi_subdevice *);
int (*do_cmdtest) (struct comedi_device *, struct comedi_subdevice *,
- struct comedi_cmd *);
+ struct comedi_kcmd *);
int (*poll) (struct comedi_device *, struct comedi_subdevice *);
int (*cancel) (struct comedi_device *, struct comedi_subdevice *);
/* int (*do_lock)(struct comedi_device *, struct comedi_subdevice *); */
@@ -168,7 +173,7 @@ struct comedi_async {
unsigned int events; /* events that have occurred */
- struct comedi_cmd cmd;
+ struct comedi_kcmd cmd;
wait_queue_head_t wait_head;
--
1.7.12
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
