Skip to content

Commit b8d47d8

Browse files
author
Al Viro
committed
comedi: get rid of compat_alloc_user_space() mess in COMEDI_INSNLIST compat
Signed-off-by: Al Viro <[email protected]>
1 parent aa332e6 commit b8d47d8

File tree

1 file changed

+48
-90
lines changed

1 file changed

+48
-90
lines changed

drivers/staging/comedi/comedi_fops.c

Lines changed: 48 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,34 +1520,19 @@ static int parse_insn(struct comedi_device *dev, struct comedi_insn *insn,
15201520
#define MIN_SAMPLES 16
15211521
#define MAX_SAMPLES 65536
15221522
static int do_insnlist_ioctl(struct comedi_device *dev,
1523-
struct comedi_insnlist __user *arg, void *file)
1523+
struct comedi_insn *insns,
1524+
unsigned int n_insns,
1525+
void *file)
15241526
{
1525-
struct comedi_insnlist insnlist;
1526-
struct comedi_insn *insns = NULL;
15271527
unsigned int *data = NULL;
15281528
unsigned int max_n_data_required = MIN_SAMPLES;
15291529
int i = 0;
15301530
int ret = 0;
15311531

15321532
lockdep_assert_held(&dev->mutex);
1533-
if (copy_from_user(&insnlist, arg, sizeof(insnlist)))
1534-
return -EFAULT;
1535-
1536-
insns = kcalloc(insnlist.n_insns, sizeof(*insns), GFP_KERNEL);
1537-
if (!insns) {
1538-
ret = -ENOMEM;
1539-
goto error;
1540-
}
1541-
1542-
if (copy_from_user(insns, insnlist.insns,
1543-
sizeof(*insns) * insnlist.n_insns)) {
1544-
dev_dbg(dev->class_dev, "copy_from_user failed\n");
1545-
ret = -EFAULT;
1546-
goto error;
1547-
}
15481533

15491534
/* Determine maximum memory needed for all instructions. */
1550-
for (i = 0; i < insnlist.n_insns; ++i) {
1535+
for (i = 0; i < n_insns; ++i) {
15511536
if (insns[i].n > MAX_SAMPLES) {
15521537
dev_dbg(dev->class_dev,
15531538
"number of samples too large\n");
@@ -1565,7 +1550,7 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
15651550
goto error;
15661551
}
15671552

1568-
for (i = 0; i < insnlist.n_insns; ++i) {
1553+
for (i = 0; i < n_insns; ++i) {
15691554
if (insns[i].insn & INSN_MASK_WRITE) {
15701555
if (copy_from_user(data, insns[i].data,
15711556
insns[i].n * sizeof(unsigned int))) {
@@ -1592,7 +1577,6 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
15921577
}
15931578

15941579
error:
1595-
kfree(insns);
15961580
kfree(data);
15971581

15981582
if (ret < 0)
@@ -2236,11 +2220,30 @@ static long comedi_unlocked_ioctl(struct file *file, unsigned int cmd,
22362220
rc = do_cmdtest_ioctl(dev, (struct comedi_cmd __user *)arg,
22372221
file);
22382222
break;
2239-
case COMEDI_INSNLIST:
2240-
rc = do_insnlist_ioctl(dev,
2241-
(struct comedi_insnlist __user *)arg,
2242-
file);
2223+
case COMEDI_INSNLIST: {
2224+
struct comedi_insnlist insnlist;
2225+
struct comedi_insn *insns = NULL;
2226+
2227+
if (copy_from_user(&insnlist, (void __user *)arg,
2228+
sizeof(insnlist))) {
2229+
rc = -EFAULT;
2230+
break;
2231+
}
2232+
insns = kcalloc(insnlist.n_insns, sizeof(*insns), GFP_KERNEL);
2233+
if (!insns) {
2234+
rc = -ENOMEM;
2235+
break;
2236+
}
2237+
if (copy_from_user(insns, insnlist.insns,
2238+
sizeof(*insns) * insnlist.n_insns)) {
2239+
rc = -EFAULT;
2240+
kfree(insns);
2241+
break;
2242+
}
2243+
rc = do_insnlist_ioctl(dev, insns, insnlist.n_insns, file);
2244+
kfree(insns);
22432245
break;
2246+
}
22442247
case COMEDI_INSN: {
22452248
struct comedi_insn insn;
22462249
if (copy_from_user(&insn, (void __user *)arg, sizeof(insn)))
@@ -3095,83 +3098,38 @@ static int get_compat_insn(struct comedi_insn *insn,
30953098
return 0;
30963099
}
30973100

3098-
/* Copy 32-bit insn structure to native insn structure. */
3099-
static int __get_compat_insn(struct comedi_insn __user *insn,
3100-
struct comedi32_insn_struct __user *insn32)
3101-
{
3102-
int err;
3103-
union {
3104-
unsigned int uint;
3105-
compat_uptr_t uptr;
3106-
} temp;
3107-
3108-
/* Copy insn structure. Ignore the unused members. */
3109-
err = 0;
3110-
if (!access_ok(insn32, sizeof(*insn32)) ||
3111-
!access_ok(insn, sizeof(*insn)))
3112-
return -EFAULT;
3113-
3114-
err |= __get_user(temp.uint, &insn32->insn);
3115-
err |= __put_user(temp.uint, &insn->insn);
3116-
err |= __get_user(temp.uint, &insn32->n);
3117-
err |= __put_user(temp.uint, &insn->n);
3118-
err |= __get_user(temp.uptr, &insn32->data);
3119-
err |= __put_user(compat_ptr(temp.uptr), &insn->data);
3120-
err |= __get_user(temp.uint, &insn32->subdev);
3121-
err |= __put_user(temp.uint, &insn->subdev);
3122-
err |= __get_user(temp.uint, &insn32->chanspec);
3123-
err |= __put_user(temp.uint, &insn->chanspec);
3124-
return err ? -EFAULT : 0;
3125-
}
3126-
31273101
/* Handle 32-bit COMEDI_INSNLIST ioctl. */
31283102
static int compat_insnlist(struct file *file, unsigned long arg)
31293103
{
3130-
struct combined_insnlist {
3131-
struct comedi_insnlist insnlist;
3132-
struct comedi_insn insn[1];
3133-
} __user *s;
3134-
struct comedi32_insnlist_struct __user *insnlist32;
3104+
struct comedi_file *cfp = file->private_data;
3105+
struct comedi_device *dev = cfp->dev;
3106+
struct comedi32_insnlist_struct insnlist32;
31353107
struct comedi32_insn_struct __user *insn32;
3136-
compat_uptr_t uptr;
3137-
unsigned int n_insns, n;
3138-
int err, rc;
3139-
3140-
insnlist32 = compat_ptr(arg);
3141-
3142-
/* Get 32-bit insnlist structure. */
3143-
if (!access_ok(insnlist32, sizeof(*insnlist32)))
3144-
return -EFAULT;
3145-
3146-
err = 0;
3147-
err |= __get_user(n_insns, &insnlist32->n_insns);
3148-
err |= __get_user(uptr, &insnlist32->insns);
3149-
insn32 = compat_ptr(uptr);
3150-
if (err)
3151-
return -EFAULT;
3152-
3153-
/* Allocate user memory to copy insnlist and insns into. */
3154-
s = compat_alloc_user_space(offsetof(struct combined_insnlist,
3155-
insn[n_insns]));
3108+
struct comedi_insn *insns;
3109+
unsigned int n;
3110+
int rc;
31563111

3157-
/* Set native insnlist structure. */
3158-
if (!access_ok(&s->insnlist, sizeof(s->insnlist)))
3112+
if (copy_from_user(&insnlist32, compat_ptr(arg), sizeof(insnlist32)))
31593113
return -EFAULT;
31603114

3161-
err |= __put_user(n_insns, &s->insnlist.n_insns);
3162-
err |= __put_user(&s->insn[0], &s->insnlist.insns);
3163-
if (err)
3164-
return -EFAULT;
3115+
insns = kcalloc(insnlist32.n_insns, sizeof(*insns), GFP_KERNEL);
3116+
if (!insns)
3117+
return -ENOMEM;
31653118

31663119
/* Copy insn structures. */
3167-
for (n = 0; n < n_insns; n++) {
3168-
rc = __get_compat_insn(&s->insn[n], &insn32[n]);
3169-
if (rc)
3120+
insn32 = compat_ptr(insnlist32.insns);
3121+
for (n = 0; n < insnlist32.n_insns; n++) {
3122+
rc = get_compat_insn(insns + n, insn32 + n);
3123+
if (rc) {
3124+
kfree(insns);
31703125
return rc;
3126+
}
31713127
}
31723128

3173-
return comedi_unlocked_ioctl(file, COMEDI_INSNLIST,
3174-
(unsigned long)&s->insnlist);
3129+
mutex_lock(&dev->mutex);
3130+
rc = do_insnlist_ioctl(dev, insns, insnlist32.n_insns, file);
3131+
mutex_unlock(&dev->mutex);
3132+
return rc;
31753133
}
31763134

31773135
/* Handle 32-bit COMEDI_INSN ioctl. */

0 commit comments

Comments
 (0)