Message ID | 1440167783-5628-1-git-send-email-benjamin.tissoires@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi On Fri, Aug 21, 2015 at 4:36 PM, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > This adds two new ioctls, UINPUT_DEV_SETUP and UI_ABS_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 two new ioctls to replace the old method. > These ioctls support 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. > > UI_ABS_SETUP also allows user-space to skip unknown axes if not set. > There is no need to copy the whole array temporarily into the kernel, > but instead the caller issues several ioctl where we copy each value > manually. > > Signed-off-by: David Herrmann <dh.herrmann@gmail.com> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > > Hi, > > this is the v2 of the patch David submitted last year. I tried to take > Dmitry's remarks into account and came out with this. > > Cheers, > Benjamin > > drivers/input/misc/uinput.c | 83 +++++++++++++++++++++++++++++++++++++++++++-- > include/linux/uinput.h | 5 +++ > include/uapi/linux/uinput.h | 83 +++++++++++++++++++++++++++++++++++++++++++-- > 3 files changed, 165 insertions(+), 6 deletions(-) > > diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c > index 345df9b..afbcc41 100644 > --- a/drivers/input/misc/uinput.c > +++ b/drivers/input/misc/uinput.c > @@ -370,8 +370,76 @@ 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 retval; > + > + if (udev->state == UIST_CREATED) > + return -EINVAL; > + if (copy_from_user(&setup, arg, sizeof(setup))) > + return -EFAULT; > + if (!setup.name[0]) > + 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; > + > + retval = uinput_validate_absbits(dev); > + if (retval < 0) > + return retval; > + > + udev->state = UIST_SETUP_COMPLETE; > + return 0; > +} > + > +static int uinput_abs_setup(struct uinput_device *udev, > + struct uinput_setup __user *arg, size_t size) > +{ > + struct uinput_abs_setup setup; > + struct input_dev *dev; > + > + memset(&setup, 0, sizeof(setup)); This could be simplified as: struct uinput_abs_setup setup = {}; > + > + if (size > sizeof(setup)) > + return -EINVAL; Maybe use something more specific here. This is something user-space may want to match for if the ioctl struct gets extended. E2BIG? > + if (udev->state == UIST_CREATED) > + return -EINVAL; > + if (copy_from_user(&setup, arg, size)) > + return -EFAULT; > + if (setup.code > ABS_MAX) > + return -ERANGE; > + > + dev = udev->dev; > + > + input_alloc_absinfo(dev); > + if (!dev->absinfo) > + return -ENOMEM; > + > + set_bit(setup.code, dev->absbit); > + If you do a second round of this patch, maybe drop that newline? > + dev->absinfo[setup.code] = setup.absinfo; > + > + /* > + * We restore the state to UIST_NEW_DEVICE because the user has to call > + * UI_DEV_SETUP in the last place before UI_DEV_CREATE to check the > + * validity of the absbits. > + */ > + udev->state = UIST_NEW_DEVICE; > + 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; > @@ -474,7 +542,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); > > @@ -735,6 +803,12 @@ 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; > + > + /* UI_ABS_SETUP is handled in the variable size ioctls */ > + > case UI_SET_EVBIT: > retval = uinput_set_bit(arg, evbit, EV_MAX); > goto out; > @@ -879,6 +953,9 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, > name = dev_name(&udev->dev->dev); > retval = uinput_str_to_user(p, name, size); > goto out; > + case UI_ABS_SETUP & ~IOCSIZE_MASK: > + retval = uinput_abs_setup(udev, p, size); > + goto out; > } > > retval = -EINVAL; > diff --git a/include/linux/uinput.h b/include/linux/uinput.h > index 0994c0d..75de43d 100644 > --- a/include/linux/uinput.h > +++ b/include/linux/uinput.h > @@ -20,6 +20,11 @@ > * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org> > * > * Changes/Revisions: > + * 0.5 08/13/2015 (David Herrmann <dh.herrmann@gmail.com> & > + * Benjamin Tissoires <benjamin.tissoires@redhat.com>) > + * - add UI_DEV_SETUP ioctl > + * - add UI_ABS_SETUP ioctl > + * - add UI_GET_VERSION ioctl > * 0.4 01/09/2014 (Benjamin Tissoires <benjamin.tissoires@redhat.com>) > * - add UI_GET_SYSNAME ioctl > * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) > diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h > index 013c9d8..ef6c9f5 100644 > --- a/include/uapi/linux/uinput.h > +++ b/include/uapi/linux/uinput.h > @@ -20,6 +20,11 @@ > * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org> > * > * Changes/Revisions: > + * 0.5 08/13/2015 (David Herrmann <dh.herrmann@gmail.com> & > + * Benjamin Tissoires <benjamin.tissoires@redhat.com>) > + * - add UI_DEV_SETUP ioctl > + * - add UI_ABS_SETUP ioctl > + * - add UI_GET_VERSION ioctl > * 0.4 01/09/2014 (Benjamin Tissoires <benjamin.tissoires@redhat.com>) > * - add UI_GET_SYSNAME ioctl > * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) > @@ -37,8 +42,8 @@ > #include <linux/types.h> > #include <linux/input.h> > > -#define UINPUT_VERSION 4 > - > +#define UINPUT_VERSION 5 > +#define UINPUT_MAX_NAME_SIZE 80 > > struct uinput_ff_upload { > __u32 request_id; > @@ -58,6 +63,79 @@ struct uinput_ff_erase { > #define UI_DEV_CREATE _IO(UINPUT_IOCTL_BASE, 1) > #define UI_DEV_DESTROY _IO(UINPUT_IOCTL_BASE, 2) > > +struct uinput_setup { > + struct input_id id; > + char name[UINPUT_MAX_NAME_SIZE]; > + __u32 ff_effects_max; > +}; > + > +/** > + * UI_DEV_SETUP - Set device parameters for setup > + * > + * This ioctl sets parameters for the input-device to be created. It must be > + * issued *before* calling UI_DEV_CREATE or it will fail. This ioctl supersedes > + * the old "struct uinput_user_dev" method, which wrote this data via write(). > + * To actually set the absolute axes, you also need to call the ioctl > + * UI_ABS_SETUP *before* calling this ioctl. > + * > + * This ioctl takes a "struct uinput_setup" object as argument. The fields of > + * this object are as follows: > + * id: See the description of "struct input_id". This field is > + * copied unchanged into the new device. > + * name: This is used unchanged as name for the new device. > + * ff_effects_max: This limits the maximum numbers of force-feedback effects. > + * See below for a description of FF with uinput. > + * > + * This ioctl can be called multiple times and will overwrite previous values. > + * If this ioctl fails with -EINVAL, you're recommended to use the old > + * "uinput_user_dev" method via write() as fallback, in case you run on an old > + * kernel that does not support this ioctl. > + * > + * This ioctl may fail with -EINVAL if it is not supported or if you passed > + * incorrect values, -ENOMEM if the kernel runs out of memory or -EFAULT if the > + * passed uinput_setup object cannot be read/written. > + * If this call fails, partial data may have already been applied to the > + * internal device. > + */ > +#define UI_DEV_SETUP _IOW(UINPUT_IOCTL_BASE, 3, struct uinput_setup) > + > +struct uinput_abs_setup { > + __u16 code; /* axis code */ > + /* __u16 filler; */ > + struct input_absinfo absinfo; > +}; > + > +/** > + * UI_ABS_SETUP - Set absolute axis information for the device to setup > + * > + * This ioctl sets one absolute axis information for the input-device to be > + * created. It must be issued *before* calling UI_DEV_SETUP and UI_DEV_CREATE > + * for every absolute axis the device exports. > + * This ioctl supersedes the old "struct uinput_user_dev" method, which wrote > + * part of this data and the content of UI_DEV_SETUP via write(). > + * > + * This ioctl takes a "struct uinput_abs_setup" object as argument. The fields > + * of this object are as follows: > + * code: The corresponding input code associated with this axis > + * (ABS_X, ABS_Y, etc...) > + * absinfo: See "struct input_absinfo" for a description of this field. > + * This field is copied unchanged into the kernel for the > + * specified axis. If the axis is not enabled via > + * UI_SET_ABSBIT, this ioctl will enable it. > + * > + * This ioctl can be called multiple times and will overwrite previous values. > + * If this ioctl fails with -EINVAL, you're recommended to use the old > + * "uinput_user_dev" method via write() as fallback, in case you run on an old > + * kernel that does not support this ioctl. > + * > + * This ioctl may fail with -EINVAL if it is not supported or if you passed > + * incorrect values, -ENOMEM if the kernel runs out of memory or -EFAULT if the > + * passed uinput_setup object cannot be read/written. > + * If this call fails, partial data may have already been applied to the > + * internal device. > + */ > +#define UI_ABS_SETUP _IOW(UINPUT_IOCTL_BASE, 4, struct uinput_abs_setup) > + > #define UI_SET_EVBIT _IOW(UINPUT_IOCTL_BASE, 100, int) > #define UI_SET_KEYBIT _IOW(UINPUT_IOCTL_BASE, 101, int) > #define UI_SET_RELBIT _IOW(UINPUT_IOCTL_BASE, 102, int) > @@ -144,7 +222,6 @@ struct uinput_ff_erase { > #define UI_FF_UPLOAD 1 > #define UI_FF_ERASE 2 > > -#define UINPUT_MAX_NAME_SIZE 80 > struct uinput_user_dev { > char name[UINPUT_MAX_NAME_SIZE]; > struct input_id id; Looks good to me! Thanks for picking it up! Feel free to append: Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Thanks David -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Aug 24 2015 or thereabouts, David Herrmann wrote: > Hi > > On Fri, Aug 21, 2015 at 4:36 PM, Benjamin Tissoires > <benjamin.tissoires@redhat.com> wrote: > > This adds two new ioctls, UINPUT_DEV_SETUP and UI_ABS_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 two new ioctls to replace the old method. > > These ioctls support 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. > > > > UI_ABS_SETUP also allows user-space to skip unknown axes if not set. > > There is no need to copy the whole array temporarily into the kernel, > > but instead the caller issues several ioctl where we copy each value > > manually. > > > > Signed-off-by: David Herrmann <dh.herrmann@gmail.com> > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > --- > > > > Hi, > > > > this is the v2 of the patch David submitted last year. I tried to take > > Dmitry's remarks into account and came out with this. > > > > Cheers, > > Benjamin > > > > drivers/input/misc/uinput.c | 83 +++++++++++++++++++++++++++++++++++++++++++-- > > include/linux/uinput.h | 5 +++ > > include/uapi/linux/uinput.h | 83 +++++++++++++++++++++++++++++++++++++++++++-- > > 3 files changed, 165 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c > > index 345df9b..afbcc41 100644 > > --- a/drivers/input/misc/uinput.c > > +++ b/drivers/input/misc/uinput.c > > @@ -370,8 +370,76 @@ 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 retval; > > + > > + if (udev->state == UIST_CREATED) > > + return -EINVAL; > > + if (copy_from_user(&setup, arg, sizeof(setup))) > > + return -EFAULT; > > + if (!setup.name[0]) > > + 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; > > + > > + retval = uinput_validate_absbits(dev); > > + if (retval < 0) > > + return retval; > > + > > + udev->state = UIST_SETUP_COMPLETE; > > + return 0; > > +} > > + > > +static int uinput_abs_setup(struct uinput_device *udev, > > + struct uinput_setup __user *arg, size_t size) > > +{ > > + struct uinput_abs_setup setup; > > + struct input_dev *dev; > > + > > + memset(&setup, 0, sizeof(setup)); > > This could be simplified as: > > struct uinput_abs_setup setup = {}; > > > + > > + if (size > sizeof(setup)) > > + return -EINVAL; > > Maybe use something more specific here. This is something user-space > may want to match for if the ioctl struct gets extended. E2BIG? > > > + if (udev->state == UIST_CREATED) > > + return -EINVAL; > > + if (copy_from_user(&setup, arg, size)) > > + return -EFAULT; > > + if (setup.code > ABS_MAX) > > + return -ERANGE; > > + > > + dev = udev->dev; > > + > > + input_alloc_absinfo(dev); > > + if (!dev->absinfo) > > + return -ENOMEM; > > + > > + set_bit(setup.code, dev->absbit); > > + > > If you do a second round of this patch, maybe drop that newline? > > > + dev->absinfo[setup.code] = setup.absinfo; > > + > > + /* > > + * We restore the state to UIST_NEW_DEVICE because the user has to call > > + * UI_DEV_SETUP in the last place before UI_DEV_CREATE to check the > > + * validity of the absbits. > > + */ > > + udev->state = UIST_NEW_DEVICE; > > + 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; > > @@ -474,7 +542,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); > > > > @@ -735,6 +803,12 @@ 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; > > + > > + /* UI_ABS_SETUP is handled in the variable size ioctls */ > > + > > case UI_SET_EVBIT: > > retval = uinput_set_bit(arg, evbit, EV_MAX); > > goto out; > > @@ -879,6 +953,9 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, > > name = dev_name(&udev->dev->dev); > > retval = uinput_str_to_user(p, name, size); > > goto out; > > + case UI_ABS_SETUP & ~IOCSIZE_MASK: > > + retval = uinput_abs_setup(udev, p, size); > > + goto out; > > } > > > > retval = -EINVAL; > > diff --git a/include/linux/uinput.h b/include/linux/uinput.h > > index 0994c0d..75de43d 100644 > > --- a/include/linux/uinput.h > > +++ b/include/linux/uinput.h > > @@ -20,6 +20,11 @@ > > * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org> > > * > > * Changes/Revisions: > > + * 0.5 08/13/2015 (David Herrmann <dh.herrmann@gmail.com> & > > + * Benjamin Tissoires <benjamin.tissoires@redhat.com>) > > + * - add UI_DEV_SETUP ioctl > > + * - add UI_ABS_SETUP ioctl > > + * - add UI_GET_VERSION ioctl > > * 0.4 01/09/2014 (Benjamin Tissoires <benjamin.tissoires@redhat.com>) > > * - add UI_GET_SYSNAME ioctl > > * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) > > diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h > > index 013c9d8..ef6c9f5 100644 > > --- a/include/uapi/linux/uinput.h > > +++ b/include/uapi/linux/uinput.h > > @@ -20,6 +20,11 @@ > > * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org> > > * > > * Changes/Revisions: > > + * 0.5 08/13/2015 (David Herrmann <dh.herrmann@gmail.com> & > > + * Benjamin Tissoires <benjamin.tissoires@redhat.com>) > > + * - add UI_DEV_SETUP ioctl > > + * - add UI_ABS_SETUP ioctl > > + * - add UI_GET_VERSION ioctl > > * 0.4 01/09/2014 (Benjamin Tissoires <benjamin.tissoires@redhat.com>) > > * - add UI_GET_SYSNAME ioctl > > * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) > > @@ -37,8 +42,8 @@ > > #include <linux/types.h> > > #include <linux/input.h> > > > > -#define UINPUT_VERSION 4 > > - > > +#define UINPUT_VERSION 5 > > +#define UINPUT_MAX_NAME_SIZE 80 > > > > struct uinput_ff_upload { > > __u32 request_id; > > @@ -58,6 +63,79 @@ struct uinput_ff_erase { > > #define UI_DEV_CREATE _IO(UINPUT_IOCTL_BASE, 1) > > #define UI_DEV_DESTROY _IO(UINPUT_IOCTL_BASE, 2) > > > > +struct uinput_setup { > > + struct input_id id; > > + char name[UINPUT_MAX_NAME_SIZE]; > > + __u32 ff_effects_max; > > +}; > > + > > +/** > > + * UI_DEV_SETUP - Set device parameters for setup > > + * > > + * This ioctl sets parameters for the input-device to be created. It must be > > + * issued *before* calling UI_DEV_CREATE or it will fail. This ioctl supersedes > > + * the old "struct uinput_user_dev" method, which wrote this data via write(). > > + * To actually set the absolute axes, you also need to call the ioctl > > + * UI_ABS_SETUP *before* calling this ioctl. > > + * > > + * This ioctl takes a "struct uinput_setup" object as argument. The fields of > > + * this object are as follows: > > + * id: See the description of "struct input_id". This field is > > + * copied unchanged into the new device. > > + * name: This is used unchanged as name for the new device. > > + * ff_effects_max: This limits the maximum numbers of force-feedback effects. > > + * See below for a description of FF with uinput. > > + * > > + * This ioctl can be called multiple times and will overwrite previous values. > > + * If this ioctl fails with -EINVAL, you're recommended to use the old > > + * "uinput_user_dev" method via write() as fallback, in case you run on an old > > + * kernel that does not support this ioctl. > > + * > > + * This ioctl may fail with -EINVAL if it is not supported or if you passed > > + * incorrect values, -ENOMEM if the kernel runs out of memory or -EFAULT if the > > + * passed uinput_setup object cannot be read/written. > > + * If this call fails, partial data may have already been applied to the > > + * internal device. > > + */ > > +#define UI_DEV_SETUP _IOW(UINPUT_IOCTL_BASE, 3, struct uinput_setup) > > + > > +struct uinput_abs_setup { > > + __u16 code; /* axis code */ > > + /* __u16 filler; */ > > + struct input_absinfo absinfo; > > +}; > > + > > +/** > > + * UI_ABS_SETUP - Set absolute axis information for the device to setup > > + * > > + * This ioctl sets one absolute axis information for the input-device to be > > + * created. It must be issued *before* calling UI_DEV_SETUP and UI_DEV_CREATE > > + * for every absolute axis the device exports. > > + * This ioctl supersedes the old "struct uinput_user_dev" method, which wrote > > + * part of this data and the content of UI_DEV_SETUP via write(). > > + * > > + * This ioctl takes a "struct uinput_abs_setup" object as argument. The fields > > + * of this object are as follows: > > + * code: The corresponding input code associated with this axis > > + * (ABS_X, ABS_Y, etc...) > > + * absinfo: See "struct input_absinfo" for a description of this field. > > + * This field is copied unchanged into the kernel for the > > + * specified axis. If the axis is not enabled via > > + * UI_SET_ABSBIT, this ioctl will enable it. > > + * > > + * This ioctl can be called multiple times and will overwrite previous values. > > + * If this ioctl fails with -EINVAL, you're recommended to use the old > > + * "uinput_user_dev" method via write() as fallback, in case you run on an old > > + * kernel that does not support this ioctl. > > + * > > + * This ioctl may fail with -EINVAL if it is not supported or if you passed > > + * incorrect values, -ENOMEM if the kernel runs out of memory or -EFAULT if the > > + * passed uinput_setup object cannot be read/written. > > + * If this call fails, partial data may have already been applied to the > > + * internal device. > > + */ > > +#define UI_ABS_SETUP _IOW(UINPUT_IOCTL_BASE, 4, struct uinput_abs_setup) > > + > > #define UI_SET_EVBIT _IOW(UINPUT_IOCTL_BASE, 100, int) > > #define UI_SET_KEYBIT _IOW(UINPUT_IOCTL_BASE, 101, int) > > #define UI_SET_RELBIT _IOW(UINPUT_IOCTL_BASE, 102, int) > > @@ -144,7 +222,6 @@ struct uinput_ff_erase { > > #define UI_FF_UPLOAD 1 > > #define UI_FF_ERASE 2 > > > > -#define UINPUT_MAX_NAME_SIZE 80 > > struct uinput_user_dev { > > char name[UINPUT_MAX_NAME_SIZE]; > > struct input_id id; > > Looks good to me! Thanks for picking it up! Feel free to append: > > Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Thanks for your comments David. I'll send a new version right away with your suggestions. Cheers, Benjamin -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index 345df9b..afbcc41 100644 --- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -370,8 +370,76 @@ 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 retval; + + if (udev->state == UIST_CREATED) + return -EINVAL; + if (copy_from_user(&setup, arg, sizeof(setup))) + return -EFAULT; + if (!setup.name[0]) + 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; + + retval = uinput_validate_absbits(dev); + if (retval < 0) + return retval; + + udev->state = UIST_SETUP_COMPLETE; + return 0; +} + +static int uinput_abs_setup(struct uinput_device *udev, + struct uinput_setup __user *arg, size_t size) +{ + struct uinput_abs_setup setup; + struct input_dev *dev; + + memset(&setup, 0, sizeof(setup)); + + if (size > sizeof(setup)) + return -EINVAL; + if (udev->state == UIST_CREATED) + return -EINVAL; + if (copy_from_user(&setup, arg, size)) + return -EFAULT; + if (setup.code > ABS_MAX) + return -ERANGE; + + dev = udev->dev; + + input_alloc_absinfo(dev); + if (!dev->absinfo) + return -ENOMEM; + + set_bit(setup.code, dev->absbit); + + dev->absinfo[setup.code] = setup.absinfo; + + /* + * We restore the state to UIST_NEW_DEVICE because the user has to call + * UI_DEV_SETUP in the last place before UI_DEV_CREATE to check the + * validity of the absbits. + */ + udev->state = UIST_NEW_DEVICE; + 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; @@ -474,7 +542,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); @@ -735,6 +803,12 @@ 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; + + /* UI_ABS_SETUP is handled in the variable size ioctls */ + case UI_SET_EVBIT: retval = uinput_set_bit(arg, evbit, EV_MAX); goto out; @@ -879,6 +953,9 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, name = dev_name(&udev->dev->dev); retval = uinput_str_to_user(p, name, size); goto out; + case UI_ABS_SETUP & ~IOCSIZE_MASK: + retval = uinput_abs_setup(udev, p, size); + goto out; } retval = -EINVAL; diff --git a/include/linux/uinput.h b/include/linux/uinput.h index 0994c0d..75de43d 100644 --- a/include/linux/uinput.h +++ b/include/linux/uinput.h @@ -20,6 +20,11 @@ * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org> * * Changes/Revisions: + * 0.5 08/13/2015 (David Herrmann <dh.herrmann@gmail.com> & + * Benjamin Tissoires <benjamin.tissoires@redhat.com>) + * - add UI_DEV_SETUP ioctl + * - add UI_ABS_SETUP ioctl + * - add UI_GET_VERSION ioctl * 0.4 01/09/2014 (Benjamin Tissoires <benjamin.tissoires@redhat.com>) * - add UI_GET_SYSNAME ioctl * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h index 013c9d8..ef6c9f5 100644 --- a/include/uapi/linux/uinput.h +++ b/include/uapi/linux/uinput.h @@ -20,6 +20,11 @@ * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org> * * Changes/Revisions: + * 0.5 08/13/2015 (David Herrmann <dh.herrmann@gmail.com> & + * Benjamin Tissoires <benjamin.tissoires@redhat.com>) + * - add UI_DEV_SETUP ioctl + * - add UI_ABS_SETUP ioctl + * - add UI_GET_VERSION ioctl * 0.4 01/09/2014 (Benjamin Tissoires <benjamin.tissoires@redhat.com>) * - add UI_GET_SYSNAME ioctl * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) @@ -37,8 +42,8 @@ #include <linux/types.h> #include <linux/input.h> -#define UINPUT_VERSION 4 - +#define UINPUT_VERSION 5 +#define UINPUT_MAX_NAME_SIZE 80 struct uinput_ff_upload { __u32 request_id; @@ -58,6 +63,79 @@ struct uinput_ff_erase { #define UI_DEV_CREATE _IO(UINPUT_IOCTL_BASE, 1) #define UI_DEV_DESTROY _IO(UINPUT_IOCTL_BASE, 2) +struct uinput_setup { + struct input_id id; + char name[UINPUT_MAX_NAME_SIZE]; + __u32 ff_effects_max; +}; + +/** + * UI_DEV_SETUP - Set device parameters for setup + * + * This ioctl sets parameters for the input-device to be created. It must be + * issued *before* calling UI_DEV_CREATE or it will fail. This ioctl supersedes + * the old "struct uinput_user_dev" method, which wrote this data via write(). + * To actually set the absolute axes, you also need to call the ioctl + * UI_ABS_SETUP *before* calling this ioctl. + * + * This ioctl takes a "struct uinput_setup" object as argument. The fields of + * this object are as follows: + * id: See the description of "struct input_id". This field is + * copied unchanged into the new device. + * name: This is used unchanged as name for the new device. + * ff_effects_max: This limits the maximum numbers of force-feedback effects. + * See below for a description of FF with uinput. + * + * This ioctl can be called multiple times and will overwrite previous values. + * If this ioctl fails with -EINVAL, you're recommended to use the old + * "uinput_user_dev" method via write() as fallback, in case you run on an old + * kernel that does not support this ioctl. + * + * This ioctl may fail with -EINVAL if it is not supported or if you passed + * incorrect values, -ENOMEM if the kernel runs out of memory or -EFAULT if the + * passed uinput_setup object cannot be read/written. + * If this call fails, partial data may have already been applied to the + * internal device. + */ +#define UI_DEV_SETUP _IOW(UINPUT_IOCTL_BASE, 3, struct uinput_setup) + +struct uinput_abs_setup { + __u16 code; /* axis code */ + /* __u16 filler; */ + struct input_absinfo absinfo; +}; + +/** + * UI_ABS_SETUP - Set absolute axis information for the device to setup + * + * This ioctl sets one absolute axis information for the input-device to be + * created. It must be issued *before* calling UI_DEV_SETUP and UI_DEV_CREATE + * for every absolute axis the device exports. + * This ioctl supersedes the old "struct uinput_user_dev" method, which wrote + * part of this data and the content of UI_DEV_SETUP via write(). + * + * This ioctl takes a "struct uinput_abs_setup" object as argument. The fields + * of this object are as follows: + * code: The corresponding input code associated with this axis + * (ABS_X, ABS_Y, etc...) + * absinfo: See "struct input_absinfo" for a description of this field. + * This field is copied unchanged into the kernel for the + * specified axis. If the axis is not enabled via + * UI_SET_ABSBIT, this ioctl will enable it. + * + * This ioctl can be called multiple times and will overwrite previous values. + * If this ioctl fails with -EINVAL, you're recommended to use the old + * "uinput_user_dev" method via write() as fallback, in case you run on an old + * kernel that does not support this ioctl. + * + * This ioctl may fail with -EINVAL if it is not supported or if you passed + * incorrect values, -ENOMEM if the kernel runs out of memory or -EFAULT if the + * passed uinput_setup object cannot be read/written. + * If this call fails, partial data may have already been applied to the + * internal device. + */ +#define UI_ABS_SETUP _IOW(UINPUT_IOCTL_BASE, 4, struct uinput_abs_setup) + #define UI_SET_EVBIT _IOW(UINPUT_IOCTL_BASE, 100, int) #define UI_SET_KEYBIT _IOW(UINPUT_IOCTL_BASE, 101, int) #define UI_SET_RELBIT _IOW(UINPUT_IOCTL_BASE, 102, int) @@ -144,7 +222,6 @@ struct uinput_ff_erase { #define UI_FF_UPLOAD 1 #define UI_FF_ERASE 2 -#define UINPUT_MAX_NAME_SIZE 80 struct uinput_user_dev { char name[UINPUT_MAX_NAME_SIZE]; struct input_id id;