Re: [GIT] HID for 3.12 merge window

[David Herrmann <dh.herrmann@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2862 bytes --]

Hi

On Fri, Sep 6, 2013 at 10:20 PM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> On 2013.09.06 at 14:00 +0200, Jiri Kosina wrote:
>>
>> David Herrmann (12):
> ...
>>       HID: wiimote: add support for Guitar-Hero drums
>
>  commit 61e00655e9cb82e034eb72b95a51072e718d14a7
>  Author: David Herrmann <dh.herrmann@gmail.com>
>  Date:   Mon Aug 26 19:14:46 2013 +0200
>
>      Input: introduce BTN/ABS bits for drums and guitars
>
> The commit above breaks my Logitech mouse. The mouse cursor just sits in
> the middle of the screen and doesn't react to movements. dmesg is
> normal, but Xorg.0.log says:

Ok, the issue is the kernel assumes ABS_MAX to be a power-of-2 minus 1
(used as mask). That wasn't really obvious to me. Attached is a patch
which should fix that. Could you apply it on top of linus/master and
give it a try?

@Dmitry: The IOC_NR part of the definition of EVIOCSABS() is now
bigger than 1-byte. I need to check how that affects the 'E' part. Any
idea what to do here?

Thanks
David

Patch is also attached as I doubt that inlining it works in that
stupid web-client:

>From 653fe4d46ad368cdbf9b56a559a8468bd6f5cb3c Mon Sep 17 00:00:00 2001
From: David Herrmann <dh.herrmann@gmail.com>
Date: Fri, 6 Sep 2013 23:46:08 +0200
Subject: [PATCH] Input: evdev: don't assume ABS_MAX to be a power-of-2 minus 1

ABS_MAX is no longer a full mask. Hence, don't use it directly to get any
parameter for ioctls. Furthermore, the parameter-region and
ioctl-definition overlap, so even bumping ABS_MAX to 0x7f wouldn't help.

Reported-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/input/evdev.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index d2b34fb..82e0073 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -939,12 +939,13 @@ static long evdev_do_ioctl(struct file *file,
unsigned int cmd,
  _IOC_NR(cmd) & EV_MAX, size,
  p, compat_mode);

- if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCGABS(0))) {
+ if (_IOC_NR(cmd) >= _IOC_NR(EVIOCGABS(0)) &&
+    _IOC_NR(cmd) <= _IOC_NR(EVIOCGABS(ABS_MAX))) {

  if (!dev->absinfo)
  return -EINVAL;

- t = _IOC_NR(cmd) & ABS_MAX;
+ t = _IOC_NR(cmd) - _IOC_NR(EVIOCGABS(0));
  abs = dev->absinfo[t];

  if (copy_to_user(p, &abs, min_t(size_t,
@@ -957,12 +958,13 @@ static long evdev_do_ioctl(struct file *file,
unsigned int cmd,

  if (_IOC_DIR(cmd) == _IOC_WRITE) {

- if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCSABS(0))) {
+ if (_IOC_NR(cmd) >= _IOC_NR(EVIOCSABS(0)) &&
+    _IOC_NR(cmd) <= _IOC_NR(EVIOCSABS(ABS_MAX))) {

  if (!dev->absinfo)
  return -EINVAL;

- t = _IOC_NR(cmd) & ABS_MAX;
+ t = _IOC_NR(cmd) - _IOC_NR(EVIOCSABS(0));

  if (copy_from_user(&abs, p, min_t(size_t,
  size, sizeof(struct input_absinfo))))
-- 
1.8.4

[-- Attachment #2: 0001-Input-evdev-don-t-assume-ABS_MAX-to-be-a-power-of-2-.patch --]
[-- Type: application/octet-stream, Size: 1820 bytes --]

From 653fe4d46ad368cdbf9b56a559a8468bd6f5cb3c Mon Sep 17 00:00:00 2001
From: David Herrmann <dh.herrmann@gmail.com>
Date: Fri, 6 Sep 2013 23:46:08 +0200
Subject: [PATCH] Input: evdev: don't assume ABS_MAX to be a power-of-2 minus 1

ABS_MAX is no longer a full mask. Hence, don't use it directly to get any
parameter for ioctls. Furthermore, the parameter-region and
ioctl-definition overlap, so even bumping ABS_MAX to 0x7f wouldn't help.

Reported-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/input/evdev.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index d2b34fb..82e0073 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -939,12 +939,13 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 						_IOC_NR(cmd) & EV_MAX, size,
 						p, compat_mode);
 
-		if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCGABS(0))) {
+		if (_IOC_NR(cmd) >= _IOC_NR(EVIOCGABS(0)) &&
+		    _IOC_NR(cmd) <= _IOC_NR(EVIOCGABS(ABS_MAX))) {
 
 			if (!dev->absinfo)
 				return -EINVAL;
 
-			t = _IOC_NR(cmd) & ABS_MAX;
+			t = _IOC_NR(cmd) - _IOC_NR(EVIOCGABS(0));
 			abs = dev->absinfo[t];
 
 			if (copy_to_user(p, &abs, min_t(size_t,
@@ -957,12 +958,13 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 
 	if (_IOC_DIR(cmd) == _IOC_WRITE) {
 
-		if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCSABS(0))) {
+		if (_IOC_NR(cmd) >= _IOC_NR(EVIOCSABS(0)) &&
+		    _IOC_NR(cmd) <= _IOC_NR(EVIOCSABS(ABS_MAX))) {
 
 			if (!dev->absinfo)
 				return -EINVAL;
 
-			t = _IOC_NR(cmd) & ABS_MAX;
+			t = _IOC_NR(cmd) - _IOC_NR(EVIOCSABS(0));
 
 			if (copy_from_user(&abs, p, min_t(size_t,
 					size, sizeof(struct input_absinfo))))
-- 
1.8.4