Skip to content

Commit aa332e6

Browse files
author
Al Viro
committed
comedi: get rid of compat_alloc_user_space() mess in COMEDI_INSN compat
Just take copy_from_user() out of do_insn_ioctl() into the caller and have compat_insn() build a native version and pass it to do_insn_ioctl() directly. One difference from the previous commits is that the helper used to convert 32bit variant to native has two users - compat_insn() and compat_insnlist(). The latter will be converted in next commit; for now we simply split the helper in two variants - "userland 32bit to kernel native" and "userland 32bit to userland native". The latter is renamed old get_compat_insn(); it will be gone in the next commit. Signed-off-by: Al Viro <[email protected]>
1 parent 3881387 commit aa332e6

File tree

1 file changed

+47
-27
lines changed

1 file changed

+47
-27
lines changed

drivers/staging/comedi/comedi_fops.c

Lines changed: 47 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1615,22 +1615,19 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
16151615
* data (for reads) to insn->data pointer
16161616
*/
16171617
static int do_insn_ioctl(struct comedi_device *dev,
1618-
struct comedi_insn __user *arg, void *file)
1618+
struct comedi_insn *insn, void *file)
16191619
{
1620-
struct comedi_insn insn;
16211620
unsigned int *data = NULL;
16221621
unsigned int n_data = MIN_SAMPLES;
16231622
int ret = 0;
16241623

16251624
lockdep_assert_held(&dev->mutex);
1626-
if (copy_from_user(&insn, arg, sizeof(insn)))
1627-
return -EFAULT;
16281625

1629-
n_data = max(n_data, insn.n);
1626+
n_data = max(n_data, insn->n);
16301627

16311628
/* This is where the behavior of insn and insnlist deviate. */
1632-
if (insn.n > MAX_SAMPLES) {
1633-
insn.n = MAX_SAMPLES;
1629+
if (insn->n > MAX_SAMPLES) {
1630+
insn->n = MAX_SAMPLES;
16341631
n_data = MAX_SAMPLES;
16351632
}
16361633

@@ -1640,26 +1637,26 @@ static int do_insn_ioctl(struct comedi_device *dev,
16401637
goto error;
16411638
}
16421639

1643-
if (insn.insn & INSN_MASK_WRITE) {
1640+
if (insn->insn & INSN_MASK_WRITE) {
16441641
if (copy_from_user(data,
1645-
insn.data,
1646-
insn.n * sizeof(unsigned int))) {
1642+
insn->data,
1643+
insn->n * sizeof(unsigned int))) {
16471644
ret = -EFAULT;
16481645
goto error;
16491646
}
16501647
}
1651-
ret = parse_insn(dev, &insn, data, file);
1648+
ret = parse_insn(dev, insn, data, file);
16521649
if (ret < 0)
16531650
goto error;
1654-
if (insn.insn & INSN_MASK_READ) {
1655-
if (copy_to_user(insn.data,
1651+
if (insn->insn & INSN_MASK_READ) {
1652+
if (copy_to_user(insn->data,
16561653
data,
1657-
insn.n * sizeof(unsigned int))) {
1654+
insn->n * sizeof(unsigned int))) {
16581655
ret = -EFAULT;
16591656
goto error;
16601657
}
16611658
}
1662-
ret = insn.n;
1659+
ret = insn->n;
16631660

16641661
error:
16651662
kfree(data);
@@ -2244,10 +2241,14 @@ static long comedi_unlocked_ioctl(struct file *file, unsigned int cmd,
22442241
(struct comedi_insnlist __user *)arg,
22452242
file);
22462243
break;
2247-
case COMEDI_INSN:
2248-
rc = do_insn_ioctl(dev, (struct comedi_insn __user *)arg,
2249-
file);
2244+
case COMEDI_INSN: {
2245+
struct comedi_insn insn;
2246+
if (copy_from_user(&insn, (void __user *)arg, sizeof(insn)))
2247+
rc = -EFAULT;
2248+
else
2249+
rc = do_insn_ioctl(dev, &insn, file);
22502250
break;
2251+
}
22512252
case COMEDI_POLL:
22522253
rc = do_poll_ioctl(dev, arg, file);
22532254
break;
@@ -3077,7 +3078,25 @@ static int compat_cmdtest(struct file *file, unsigned long arg)
30773078
}
30783079

30793080
/* Copy 32-bit insn structure to native insn structure. */
3080-
static int get_compat_insn(struct comedi_insn __user *insn,
3081+
static int get_compat_insn(struct comedi_insn *insn,
3082+
struct comedi32_insn_struct __user *insn32)
3083+
{
3084+
struct comedi32_insn_struct v32;
3085+
3086+
/* Copy insn structure. Ignore the unused members. */
3087+
if (copy_from_user(&v32, insn32, sizeof(v32)))
3088+
return -EFAULT;
3089+
memset(insn, 0, sizeof(*insn));
3090+
insn->insn = v32.insn;
3091+
insn->n = v32.n;
3092+
insn->data = compat_ptr(v32.data);
3093+
insn->subdev = v32.subdev;
3094+
insn->chanspec = v32.chanspec;
3095+
return 0;
3096+
}
3097+
3098+
/* Copy 32-bit insn structure to native insn structure. */
3099+
static int __get_compat_insn(struct comedi_insn __user *insn,
30813100
struct comedi32_insn_struct __user *insn32)
30823101
{
30833102
int err;
@@ -3146,7 +3165,7 @@ static int compat_insnlist(struct file *file, unsigned long arg)
31463165

31473166
/* Copy insn structures. */
31483167
for (n = 0; n < n_insns; n++) {
3149-
rc = get_compat_insn(&s->insn[n], &insn32[n]);
3168+
rc = __get_compat_insn(&s->insn[n], &insn32[n]);
31503169
if (rc)
31513170
return rc;
31523171
}
@@ -3158,18 +3177,19 @@ static int compat_insnlist(struct file *file, unsigned long arg)
31583177
/* Handle 32-bit COMEDI_INSN ioctl. */
31593178
static int compat_insn(struct file *file, unsigned long arg)
31603179
{
3161-
struct comedi_insn __user *insn;
3162-
struct comedi32_insn_struct __user *insn32;
3180+
struct comedi_file *cfp = file->private_data;
3181+
struct comedi_device *dev = cfp->dev;
3182+
struct comedi_insn insn;
31633183
int rc;
31643184

3165-
insn32 = compat_ptr(arg);
3166-
insn = compat_alloc_user_space(sizeof(*insn));
3167-
3168-
rc = get_compat_insn(insn, insn32);
3185+
rc = get_compat_insn(&insn, (void __user *)arg);
31693186
if (rc)
31703187
return rc;
31713188

3172-
return comedi_unlocked_ioctl(file, COMEDI_INSN, (unsigned long)insn);
3189+
mutex_lock(&dev->mutex);
3190+
rc = do_insn_ioctl(dev, &insn, file);
3191+
mutex_unlock(&dev->mutex);
3192+
return rc;
31733193
}
31743194

31753195
/*

0 commit comments

Comments
 (0)