[PATCH libevdev 3/3] Check max to see if an event type is valid

[David Herrmann]

Hi Peter

On Thu, Oct 24, 2013 at 7:14 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> There's a gap in the range between EV_SW and EV_LED. Trying to enable one
> of those bits will segfault.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>  libevdev/libevdev.c            | 16 ++++++++++++++--
>  test/test-libevdev-has-event.c |  6 ++++++
>  2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c
> index 7bebe32..ca57e26 100644
> --- a/libevdev/libevdev.c
> +++ b/libevdev/libevdev.c
> @@ -1139,12 +1139,18 @@ libevdev_set_abs_info(struct libevdev *dev, unsigned int code, const struct inpu
>  LIBEVDEV_EXPORT int
>  libevdev_enable_event_type(struct libevdev *dev, unsigned int type)
>  {
> +       int max;
> +
>         if (type > EV_MAX)
>                 return -1;
>
>         if (libevdev_has_event_type(dev, type))
>                 return 0;
>
> +       max = libevdev_event_type_get_max(type);
> +       if (max == -1)
> +               return -1;
> +
>         set_bit(dev->bits, type);
>
>         if (type == EV_REP) {
> @@ -1158,9 +1164,15 @@ libevdev_enable_event_type(struct libevdev *dev, unsigned int type)
>  LIBEVDEV_EXPORT int
>  libevdev_disable_event_type(struct libevdev *dev, unsigned int type)
>  {
> +       int max;
> +
>         if (type > EV_MAX || type == EV_SYN)
>                 return -1;
>
> +       max = libevdev_event_type_get_max(type);
> +       if (max == -1)
> +               return -1;
> +
>         clear_bit(dev->bits, type);
>
>         return 0;
> @@ -1192,7 +1204,7 @@ libevdev_enable_event_code(struct libevdev *dev, unsigned int type,
>
>         max = type_to_mask(dev, type, &mask);
>
> -       if (code > max)
> +       if (code > max || (int)max == -1)

type_to_mask() returns an "int" so why is "max" unsigned? The cast
here looks stupid this way. I'd prefer this:

       int max;

       if (max < 0 || code > (unsigned)max)

To me it's clearer that the <0 + (unsigned) protects against integer
overflow. But maybe that's just arguing about coding-style. Both works
so:

Reviewed-by: David Herrmann <dh.herrmann at gmail.com>

Thanks
David

>                 return -1;
>
>         set_bit(mask, code);
> @@ -1219,7 +1231,7 @@ libevdev_disable_event_code(struct libevdev *dev, unsigned int type, unsigned in
>
>         max = type_to_mask(dev, type, &mask);
>
> -       if (code > max)
> +       if (code > max || (int)max == -1)
>                 return -1;
>
>         clear_bit(mask, code);
> diff --git a/test/test-libevdev-has-event.c b/test/test-libevdev-has-event.c
> index 5af3530..3aca23b 100644
> --- a/test/test-libevdev-has-event.c
> +++ b/test/test-libevdev-has-event.c
> @@ -775,6 +775,9 @@ START_TEST(test_device_enable_bit_invalid)
>         ck_assert_int_eq(libevdev_enable_event_code(dev, EV_ABS, ABS_MAX + 1, &abs), -1);
>         ck_assert_int_eq(libevdev_enable_event_code(dev, EV_MAX + 1, ABS_MAX + 1, &abs), -1);
>         ck_assert_int_eq(libevdev_enable_event_type(dev, EV_MAX + 1), -1);
> +       /* there's a gap between EV_SW and EV_LED */
> +       ck_assert_int_eq(libevdev_enable_event_type(dev, EV_LED - 1), -1);
> +       ck_assert_int_eq(libevdev_enable_event_code(dev, EV_LED - 1, 0, NULL), -1);
>
>         ck_assert_int_eq(libevdev_enable_event_code(dev, EV_ABS, ABS_Y, NULL), -1);
>         ck_assert_int_eq(libevdev_enable_event_code(dev, EV_REP, REP_DELAY, NULL), -1);
> @@ -843,6 +846,9 @@ START_TEST(test_device_disable_bit_invalid)
>         rc = test_create_abs_device(&uidev, &dev, 1, &abs, -1);
>         ck_assert_msg(rc == 0, "Failed to create uinput device: %s", strerror(-rc));
>
> +       /* there's a gap between EV_SW and EV_LED */
> +       ck_assert_int_eq(libevdev_disable_event_type(dev, EV_LED - 1), -1);
> +       ck_assert_int_eq(libevdev_disable_event_code(dev, EV_LED - 1, 0), -1);
>         ck_assert_int_eq(libevdev_disable_event_code(dev, EV_ABS, ABS_MAX + 1), -1);
>         ck_assert_int_eq(libevdev_disable_event_code(dev, EV_MAX + 1, ABS_MAX + 1), -1);
>         ck_assert_int_eq(libevdev_disable_event_type(dev, EV_MAX + 1), -1);
> --
> 1.8.3.1
>
> _______________________________________________
> Input-tools mailing list
> Input-tools at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/input-tools