lkml.org 
[lkml]   [2020]   [May]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 05/10] comedi: get rid of compat_alloc_user_space() mess in COMEDI_INSN compat
    Date
    From: Al Viro <viro@zeniv.linux.org.uk>

    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 <viro@zeniv.linux.org.uk>
    ---
    drivers/staging/comedi/comedi_fops.c | 73 +++++++++++++++++++++++-------------
    1 file changed, 46 insertions(+), 27 deletions(-)

    diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
    index d96dc85d8a98..ae0067ab5ead 100644
    --- a/drivers/staging/comedi/comedi_fops.c
    +++ b/drivers/staging/comedi/comedi_fops.c
    @@ -1615,22 +1615,19 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
    * data (for reads) to insn->data pointer
    */
    static int do_insn_ioctl(struct comedi_device *dev,
    - struct comedi_insn __user *arg, void *file)
    + struct comedi_insn *insn, void *file)
    {
    - struct comedi_insn insn;
    unsigned int *data = NULL;
    unsigned int n_data = MIN_SAMPLES;
    int ret = 0;

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

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

    /* This is where the behavior of insn and insnlist deviate. */
    - if (insn.n > MAX_SAMPLES) {
    - insn.n = MAX_SAMPLES;
    + if (insn->n > MAX_SAMPLES) {
    + insn->n = MAX_SAMPLES;
    n_data = MAX_SAMPLES;
    }

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

    - if (insn.insn & INSN_MASK_WRITE) {
    + if (insn->insn & INSN_MASK_WRITE) {
    if (copy_from_user(data,
    - insn.data,
    - insn.n * sizeof(unsigned int))) {
    + insn->data,
    + insn->n * sizeof(unsigned int))) {
    ret = -EFAULT;
    goto error;
    }
    }
    - ret = parse_insn(dev, &insn, data, file);
    + ret = parse_insn(dev, insn, data, file);
    if (ret < 0)
    goto error;
    - if (insn.insn & INSN_MASK_READ) {
    - if (copy_to_user(insn.data,
    + if (insn->insn & INSN_MASK_READ) {
    + if (copy_to_user(insn->data,
    data,
    - insn.n * sizeof(unsigned int))) {
    + insn->n * sizeof(unsigned int))) {
    ret = -EFAULT;
    goto error;
    }
    }
    - ret = insn.n;
    + ret = insn->n;

    error:
    kfree(data);
    @@ -2244,10 +2241,13 @@ static long comedi_unlocked_ioctl(struct file *file, unsigned int cmd,
    (struct comedi_insnlist __user *)arg,
    file);
    break;
    - case COMEDI_INSN:
    - rc = do_insn_ioctl(dev, (struct comedi_insn __user *)arg,
    - file);
    + case COMEDI_INSN: {
    + struct comedi_insn insn;
    + if (copy_from_user(&insn, (void __user *)arg, sizeof(insn)))
    + rc = -EFAULT;
    + rc = do_insn_ioctl(dev, &insn, file);
    break;
    + }
    case COMEDI_POLL:
    rc = do_poll_ioctl(dev, arg, file);
    break;
    @@ -3077,7 +3077,25 @@ static int compat_cmdtest(struct file *file, unsigned long arg)
    }

    /* Copy 32-bit insn structure to native insn structure. */
    -static int get_compat_insn(struct comedi_insn __user *insn,
    +static int get_compat_insn(struct comedi_insn *insn,
    + struct comedi32_insn_struct __user *insn32)
    +{
    + struct comedi32_insn_struct v32;
    +
    + /* Copy insn structure. Ignore the unused members. */
    + if (copy_from_user(&v32, insn32, sizeof(v32)))
    + return -EFAULT;
    + memset(insn, 0, sizeof(*insn));
    + insn->insn = v32.insn;
    + insn->n = v32.n;
    + insn->data = compat_ptr(v32.data);
    + insn->subdev = v32.subdev;
    + insn->chanspec = v32.chanspec;
    + 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;
    @@ -3146,7 +3164,7 @@ static int compat_insnlist(struct file *file, unsigned long arg)

    /* Copy insn structures. */
    for (n = 0; n < n_insns; n++) {
    - rc = get_compat_insn(&s->insn[n], &insn32[n]);
    + rc = __get_compat_insn(&s->insn[n], &insn32[n]);
    if (rc)
    return rc;
    }
    @@ -3158,18 +3176,19 @@ static int compat_insnlist(struct file *file, unsigned long arg)
    /* Handle 32-bit COMEDI_INSN ioctl. */
    static int compat_insn(struct file *file, unsigned long arg)
    {
    - struct comedi_insn __user *insn;
    - struct comedi32_insn_struct __user *insn32;
    + struct comedi_file *cfp = file->private_data;
    + struct comedi_device *dev = cfp->dev;
    + struct comedi_insn insn;
    int rc;

    - insn32 = compat_ptr(arg);
    - insn = compat_alloc_user_space(sizeof(*insn));
    -
    - rc = get_compat_insn(insn, insn32);
    + rc = get_compat_insn(&insn, (void __user *)arg);
    if (rc)
    return rc;

    - return comedi_unlocked_ioctl(file, COMEDI_INSN, (unsigned long)insn);
    + mutex_lock(&dev->mutex);
    + rc = do_insn_ioctl(dev, &insn, file);
    + mutex_unlock(&dev->mutex);
    + return rc;
    }

    /*
    --
    2.11.0
    \
     
     \ /
      Last update: 2020-05-29 02:37    [W:4.318 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site