From: Peter Hutterer <[email protected]>
Input resolution has been added in kernel v2.6.31 with commit
ec20a022aa24fc63. However, uinput was never updated since, meaning that
devices cannot be emulated in full. User-space processes that rely on the
resolution to be accurate need this.
Resolution is added in struct uinput_user_dev in a way it won't break
current programs relying on the old implementation.
Bump the UINPUT_VERSION number to 4 to aid detection of this in user-space.
Signed-off-by: Peter Hutterer <[email protected]>
Signed-off-by: Benjamin Tissoires <[email protected]>
---
Sorry for that, I mixed up the "From:" field.
This v2 contains the right From in the first patch , and this is the only
difference...
Cheers,
Benjamin
drivers/input/misc/uinput.c | 17 ++++++++++++-----
include/linux/uinput.h | 2 ++
include/uapi/linux/uinput.h | 3 +++
3 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index a0a4bba..7d518b4 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -20,6 +20,8 @@
* Author: Aristeu Sergio Rozanski Filho <[email protected]>
*
* Changes/Revisions:
+ * 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
+ * - update uinput_user_dev struct to allow abs resolution
* 0.3 09/04/2006 (Anssi Hannula <[email protected]>)
* - updated ff support for the changes in kernel interface
* - added MODULE_VERSION
@@ -364,9 +366,9 @@ static int uinput_setup_device(struct uinput_device *udev,
struct input_dev *dev;
int i;
int retval;
+ size_t size;
- if (count != sizeof(struct uinput_user_dev))
- return -EINVAL;
+ size = min_t(size_t, count, sizeof(struct uinput_user_dev));
if (!udev->dev) {
retval = uinput_allocate_device(udev);
@@ -376,9 +378,13 @@ static int uinput_setup_device(struct uinput_device *udev,
dev = udev->dev;
- user_dev = memdup_user(buffer, sizeof(struct uinput_user_dev));
- if (IS_ERR(user_dev))
- return PTR_ERR(user_dev);
+ user_dev = kzalloc(sizeof(struct uinput_user_dev), GFP_KERNEL);
+ if (!user_dev)
+ return -ENOMEM;
+ if (copy_from_user(user_dev, buffer, size)) {
+ retval = -EFAULT;
+ goto exit;
+ }
udev->ff_effects_max = user_dev->ff_effects_max;
@@ -406,6 +412,7 @@ static int uinput_setup_device(struct uinput_device *udev,
input_abs_set_min(dev, i, user_dev->absmin[i]);
input_abs_set_fuzz(dev, i, user_dev->absfuzz[i]);
input_abs_set_flat(dev, i, user_dev->absflat[i]);
+ input_abs_set_res(dev, i, user_dev->absres[i]);
}
/* check if absmin/absmax/absfuzz/absflat are filled as
diff --git a/include/linux/uinput.h b/include/linux/uinput.h
index 0a4487d..6291a22 100644
--- a/include/linux/uinput.h
+++ b/include/linux/uinput.h
@@ -20,6 +20,8 @@
* Author: Aristeu Sergio Rozanski Filho <[email protected]>
*
* Changes/Revisions:
+ * 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
+ * - update uinput_user_dev struct to allow abs resolution
* 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
* - update ff support for the changes in kernel interface
* - add UINPUT_VERSION
diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
index fe46431..f6a393b 100644
--- a/include/uapi/linux/uinput.h
+++ b/include/uapi/linux/uinput.h
@@ -20,6 +20,8 @@
* Author: Aristeu Sergio Rozanski Filho <[email protected]>
*
* Changes/Revisions:
+ * 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
+ * - update uinput_user_dev struct to allow abs resolution
* 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
* - update ff support for the changes in kernel interface
* - add UINPUT_VERSION
@@ -133,5 +135,6 @@ struct uinput_user_dev {
__s32 absmin[ABS_CNT];
__s32 absfuzz[ABS_CNT];
__s32 absflat[ABS_CNT];
+ __s32 absres[ABS_CNT];
};
#endif /* _UAPI__UINPUT_H_ */
--
1.8.3.1
Evemu [1] uses uinput to replay devices traces it has recorded. However,
the way evemu uses uinput is slightly different from how uinput is
supposed to be used.
Evemu creates the device node through uinput, bu inject events through
the input device node directly (and skipping the uinput node).
Currently, evemu relies on an heuristic to guess which input node was
created. The problem is that is heuristic is subjected to races between
different uinput devices or even with physical devices. Having a way
to retrieve the sysfs path allows us to find the event node without
having to rely on this heuristic.
[1] http://www.freedesktop.org/wiki/Evemu/
Signed-off-by: Benjamin Tissoires <[email protected]>
---
drivers/input/misc/uinput.c | 37 ++++++++++++++++++++++++++++++++++++-
include/linux/uinput.h | 1 +
include/uapi/linux/uinput.h | 3 +++
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 7d518b4..49a9f7d 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -22,6 +22,7 @@
* Changes/Revisions:
* 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
* - update uinput_user_dev struct to allow abs resolution
+ * - add UI_GET_SYSPATH ioctl
* 0.3 09/04/2006 (Anssi Hannula <[email protected]>)
* - updated ff support for the changes in kernel interface
* - added MODULE_VERSION
@@ -667,6 +668,21 @@ static int uinput_ff_upload_from_user(const char __user *buffer,
__ret; \
})
+static int uinput_str_to_user(const char *str, unsigned int maxlen,
+ void __user *p)
+{
+ int len;
+
+ if (!str)
+ return -ENOENT;
+
+ len = strlen(str) + 1;
+ if (len > maxlen)
+ len = maxlen;
+
+ return copy_to_user(p, str, len) ? -EFAULT : len;
+}
+
static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
unsigned long arg, void __user *p)
{
@@ -676,6 +692,8 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
struct uinput_ff_erase ff_erase;
struct uinput_request *req;
char *phys;
+ const char *path;
+ unsigned int size;
retval = mutex_lock_interruptible(&udev->mutex);
if (retval)
@@ -828,7 +846,24 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
break;
default:
- retval = -EINVAL;
+ retval = -EAGAIN;
+ }
+
+ if (retval == -EAGAIN) {
+ size = _IOC_SIZE(cmd);
+
+ /* Now check variable-length commands */
+ switch (cmd & ~IOCSIZE_MASK) {
+ case UI_GET_SYSPATH(0):
+ path = kobject_get_path(&udev->dev->dev.kobj,
+ GFP_KERNEL);
+ retval = uinput_str_to_user(path, size, p);
+ kfree(path);
+ break;
+
+ default:
+ retval = -EINVAL;
+ }
}
out:
diff --git a/include/linux/uinput.h b/include/linux/uinput.h
index 6291a22..64fab81 100644
--- a/include/linux/uinput.h
+++ b/include/linux/uinput.h
@@ -22,6 +22,7 @@
* Changes/Revisions:
* 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
* - update uinput_user_dev struct to allow abs resolution
+ * - add UI_GET_SYSPATH ioctl
* 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
* - update ff support for the changes in kernel interface
* - add UINPUT_VERSION
diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
index f6a393b..d826409 100644
--- a/include/uapi/linux/uinput.h
+++ b/include/uapi/linux/uinput.h
@@ -22,6 +22,7 @@
* Changes/Revisions:
* 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
* - update uinput_user_dev struct to allow abs resolution
+ * - add UI_GET_SYSPATH ioctl
* 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
* - update ff support for the changes in kernel interface
* - add UINPUT_VERSION
@@ -75,6 +76,8 @@ struct uinput_ff_erase {
#define UI_BEGIN_FF_ERASE _IOWR(UINPUT_IOCTL_BASE, 202, struct uinput_ff_erase)
#define UI_END_FF_ERASE _IOW(UINPUT_IOCTL_BASE, 203, struct uinput_ff_erase)
+#define UI_GET_SYSPATH(len) _IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len)
+
/*
* To write a force-feedback-capable driver, the upload_effect
* and erase_effect callbacks in input_dev must be implemented.
--
1.8.3.1
On Mon, Jul 15, 2013 at 03:37:09PM +0200, Benjamin Tissoires wrote:
> Evemu [1] uses uinput to replay devices traces it has recorded. However,
> the way evemu uses uinput is slightly different from how uinput is
> supposed to be used.
> Evemu creates the device node through uinput, bu inject events through
> the input device node directly (and skipping the uinput node).
>
> Currently, evemu relies on an heuristic to guess which input node was
> created. The problem is that is heuristic is subjected to races between
> different uinput devices or even with physical devices. Having a way
> to retrieve the sysfs path allows us to find the event node without
> having to rely on this heuristic.
>
> [1] http://www.freedesktop.org/wiki/Evemu/
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
Tested-by: Peter Hutterer <[email protected]>
I'd really appreciate a comment above the ioctl define stating what the
ioctl actually returns. only comment regarding the code:
> +static int uinput_str_to_user(const char *str, unsigned int maxlen,
> + void __user *p)
I'd do this with a signature of the type (dest, src, len).
Cheers,
Peter
> ---
> drivers/input/misc/uinput.c | 37 ++++++++++++++++++++++++++++++++++++-
> include/linux/uinput.h | 1 +
> include/uapi/linux/uinput.h | 3 +++
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index 7d518b4..49a9f7d 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -22,6 +22,7 @@
> * Changes/Revisions:
> * 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
> * - update uinput_user_dev struct to allow abs resolution
> + * - add UI_GET_SYSPATH ioctl
> * 0.3 09/04/2006 (Anssi Hannula <[email protected]>)
> * - updated ff support for the changes in kernel interface
> * - added MODULE_VERSION
> @@ -667,6 +668,21 @@ static int uinput_ff_upload_from_user(const char __user *buffer,
> __ret; \
> })
>
> +static int uinput_str_to_user(const char *str, unsigned int maxlen,
> + void __user *p)
> +{
> + int len;
> +
> + if (!str)
> + return -ENOENT;
> +
> + len = strlen(str) + 1;
> + if (len > maxlen)
> + len = maxlen;
> +
> + return copy_to_user(p, str, len) ? -EFAULT : len;
> +}
> +
> static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
> unsigned long arg, void __user *p)
> {
> @@ -676,6 +692,8 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
> struct uinput_ff_erase ff_erase;
> struct uinput_request *req;
> char *phys;
> + const char *path;
> + unsigned int size;
>
> retval = mutex_lock_interruptible(&udev->mutex);
> if (retval)
> @@ -828,7 +846,24 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
> break;
>
> default:
> - retval = -EINVAL;
> + retval = -EAGAIN;
> + }
> +
> + if (retval == -EAGAIN) {
> + size = _IOC_SIZE(cmd);
> +
> + /* Now check variable-length commands */
> + switch (cmd & ~IOCSIZE_MASK) {
> + case UI_GET_SYSPATH(0):
> + path = kobject_get_path(&udev->dev->dev.kobj,
> + GFP_KERNEL);
> + retval = uinput_str_to_user(path, size, p);
> + kfree(path);
> + break;
> +
> + default:
> + retval = -EINVAL;
> + }
> }
>
> out:
> diff --git a/include/linux/uinput.h b/include/linux/uinput.h
> index 6291a22..64fab81 100644
> --- a/include/linux/uinput.h
> +++ b/include/linux/uinput.h
> @@ -22,6 +22,7 @@
> * Changes/Revisions:
> * 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
> * - update uinput_user_dev struct to allow abs resolution
> + * - add UI_GET_SYSPATH ioctl
> * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
> * - update ff support for the changes in kernel interface
> * - add UINPUT_VERSION
> diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
> index f6a393b..d826409 100644
> --- a/include/uapi/linux/uinput.h
> +++ b/include/uapi/linux/uinput.h
> @@ -22,6 +22,7 @@
> * Changes/Revisions:
> * 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
> * - update uinput_user_dev struct to allow abs resolution
> + * - add UI_GET_SYSPATH ioctl
> * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
> * - update ff support for the changes in kernel interface
> * - add UINPUT_VERSION
> @@ -75,6 +76,8 @@ struct uinput_ff_erase {
> #define UI_BEGIN_FF_ERASE _IOWR(UINPUT_IOCTL_BASE, 202, struct uinput_ff_erase)
> #define UI_END_FF_ERASE _IOW(UINPUT_IOCTL_BASE, 203, struct uinput_ff_erase)
>
> +#define UI_GET_SYSPATH(len) _IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len)
> +
> /*
> * To write a force-feedback-capable driver, the upload_effect
> * and erase_effect callbacks in input_dev must be implemented.
> --
> 1.8.3.1
>
Hi
On Mon, Jul 15, 2013 at 3:37 PM, Benjamin Tissoires
<[email protected]> wrote:
> Evemu [1] uses uinput to replay devices traces it has recorded. However,
> the way evemu uses uinput is slightly different from how uinput is
> supposed to be used.
> Evemu creates the device node through uinput, bu inject events through
> the input device node directly (and skipping the uinput node).
>
> Currently, evemu relies on an heuristic to guess which input node was
> created. The problem is that is heuristic is subjected to races between
> different uinput devices or even with physical devices. Having a way
> to retrieve the sysfs path allows us to find the event node without
> having to rely on this heuristic.
It would actually be enough to return the "input_no" from
input_register_device() (which is currently local but we could save it
in "dev"). Or only the device-name. I don't know why you want the full
syspath. It's just overhead in the kernel that we could easily let
user-space do. And the path /sys/class/input/<devname> can be put
together by user-space.
Anyway, Dmitry has to decide on that. Apart from some style-issues I
mentioned below:
Reviewed-by: David Herrmann <[email protected]>
Cheers
David
> [1] http://www.freedesktop.org/wiki/Evemu/
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
> ---
> drivers/input/misc/uinput.c | 37 ++++++++++++++++++++++++++++++++++++-
> include/linux/uinput.h | 1 +
> include/uapi/linux/uinput.h | 3 +++
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index 7d518b4..49a9f7d 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -22,6 +22,7 @@
> * Changes/Revisions:
> * 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
> * - update uinput_user_dev struct to allow abs resolution
> + * - add UI_GET_SYSPATH ioctl
What tree is that patch against? I cannot see an "0.4" entry in:
http://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/tree/drivers/input/misc/uinput.c?h=next
> * 0.3 09/04/2006 (Anssi Hannula <[email protected]>)
> * - updated ff support for the changes in kernel interface
> * - added MODULE_VERSION
> @@ -667,6 +668,21 @@ static int uinput_ff_upload_from_user(const char __user *buffer,
> __ret; \
> })
>
> +static int uinput_str_to_user(const char *str, unsigned int maxlen,
> + void __user *p)
As Peter mentioned, I'd move "maxlen" to the end.
> +{
> + int len;
> +
> + if (!str)
> + return -ENOENT;
> +
> + len = strlen(str) + 1;
> + if (len > maxlen)
> + len = maxlen;
> +
> + return copy_to_user(p, str, len) ? -EFAULT : len;
I'd prefer a "strlcpy()" so we guarantee a terminating 0 for
user-space, but I guess that'd be rather complex to do here. I
couldn't find any strlcpy_to_user()...
> +}
> +
> static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
> unsigned long arg, void __user *p)
> {
> @@ -676,6 +692,8 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
> struct uinput_ff_erase ff_erase;
> struct uinput_request *req;
> char *phys;
> + const char *path;
> + unsigned int size;
>
> retval = mutex_lock_interruptible(&udev->mutex);
> if (retval)
> @@ -828,7 +846,24 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
> break;
>
> default:
> - retval = -EINVAL;
> + retval = -EAGAIN;
> + }
> +
> + if (retval == -EAGAIN) {
> + size = _IOC_SIZE(cmd);
> +
> + /* Now check variable-length commands */
> + switch (cmd & ~IOCSIZE_MASK) {
> + case UI_GET_SYSPATH(0):
> + path = kobject_get_path(&udev->dev->dev.kobj,
> + GFP_KERNEL);
I know, device registration is protected by udev->mutex but I'd still prefer:
if (udev->state != UIST_CREATED)
return -ENOENT;
But that's probably a matter of taste.
> + retval = uinput_str_to_user(path, size, p);
> + kfree(path);
> + break;
> +
> + default:
> + retval = -EINVAL;
> + }
> }
>
> out:
> diff --git a/include/linux/uinput.h b/include/linux/uinput.h
> index 6291a22..64fab81 100644
> --- a/include/linux/uinput.h
> +++ b/include/linux/uinput.h
> @@ -22,6 +22,7 @@
> * Changes/Revisions:
> * 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
> * - update uinput_user_dev struct to allow abs resolution
> + * - add UI_GET_SYSPATH ioctl
> * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
> * - update ff support for the changes in kernel interface
> * - add UINPUT_VERSION
> diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
> index f6a393b..d826409 100644
> --- a/include/uapi/linux/uinput.h
> +++ b/include/uapi/linux/uinput.h
> @@ -22,6 +22,7 @@
> * Changes/Revisions:
> * 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
> * - update uinput_user_dev struct to allow abs resolution
> + * - add UI_GET_SYSPATH ioctl
> * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
> * - update ff support for the changes in kernel interface
> * - add UINPUT_VERSION
> @@ -75,6 +76,8 @@ struct uinput_ff_erase {
> #define UI_BEGIN_FF_ERASE _IOWR(UINPUT_IOCTL_BASE, 202, struct uinput_ff_erase)
> #define UI_END_FF_ERASE _IOW(UINPUT_IOCTL_BASE, 203, struct uinput_ff_erase)
>
> +#define UI_GET_SYSPATH(len) _IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len)
> +
> /*
> * To write a force-feedback-capable driver, the upload_effect
> * and erase_effect callbacks in input_dev must be implemented.
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
Hi David,
On Sat, Jul 27, 2013 at 3:07 PM, David Herrmann <[email protected]> wrote:
> Hi
>
> On Mon, Jul 15, 2013 at 3:37 PM, Benjamin Tissoires
> <[email protected]> wrote:
>> Evemu [1] uses uinput to replay devices traces it has recorded. However,
>> the way evemu uses uinput is slightly different from how uinput is
>> supposed to be used.
>> Evemu creates the device node through uinput, bu inject events through
>> the input device node directly (and skipping the uinput node).
>>
>> Currently, evemu relies on an heuristic to guess which input node was
>> created. The problem is that is heuristic is subjected to races between
>> different uinput devices or even with physical devices. Having a way
>> to retrieve the sysfs path allows us to find the event node without
>> having to rely on this heuristic.
>
> It would actually be enough to return the "input_no" from
> input_register_device() (which is currently local but we could save it
> in "dev"). Or only the device-name. I don't know why you want the full
> syspath. It's just overhead in the kernel that we could easily let
> user-space do. And the path /sys/class/input/<devname> can be put
> together by user-space.
I agree that the "input_no" is enough, but that would infer a change
in the input struct. However, we (Peter and I) want to be able to use
it in RHEL, or any other legacy distribution, where it is not
acceptable to ask to recompile a new kernel to do regressions tests.
For those distributions, I have set up a backport of uinput [2] and
with this backport, you only need to recompile the uinput module (not
the whole kernel tree).
Anyway, returning the input name seems enough too. It will remove the
path retrieval and a free :)
I agree this is not the kind of argument we usually like in the kernel
tree, but as this feature (resolution and input retrieval) seems to be
only valuable for regression tests (Xorg or Wayland), I think it is
valuable.
>
> Anyway, Dmitry has to decide on that. Apart from some style-issues I
> mentioned below:
> Reviewed-by: David Herrmann <[email protected]>
thanks!
>
> Cheers
> David
>
>> [1] http://www.freedesktop.org/wiki/Evemu/
[2] https://github.com/bentiss/uinput
>>
>> Signed-off-by: Benjamin Tissoires <[email protected]>
>> ---
>> drivers/input/misc/uinput.c | 37 ++++++++++++++++++++++++++++++++++++-
>> include/linux/uinput.h | 1 +
>> include/uapi/linux/uinput.h | 3 +++
>> 3 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
>> index 7d518b4..49a9f7d 100644
>> --- a/drivers/input/misc/uinput.c
>> +++ b/drivers/input/misc/uinput.c
>> @@ -22,6 +22,7 @@
>> * Changes/Revisions:
>> * 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
>> * - update uinput_user_dev struct to allow abs resolution
>> + * - add UI_GET_SYSPATH ioctl
>
> What tree is that patch against? I cannot see an "0.4" entry in:
> http://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/tree/drivers/input/misc/uinput.c?h=next
looks like you miss the msg title: 2/2. It is part of a series where
the first patch was done by Peter here:
https://patchwork.kernel.org/patch/2827521/
>
>> * 0.3 09/04/2006 (Anssi Hannula <[email protected]>)
>> * - updated ff support for the changes in kernel interface
>> * - added MODULE_VERSION
>> @@ -667,6 +668,21 @@ static int uinput_ff_upload_from_user(const char __user *buffer,
>> __ret; \
>> })
>>
>> +static int uinput_str_to_user(const char *str, unsigned int maxlen,
>> + void __user *p)
>
> As Peter mentioned, I'd move "maxlen" to the end.
right. I just copied/pasted the code in evdev.c, but this can be easily changed.
>
>> +{
>> + int len;
>> +
>> + if (!str)
>> + return -ENOENT;
>> +
>> + len = strlen(str) + 1;
>> + if (len > maxlen)
>> + len = maxlen;
>> +
>> + return copy_to_user(p, str, len) ? -EFAULT : len;
>
> I'd prefer a "strlcpy()" so we guarantee a terminating 0 for
> user-space, but I guess that'd be rather complex to do here. I
> couldn't find any strlcpy_to_user()...
Definitively, adding a terminating 0 seems a good idea to prevent
leaking of information from the kernel.
>
>> +}
>> +
>> static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
>> unsigned long arg, void __user *p)
>> {
>> @@ -676,6 +692,8 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
>> struct uinput_ff_erase ff_erase;
>> struct uinput_request *req;
>> char *phys;
>> + const char *path;
>> + unsigned int size;
>>
>> retval = mutex_lock_interruptible(&udev->mutex);
>> if (retval)
>> @@ -828,7 +846,24 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
>> break;
>>
>> default:
>> - retval = -EINVAL;
>> + retval = -EAGAIN;
>> + }
>> +
>> + if (retval == -EAGAIN) {
>> + size = _IOC_SIZE(cmd);
>> +
>> + /* Now check variable-length commands */
>> + switch (cmd & ~IOCSIZE_MASK) {
>> + case UI_GET_SYSPATH(0):
>> + path = kobject_get_path(&udev->dev->dev.kobj,
>> + GFP_KERNEL);
>
> I know, device registration is protected by udev->mutex but I'd still prefer:
> if (udev->state != UIST_CREATED)
> return -ENOENT;
oops. Definitively, this should be added.
>
> But that's probably a matter of taste.
I think it is a mistake not adding this. Without it (in the current
implementation), the user-space can call it before the device is
created, leading to some garbage (or even oopses) depending on how
kobject_get_path() handles the queries for non added devices.
Cheers,
Benjamin
>
>> + retval = uinput_str_to_user(path, size, p);
>> + kfree(path);
>> + break;
>> +
>> + default:
>> + retval = -EINVAL;
>> + }
>> }
>>
>> out:
>> diff --git a/include/linux/uinput.h b/include/linux/uinput.h
>> index 6291a22..64fab81 100644
>> --- a/include/linux/uinput.h
>> +++ b/include/linux/uinput.h
>> @@ -22,6 +22,7 @@
>> * Changes/Revisions:
>> * 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
>> * - update uinput_user_dev struct to allow abs resolution
>> + * - add UI_GET_SYSPATH ioctl
>> * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
>> * - update ff support for the changes in kernel interface
>> * - add UINPUT_VERSION
>> diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
>> index f6a393b..d826409 100644
>> --- a/include/uapi/linux/uinput.h
>> +++ b/include/uapi/linux/uinput.h
>> @@ -22,6 +22,7 @@
>> * Changes/Revisions:
>> * 0.4 12/07/2013 (Peter Hutterer <[email protected]>)
>> * - update uinput_user_dev struct to allow abs resolution
>> + * - add UI_GET_SYSPATH ioctl
>> * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
>> * - update ff support for the changes in kernel interface
>> * - add UINPUT_VERSION
>> @@ -75,6 +76,8 @@ struct uinput_ff_erase {
>> #define UI_BEGIN_FF_ERASE _IOWR(UINPUT_IOCTL_BASE, 202, struct uinput_ff_erase)
>> #define UI_END_FF_ERASE _IOW(UINPUT_IOCTL_BASE, 203, struct uinput_ff_erase)
>>
>> +#define UI_GET_SYSPATH(len) _IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len)
>> +
>> /*
>> * To write a force-feedback-capable driver, the upload_effect
>> * and erase_effect callbacks in input_dev must be implemented.
>> --
>> 1.8.3.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/