Hi
On Mon, Jun 23, 2014 at 4:53 AM, Peter Hutterer
<peter.hutterer@xxxxxxxxx> wrote:
> On Fri, Jun 20, 2014 at 01:26:40PM +0200, David Herrmann wrote:
>> This adds a new ioctl UINPUT_DEV_SETUP that replaces the old device setup
>> method (by write()'ing "struct uinput_user_dev" to the node). The old
>> method is not easily extendable and requires huge payloads. Furthermore,
>> overloading write() without properly versioned objects is error-prone.
>>
>> Therefore, we introduce a new ioctl to replace the old method. The ioctl
>> supports all features of the old method, plus a "resolution" field for
>> absinfo. Furthermore, it's properly forward-compatible to new ABS codes
>> and a growing "struct input_absinfo" structure.
>>
>> The ioctl also allows user-space to skip unknown axes if not set. The
>> payload-size can now be specified by the caller. There is no need to copy
>> the whole array temporarily into the kernel, but instead we can iterate
>> over it and copy each value manually.
>>
>> Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx>
>> ---
>> v2:
>> - drop "version" field
>> - drop "size" field
>> - only copy absinfo if UI_SET_ABSBIT was called for the axis
>> - minor linguistic changes
>>
>> drivers/input/misc/uinput.c | 67 +++++++++++++++++++++++++++++++++++++++++++--
>> include/uapi/linux/uinput.h | 55 +++++++++++++++++++++++++++++++++++--
>> 2 files changed, 116 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
>> index a2a3895..5c125e8 100644
>> --- a/drivers/input/misc/uinput.c
>> +++ b/drivers/input/misc/uinput.c
>> @@ -371,8 +371,65 @@ static int uinput_allocate_device(struct uinput_device *udev)
>> return 0;
>> }
>>
>> -static int uinput_setup_device(struct uinput_device *udev,
>> - const char __user *buffer, size_t count)
>> +static int uinput_dev_setup(struct uinput_device *udev,
>> + struct uinput_setup __user *arg)
>> +{
>> + struct uinput_setup setup;
>> + struct input_dev *dev;
>> + int i, retval;
>
> don't you want to a check for udev->state != UIST_CREATED here?
Nice catch, added!
>> +
>> + if (copy_from_user(&setup, arg, sizeof(setup)))
>> + return -EFAULT;
>> + if (!setup.name[0])
>> + return -EINVAL;
>> + /* So far we only support the original "struct input_absinfo", but be
>> + * forward compatible and allow larger payloads. */
>> + if (setup.absinfo_size < sizeof(struct input_absinfo))
>> + return -EINVAL;
>> +
>> + dev = udev->dev;
>> + dev->id = setup.id;
>> + udev->ff_effects_max = setup.ff_effects_max;
>> +
>> + kfree(dev->name);
>> + dev->name = kstrndup(setup.name, UINPUT_MAX_NAME_SIZE, GFP_KERNEL);
>> + if (!dev->name)
>> + return -ENOMEM;
>> +
>> + if (setup.abs_cnt > ABS_CNT)
>> + setup.abs_cnt = ABS_CNT;
>> +
>> + if (setup.abs_cnt > 0) {
>> + u8 __user *p = (u8 __user*)arg->abs;
>> +
>> + input_alloc_absinfo(dev);
>> + if (!dev->absinfo)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < setup.abs_cnt; ++i, p += setup.absinfo_size) {
>> + struct input_absinfo absinfo;
>> +
>> + if (!test_bit(i, dev->absbit))
>> + continue;
>> +
>> + if (copy_from_user(&absinfo, p, sizeof(absinfo)))
>> + return -EFAULT;
>> +
>> + dev->absinfo[i] = absinfo;
>> + }
>> + }
>> +
>> + retval = uinput_validate_absbits(dev);
>> + if (retval < 0)
>> + return retval;
>> +
>> + udev->state = UIST_SETUP_COMPLETE;
>> + return 0;
>> +}
>> +
>> +/* legacy setup via write() */
>> +static int uinput_setup_device_legacy(struct uinput_device *udev,
>> + const char __user *buffer, size_t count)
>> {
>> struct uinput_user_dev *user_dev;
>> struct input_dev *dev;
>> @@ -475,7 +532,7 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer,
>>
>> retval = udev->state == UIST_CREATED ?
>> uinput_inject_events(udev, buffer, count) :
>> - uinput_setup_device(udev, buffer, count);
>> + uinput_setup_device_legacy(udev, buffer, count);
>>
>> mutex_unlock(&udev->mutex);
>>
>> @@ -736,6 +793,10 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
>> uinput_destroy_device(udev);
>> goto out;
>>
>> + case UI_DEV_SETUP:
>> + retval = uinput_dev_setup(udev, p);
>> + goto out;
>> +
>> case UI_SET_EVBIT:
>> retval = uinput_set_bit(arg, evbit, EV_MAX);
>> goto out;
>> diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
>> index 19339cf..982144d 100644
>> --- a/include/uapi/linux/uinput.h
>> +++ b/include/uapi/linux/uinput.h
>> @@ -20,6 +20,8 @@
>> * Author: Aristeu Sergio Rozanski Filho <aris@xxxxxxxxxxxxxxxxx>
>> *
>> * Changes/Revisions:
>> + * 5 06/20/2014 (David Herrmann <dh.herrmann@xxxxxxxxx>
>> + * - add UI_DEV_SETUP ioctl
>> * 0.4 01/09/2014 (Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>)
>> * - add UI_GET_SYSNAME ioctl
>> * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
>
> hm, I think you misunderstood my comment. I was hoping to fix all up in one
> go, I had noticed that the others were 0.x as well :)
>
> With the extra check, both patches Reviewed-by: Peter Hutterer <peter.hutterer@xxxxxxxxx>
Not sure I like doing that in this patch. I think I will keep 0.5 and
send a followup to fix all of these.
Thanks for reviewing!
David
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
