Re: [PATCH libinput] evdev: drop relative x/y motion from a device not marked as pointer
Hi,
On 18-08-15 07:08, Peter Hutterer wrote:
A device with REL_X/Y and keys gets marked only as ID_INPUT_KEY, initializes
as keyboard and then segfaults when we send x/y coordinates - pointer
acceleration never initializes.
Ignore the events and log a bug instead. This intentionally only papers over
the underlying issue, let's wait for a real device to trigger this and then
look at the correct solution.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
Looks good to me:
Reviewed-by: Hans de Goede hdego...@redhat.com
Regards,
Hans
---
src/evdev.c | 35 +
src/evdev.h | 1 +
test/device.c | 155 ++
test/litest.c | 2 +
4 files changed, 193 insertions(+)
diff --git a/src/evdev.c b/src/evdev.c
index 9414d9d..97c007c 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -638,6 +638,36 @@ evdev_notify_axis(struct evdev_device *device,
discrete);
}
+static inline bool
+evdev_reject_relative(struct evdev_device *device,
+ const struct input_event *e,
+ uint64_t time)
+{
+ struct libinput *libinput = device-base.seat-libinput;
+
+ if ((e-code == REL_X || e-code == REL_Y)
+ (device-seat_caps EVDEV_DEVICE_POINTER) == 0) {
+ switch (ratelimit_test(device-nonpointer_rel_limit)) {
+ case RATELIMIT_PASS:
+ log_bug_libinput(libinput,
+REL_X/Y from device '%s', but this device
is not a pointer\n,
+device-devname);
+ break;
+ case RATELIMIT_THRESHOLD:
+ log_bug_libinput(libinput,
+REL_X/Y event flood from '%s'\n,
+device-devname);
+ break;
+ case RATELIMIT_EXCEEDED:
+ break;
+ }
+
+ return true;
+ }
+
+ return false;
+}
+
static inline void
evdev_process_relative(struct evdev_device *device,
struct input_event *e, uint64_t time)
@@ -645,6 +675,9 @@ evdev_process_relative(struct evdev_device *device,
struct normalized_coords wheel_degrees = { 0.0, 0.0 };
struct discrete_coords discrete = { 0.0, 0.0 };
+ if (evdev_reject_relative(device, e, time))
+ return;
+
switch (e-code) {
case REL_X:
if (device-pending_event != EVDEV_RELATIVE_MOTION)
@@ -2157,6 +2190,8 @@ evdev_device_create(struct libinput_seat *seat,
/* at most 5 SYN_DROPPED log-messages per 30s */
ratelimit_init(device-syn_drop_limit, s2us(30), 5);
+ /* at most 5 log-messages per 5s */
+ ratelimit_init(device-nonpointer_rel_limit, s2us(5), 5);
matrix_init_identity(device-abs.calibration);
matrix_init_identity(device-abs.usermatrix);
diff --git a/src/evdev.h b/src/evdev.h
index 9f026b8..e44a65d 100644
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -223,6 +223,7 @@ struct evdev_device {
int dpi; /* HW resolution */
struct ratelimit syn_drop_limit; /* ratelimit for SYN_DROPPED logging */
+ struct ratelimit nonpointer_rel_limit; /* ratelimit for REL_* events
from non-pointer devices */
uint32_t model_flags;
};
diff --git a/test/device.c b/test/device.c
index 59939d6..aff5ee2 100644
--- a/test/device.c
+++ b/test/device.c
@@ -1030,6 +1030,156 @@ START_TEST(device_udev_tag_synaptics_serial)
}
END_TEST
+START_TEST(device_nonpointer_rel)
+{
+ struct libevdev_uinput *uinput;
+ struct libinput *li;
+ struct libinput_device *device;
+ int i;
+
+ uinput = litest_create_uinput_device(test device,
+NULL,
+EV_KEY, KEY_A,
+EV_KEY, KEY_B,
+EV_REL, REL_X,
+EV_REL, REL_Y,
+-1);
+ li = litest_create_context();
+ device = libinput_path_add_device(li,
+ libevdev_uinput_get_devnode(uinput));
+ ck_assert(device != NULL);
+
+ litest_disable_log_handler(li);
+ for (i = 0; i 100; i++) {
+ libevdev_uinput_write_event(uinput, EV_REL, REL_X, 1);
+ libevdev_uinput_write_event(uinput, EV_REL, REL_Y, -1);
+ libevdev_uinput_write_event(uinput, EV_SYN, SYN_REPORT, 0);
+ libinput_dispatch(li);
+ }
+ litest_restore_log_handler(li);
+
+ libinput_unref(li);
+ libevdev_uinput_destroy(uinput);
+}
+END_TEST
+
+START_TEST(device_touchpad_rel)
+{
+ struct libevdev_uinput *uinput;
+ struct libinput *li;
+ struct libinput_device *device;
+
Re: [PATCH libinput] evdev: drop relative x/y motion from a device not marked as pointer
On Mon, Aug 17, 2015 at 10:08 PM, Peter Hutterer peter.hutte...@who-t.net
wrote:
A device with REL_X/Y and keys gets marked only as ID_INPUT_KEY,
initializes
as keyboard and then segfaults when we send x/y coordinates - pointer
acceleration never initializes.
Ignore the events and log a bug instead. This intentionally only papers
over
the underlying issue, let's wait for a real device to trigger this and then
look at the correct solution.
+static inline bool
+evdev_reject_relative(struct evdev_device *device,
+ const struct input_event *e,
+ uint64_t time)
+{
+ struct libinput *libinput = device-base.seat-libinput;
+
+ if ((e-code == REL_X || e-code == REL_Y)
+ (device-seat_caps EVDEV_DEVICE_POINTER) == 0) {
+ switch (ratelimit_test(device-nonpointer_rel_limit)) {
+ case RATELIMIT_PASS:
+ log_bug_libinput(libinput,
+REL_X/Y from device '%s', but
this device is not a pointer\n,
+device-devname);
+ break;
+ case RATELIMIT_THRESHOLD:
+ log_bug_libinput(libinput,
+REL_X/Y event flood from '%s'\n,
+device-devname);
+ break;
+ case RATELIMIT_EXCEEDED:
+ break;
+ }
+
+ return true;
+ }
+
+ return false;
+}
It seems like some kind of ratelimit_log(limit, format, ...) function
might be useful so this is not copied everywhere.
Also wondering if there is a direct test for pointer acceleration never
initializes and perhaps you should do that test instead.
+
static inline void
evdev_process_relative(struct evdev_device *device,
struct input_event *e, uint64_t time)
@@ -645,6 +675,9 @@ evdev_process_relative(struct evdev_device *device,
struct normalized_coords wheel_degrees = { 0.0, 0.0 };
struct discrete_coords discrete = { 0.0, 0.0 };
+ if (evdev_reject_relative(device, e, time))
+ return;
+
switch (e-code) {
case REL_X:
if (device-pending_event != EVDEV_RELATIVE_MOTION)
I think it would look better and be clearer if you put the
evdev_reject_relative call in the REL_X and REL_Y cases of the switch (you
could also remove the test for REL_X/Y from the function itself).
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] evdev: drop relative x/y motion from a device not marked as pointer
On Tue, Aug 18, 2015 at 06:06:56PM -0700, Bill Spitzak wrote:
On Mon, Aug 17, 2015 at 10:08 PM, Peter Hutterer peter.hutte...@who-t.net
wrote:
A device with REL_X/Y and keys gets marked only as ID_INPUT_KEY,
initializes
as keyboard and then segfaults when we send x/y coordinates - pointer
acceleration never initializes.
Ignore the events and log a bug instead. This intentionally only papers
over
the underlying issue, let's wait for a real device to trigger this and then
look at the correct solution.
+static inline bool
+evdev_reject_relative(struct evdev_device *device,
+ const struct input_event *e,
+ uint64_t time)
+{
+ struct libinput *libinput = device-base.seat-libinput;
+
+ if ((e-code == REL_X || e-code == REL_Y)
+ (device-seat_caps EVDEV_DEVICE_POINTER) == 0) {
+ switch (ratelimit_test(device-nonpointer_rel_limit)) {
+ case RATELIMIT_PASS:
+ log_bug_libinput(libinput,
+REL_X/Y from device '%s', but
this device is not a pointer\n,
+device-devname);
+ break;
+ case RATELIMIT_THRESHOLD:
+ log_bug_libinput(libinput,
+REL_X/Y event flood from '%s'\n,
+device-devname);
+ break;
+ case RATELIMIT_EXCEEDED:
+ break;
+ }
+
+ return true;
+ }
+
+ return false;
+}
It seems like some kind of ratelimit_log(limit, format, ...) function
might be useful so this is not copied everywhere.
good idea, patch is in flight.
Also wondering if there is a direct test for pointer acceleration never
initializes and perhaps you should do that test instead.
not the same, we can concievably have a device that is a pointer but does
not have pointer acceleration. So checking for the seat caps here is better,
even though the crash is casued by the accel filters missing.
+
static inline void
evdev_process_relative(struct evdev_device *device,
struct input_event *e, uint64_t time)
@@ -645,6 +675,9 @@ evdev_process_relative(struct evdev_device *device,
struct normalized_coords wheel_degrees = { 0.0, 0.0 };
struct discrete_coords discrete = { 0.0, 0.0 };
+ if (evdev_reject_relative(device, e, time))
+ return;
+
switch (e-code) {
case REL_X:
if (device-pending_event != EVDEV_RELATIVE_MOTION)
I think it would look better and be clearer if you put the
evdev_reject_relative call in the REL_X and REL_Y cases of the switch (you
could also remove the test for REL_X/Y from the function itself).
it's a generic function even though it only handles one case at this
point, see evdev_reject_device() which is similar. The reason is simple:
mental capacity is limited, this approach separates the when do we reject
events from how do we handle events, you only need to care about one at a
time.
Cheers,
Peter
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput] evdev: drop relative x/y motion from a device not marked as pointer
A device with REL_X/Y and keys gets marked only as ID_INPUT_KEY, initializes
as keyboard and then segfaults when we send x/y coordinates - pointer
acceleration never initializes.
Ignore the events and log a bug instead. This intentionally only papers over
the underlying issue, let's wait for a real device to trigger this and then
look at the correct solution.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev.c | 35 +
src/evdev.h | 1 +
test/device.c | 155 ++
test/litest.c | 2 +
4 files changed, 193 insertions(+)
diff --git a/src/evdev.c b/src/evdev.c
index 9414d9d..97c007c 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -638,6 +638,36 @@ evdev_notify_axis(struct evdev_device *device,
discrete);
}
+static inline bool
+evdev_reject_relative(struct evdev_device *device,
+ const struct input_event *e,
+ uint64_t time)
+{
+ struct libinput *libinput = device-base.seat-libinput;
+
+ if ((e-code == REL_X || e-code == REL_Y)
+ (device-seat_caps EVDEV_DEVICE_POINTER) == 0) {
+ switch (ratelimit_test(device-nonpointer_rel_limit)) {
+ case RATELIMIT_PASS:
+ log_bug_libinput(libinput,
+REL_X/Y from device '%s', but this
device is not a pointer\n,
+device-devname);
+ break;
+ case RATELIMIT_THRESHOLD:
+ log_bug_libinput(libinput,
+REL_X/Y event flood from '%s'\n,
+device-devname);
+ break;
+ case RATELIMIT_EXCEEDED:
+ break;
+ }
+
+ return true;
+ }
+
+ return false;
+}
+
static inline void
evdev_process_relative(struct evdev_device *device,
struct input_event *e, uint64_t time)
@@ -645,6 +675,9 @@ evdev_process_relative(struct evdev_device *device,
struct normalized_coords wheel_degrees = { 0.0, 0.0 };
struct discrete_coords discrete = { 0.0, 0.0 };
+ if (evdev_reject_relative(device, e, time))
+ return;
+
switch (e-code) {
case REL_X:
if (device-pending_event != EVDEV_RELATIVE_MOTION)
@@ -2157,6 +2190,8 @@ evdev_device_create(struct libinput_seat *seat,
/* at most 5 SYN_DROPPED log-messages per 30s */
ratelimit_init(device-syn_drop_limit, s2us(30), 5);
+ /* at most 5 log-messages per 5s */
+ ratelimit_init(device-nonpointer_rel_limit, s2us(5), 5);
matrix_init_identity(device-abs.calibration);
matrix_init_identity(device-abs.usermatrix);
diff --git a/src/evdev.h b/src/evdev.h
index 9f026b8..e44a65d 100644
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -223,6 +223,7 @@ struct evdev_device {
int dpi; /* HW resolution */
struct ratelimit syn_drop_limit; /* ratelimit for SYN_DROPPED logging */
+ struct ratelimit nonpointer_rel_limit; /* ratelimit for REL_* events
from non-pointer devices */
uint32_t model_flags;
};
diff --git a/test/device.c b/test/device.c
index 59939d6..aff5ee2 100644
--- a/test/device.c
+++ b/test/device.c
@@ -1030,6 +1030,156 @@ START_TEST(device_udev_tag_synaptics_serial)
}
END_TEST
+START_TEST(device_nonpointer_rel)
+{
+ struct libevdev_uinput *uinput;
+ struct libinput *li;
+ struct libinput_device *device;
+ int i;
+
+ uinput = litest_create_uinput_device(test device,
+NULL,
+EV_KEY, KEY_A,
+EV_KEY, KEY_B,
+EV_REL, REL_X,
+EV_REL, REL_Y,
+-1);
+ li = litest_create_context();
+ device = libinput_path_add_device(li,
+ libevdev_uinput_get_devnode(uinput));
+ ck_assert(device != NULL);
+
+ litest_disable_log_handler(li);
+ for (i = 0; i 100; i++) {
+ libevdev_uinput_write_event(uinput, EV_REL, REL_X, 1);
+ libevdev_uinput_write_event(uinput, EV_REL, REL_Y, -1);
+ libevdev_uinput_write_event(uinput, EV_SYN, SYN_REPORT, 0);
+ libinput_dispatch(li);
+ }
+ litest_restore_log_handler(li);
+
+ libinput_unref(li);
+ libevdev_uinput_destroy(uinput);
+}
+END_TEST
+
+START_TEST(device_touchpad_rel)
+{
+ struct libevdev_uinput *uinput;
+ struct libinput *li;
+ struct libinput_device *device;
+ const struct input_absinfo abs[] = {
+ { ABS_X, 0, 10, 0, 0, 10 },
+ { ABS_Y, 0, 10, 0, 0, 10 },
+
