lkml.org 
[lkml]   [2011]   [Feb]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] uinput strnlen bugfix
    On Mon, Feb 07, 2011 at 11:38:48PM -0800, Dmitry Torokhov wrote:
    > Input: uinput - use memdup_user() and friends
    >
    > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
    >
    > Instead of open-coding copying of data structures from userspace use
    > memdup_user() and strndup_user(). Note that this introduces change in
    > behavior because driver used to truncate 'phys' longer than 1024 bytes,
    > but now it will refuse to set 'phys' that long. Arguably trying to set
    > such 'phys' is suspect anyways.
    >
    > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
    > ---
    >
    > drivers/input/misc/uinput.c | 35 ++++++++++-------------------------
    > 1 files changed, 10 insertions(+), 25 deletions(-)
    >
    >
    > diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
    > index c0888e3..7f8331f 100644
    > --- a/drivers/input/misc/uinput.c
    > +++ b/drivers/input/misc/uinput.c
    > @@ -361,14 +361,9 @@ static int uinput_setup_device(struct uinput_device *udev, const char __user *bu
    >
    > dev = udev->dev;
    >
    > - user_dev = kmalloc(sizeof(struct uinput_user_dev), GFP_KERNEL);
    > - if (!user_dev)
    > - return -ENOMEM;
    > -
    > - if (copy_from_user(user_dev, buffer, sizeof(struct uinput_user_dev))) {
    > - retval = -EFAULT;
    > - goto exit;
    > - }
    > + user_dev = memdup_user(buffer, sizeof(struct uinput_user_dev));
    > + if (!IS_ERR(user_dev))
    > + return PTR_ERR(user_dev);
    >
    > udev->ff_effects_max = user_dev->ff_effects_max;
    >
    > @@ -621,7 +616,6 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
    > struct uinput_ff_upload ff_up;
    > struct uinput_ff_erase ff_erase;
    > struct uinput_request *req;
    > - int length;
    > char *phys;
    >
    > retval = mutex_lock_interruptible(&udev->mutex);
    > @@ -688,24 +682,15 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
    > retval = -EINVAL;
    > goto out;
    > }
    > - length = strnlen_user(p, 1024);
    > - if (length <= 0) {
    > - retval = -EFAULT;
    > - break;
    > +
    > + phys = strndup_user(p, 1024);
    > + if (IS_ERR(phys)) {
    > + retval = PTR_ERR(phys);
    > + goto out;
    > }
    > +
    > kfree(udev->dev->phys);
    > - udev->dev->phys = phys = kmalloc(length, GFP_KERNEL);
    > - if (!phys) {
    > - retval = -ENOMEM;
    > - break;
    > - }
    > - if (copy_from_user(phys, p, length)) {
    > - udev->dev->phys = NULL;
    > - kfree(phys);
    > - retval = -EFAULT;
    > - break;
    > - }
    > - phys[length - 1] = '\0';
    > + udev->dev->phys = phys;
    > break;
    >
    > case UI_BEGIN_FF_UPLOAD:
    looks good to me
    Acked-by: Aristeu Rozanski <aris@ruivo.org>

    --
    Aristeu



    \
     
     \ /
      Last update: 2011-02-08 21:01    [W:0.024 / U:58.712 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site