lkml.org 
[lkml]   [2014]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/4] Input: uinput: add full absinfo support
    On Thu, Dec 19, 2013 at 08:27:32AM +1000, Peter Hutterer wrote:
    > On Tue, Dec 17, 2013 at 04:48:51PM +0100, David Herrmann wrote:
    > > +
    > > + user_dev2->version = UINPUT_VERSION;
    > > + memcpy(user_dev2->name, user_dev->name, UINPUT_MAX_NAME_SIZE);
    > > + memcpy(&user_dev2->id, &user_dev->id, sizeof(struct input_id));
    >
    > you copy the id bits one-by-one into the input_dev but you memcpy it here.
    > is this intentional?

    That should simply be:

    user_dev2->id = user_dev->id;

    and in othe rplace as well I think.

    >
    > > + user_dev2->ff_effects_max = user_dev->ff_effects_max;
    > > +
    > > + for (i = 0; i < ABS_CNT; ++i) {
    > > + user_dev2->abs[i].value = 0;
    > > + user_dev2->abs[i].maximum = user_dev->absmax[i];
    > > + user_dev2->abs[i].minimum = user_dev->absmin[i];
    > > + user_dev2->abs[i].fuzz = user_dev->absfuzz[i];
    > > + user_dev2->abs[i].flat = user_dev->absflat[i];
    > > + user_dev2->abs[i].resolution = 0;
    > > + }
    > > +
    > > + retval = uinput_setup_device(udev, user_dev2, ABS_CNT);
    > >
    > > - exit:
    > > kfree(user_dev);
    > > - return retval;
    > > + kfree(user_dev2);
    > > +
    > > + return retval ? retval : count;
    > > +}
    > > +
    > > +static int uinput_setup_device2(struct uinput_device *udev,
    > > + const char __user *buffer, size_t count)
    > > +{
    > > + struct uinput_user_dev2 *user_dev2;
    > > + int retval;
    > > + size_t off, abscnt, max;
    > > +
    > > + /* The first revision of "uinput_user_dev2" is bigger than
    > > + * "uinput_user_dev" and growing. Disallow any smaller payloads. */
    > > + if (count <= sizeof(struct uinput_user_dev))
    > > + return -EINVAL;
    > > +
    > > + /* rough check to avoid huge kernel space allocations */
    > > + max = ABS_CNT * sizeof(*user_dev2->abs) + sizeof(*user_dev2);
    > > + if (count > max)
    > > + return -EINVAL;
    > > +
    > > + user_dev2 = memdup_user(buffer, count);
    > > + if (IS_ERR(user_dev2))
    > > + return PTR_ERR(user_dev2);
    > > +
    > > + if (user_dev2->version > UINPUT_VERSION) {
    > > + retval = -EINVAL;
    > > + } else {
    > > + off = offsetof(struct uinput_user_dev2, abs);
    > > + abscnt = (count - off) / sizeof(*user_dev2->abs);
    > > + retval = uinput_setup_device(udev, user_dev2, abscnt);
    > > + }
    > > +
    >
    > I really wish uinput would be a bit easier to debug than just returning
    > -EINVAL when it's not happy. having said that, the only errno that would
    > remotely make sense is -ERANGE for count > max and even that is a bit meh.

    Maybe we should add a few dev_dbg() and rely on having dynamic debug to
    activate logging on demand?

    Thanks.

    --
    Dmitry


    \
     
     \ /
      Last update: 2014-01-12 21:21    [W:6.767 / U:0.608 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site