From b8d47d8813055ce38c0d2ad913d5462017e52692 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 25 Apr 2020 20:33:04 -0400 Subject: [PATCH] comedi: get rid of compat_alloc_user_space() mess in COMEDI_INSNLIST compat Signed-off-by: Al Viro --- drivers/staging/comedi/comedi_fops.c | 138 ++++++++++----------------- 1 file changed, 48 insertions(+), 90 deletions(-) diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c index 0e7ba0d3fa03..10ab24019fa5 100644 --- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -1520,34 +1520,19 @@ out: #define MIN_SAMPLES 16 #define MAX_SAMPLES 65536 static int do_insnlist_ioctl(struct comedi_device *dev, - struct comedi_insnlist __user *arg, void *file) + struct comedi_insn *insns, + unsigned int n_insns, + void *file) { - struct comedi_insnlist insnlist; - struct comedi_insn *insns = NULL; unsigned int *data = NULL; unsigned int max_n_data_required = MIN_SAMPLES; int i = 0; int ret = 0; lockdep_assert_held(&dev->mutex); - if (copy_from_user(&insnlist, arg, sizeof(insnlist))) - return -EFAULT; - - insns = kcalloc(insnlist.n_insns, sizeof(*insns), GFP_KERNEL); - if (!insns) { - ret = -ENOMEM; - goto error; - } - - if (copy_from_user(insns, insnlist.insns, - sizeof(*insns) * insnlist.n_insns)) { - dev_dbg(dev->class_dev, "copy_from_user failed\n"); - ret = -EFAULT; - goto error; - } /* Determine maximum memory needed for all instructions. */ - for (i = 0; i < insnlist.n_insns; ++i) { + for (i = 0; i < n_insns; ++i) { if (insns[i].n > MAX_SAMPLES) { dev_dbg(dev->class_dev, "number of samples too large\n"); @@ -1565,7 +1550,7 @@ static int do_insnlist_ioctl(struct comedi_device *dev, goto error; } - for (i = 0; i < insnlist.n_insns; ++i) { + for (i = 0; i < n_insns; ++i) { if (insns[i].insn & INSN_MASK_WRITE) { if (copy_from_user(data, insns[i].data, insns[i].n * sizeof(unsigned int))) { @@ -1592,7 +1577,6 @@ static int do_insnlist_ioctl(struct comedi_device *dev, } error: - kfree(insns); kfree(data); if (ret < 0) @@ -2236,11 +2220,30 @@ static long comedi_unlocked_ioctl(struct file *file, unsigned int cmd, rc = do_cmdtest_ioctl(dev, (struct comedi_cmd __user *)arg, file); break; - case COMEDI_INSNLIST: - rc = do_insnlist_ioctl(dev, - (struct comedi_insnlist __user *)arg, - file); + case COMEDI_INSNLIST: { + struct comedi_insnlist insnlist; + struct comedi_insn *insns = NULL; + + if (copy_from_user(&insnlist, (void __user *)arg, + sizeof(insnlist))) { + rc = -EFAULT; + break; + } + insns = kcalloc(insnlist.n_insns, sizeof(*insns), GFP_KERNEL); + if (!insns) { + rc = -ENOMEM; + break; + } + if (copy_from_user(insns, insnlist.insns, + sizeof(*insns) * insnlist.n_insns)) { + rc = -EFAULT; + kfree(insns); + break; + } + rc = do_insnlist_ioctl(dev, insns, insnlist.n_insns, file); + kfree(insns); break; + } case COMEDI_INSN: { struct comedi_insn insn; if (copy_from_user(&insn, (void __user *)arg, sizeof(insn))) @@ -3095,83 +3098,38 @@ static int get_compat_insn(struct comedi_insn *insn, return 0; } -/* Copy 32-bit insn structure to native insn structure. */ -static int __get_compat_insn(struct comedi_insn __user *insn, - struct comedi32_insn_struct __user *insn32) -{ - int err; - union { - unsigned int uint; - compat_uptr_t uptr; - } temp; - - /* Copy insn structure. Ignore the unused members. */ - err = 0; - if (!access_ok(insn32, sizeof(*insn32)) || - !access_ok(insn, sizeof(*insn))) - return -EFAULT; - - err |= __get_user(temp.uint, &insn32->insn); - err |= __put_user(temp.uint, &insn->insn); - err |= __get_user(temp.uint, &insn32->n); - err |= __put_user(temp.uint, &insn->n); - err |= __get_user(temp.uptr, &insn32->data); - err |= __put_user(compat_ptr(temp.uptr), &insn->data); - err |= __get_user(temp.uint, &insn32->subdev); - err |= __put_user(temp.uint, &insn->subdev); - err |= __get_user(temp.uint, &insn32->chanspec); - err |= __put_user(temp.uint, &insn->chanspec); - return err ? -EFAULT : 0; -} - /* Handle 32-bit COMEDI_INSNLIST ioctl. */ static int compat_insnlist(struct file *file, unsigned long arg) { - struct combined_insnlist { - struct comedi_insnlist insnlist; - struct comedi_insn insn[1]; - } __user *s; - struct comedi32_insnlist_struct __user *insnlist32; + struct comedi_file *cfp = file->private_data; + struct comedi_device *dev = cfp->dev; + struct comedi32_insnlist_struct insnlist32; struct comedi32_insn_struct __user *insn32; - compat_uptr_t uptr; - unsigned int n_insns, n; - int err, rc; + struct comedi_insn *insns; + unsigned int n; + int rc; - insnlist32 = compat_ptr(arg); - - /* Get 32-bit insnlist structure. */ - if (!access_ok(insnlist32, sizeof(*insnlist32))) + if (copy_from_user(&insnlist32, compat_ptr(arg), sizeof(insnlist32))) return -EFAULT; - err = 0; - err |= __get_user(n_insns, &insnlist32->n_insns); - err |= __get_user(uptr, &insnlist32->insns); - insn32 = compat_ptr(uptr); - if (err) - return -EFAULT; - - /* Allocate user memory to copy insnlist and insns into. */ - s = compat_alloc_user_space(offsetof(struct combined_insnlist, - insn[n_insns])); - - /* Set native insnlist structure. */ - if (!access_ok(&s->insnlist, sizeof(s->insnlist))) - return -EFAULT; - - err |= __put_user(n_insns, &s->insnlist.n_insns); - err |= __put_user(&s->insn[0], &s->insnlist.insns); - if (err) - return -EFAULT; + insns = kcalloc(insnlist32.n_insns, sizeof(*insns), GFP_KERNEL); + if (!insns) + return -ENOMEM; /* Copy insn structures. */ - for (n = 0; n < n_insns; n++) { - rc = __get_compat_insn(&s->insn[n], &insn32[n]); - if (rc) + insn32 = compat_ptr(insnlist32.insns); + for (n = 0; n < insnlist32.n_insns; n++) { + rc = get_compat_insn(insns + n, insn32 + n); + if (rc) { + kfree(insns); return rc; + } } - return comedi_unlocked_ioctl(file, COMEDI_INSNLIST, - (unsigned long)&s->insnlist); + mutex_lock(&dev->mutex); + rc = do_insnlist_ioctl(dev, insns, insnlist32.n_insns, file); + mutex_unlock(&dev->mutex); + return rc; } /* Handle 32-bit COMEDI_INSN ioctl. */