[PATCH libevdev 2/5] Dynamically allocate the slot values
Benjamin Tissoires
benjamin.tissoires at gmail.com
Mon Mar 3 07:37:50 PST 2014
On Thu, Feb 27, 2014 at 1:27 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> Instead of relying on a static MAX_SLOTS array, allocated it based on the
> number of slots we have on the device. The previous checks for MAX_SLOTS were
> incomplete, causing out-of-bound reads.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> libevdev/libevdev-int.h | 2 +-
> libevdev/libevdev.c | 41 +++++++++++++++++++++++++++++++----------
> 2 files changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/libevdev/libevdev-int.h b/libevdev/libevdev-int.h
> index 847fe56..42141ca 100644
> --- a/libevdev/libevdev-int.h
> +++ b/libevdev/libevdev-int.h
> @@ -95,7 +95,7 @@ struct libevdev {
> unsigned long led_values[NLONGS(LED_CNT)];
> unsigned long sw_values[NLONGS(SW_CNT)];
> struct input_absinfo abs_info[ABS_CNT];
> - int mt_slot_vals[MAX_SLOTS][ABS_MT_CNT];
> + int *mt_slot_vals; /* [num_slots * ABS_MT_CNT] */
> int num_slots; /**< valid slots in mt_slot_vals */
> int current_slot;
> int rep_values[REP_CNT];
> diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c
> index 36359d4..4f5062d 100644
> --- a/libevdev/libevdev.c
> +++ b/libevdev/libevdev.c
> @@ -38,6 +38,21 @@
>
> static int sync_mt_state(struct libevdev *dev, int create_events);
>
> +static inline int*
> +slot_value(const struct libevdev *dev, int slot, int axis)
> +{
> + if (slot > dev->num_slots) {
> + log_bug("Slot %d exceeds number of slots (%d)\n", slot, dev->num_slots);
> + slot = 0;
> + }
> + if (axis < ABS_MT_MIN || axis > ABS_MT_MAX) {
> + log_bug("MT axis %d is outside the valid range [%d,%d]\n",
> + axis, ABS_MT_MIN, ABS_MT_MAX);
> + axis = ABS_MT_MIN;
> + }
> + return &dev->mt_slot_vals[slot * ABS_MT_CNT + axis - ABS_MT_MIN];
> +}
> +
I am a little bit puzzled by this one.
If the slot is not valid or if the axis is not valid, we return a
random address, which may be used to overwrite actual data.
Also, in some places later, the code actually checks if slot is valid,
which means that we do not have the log_bug() message there.
How about splitting this function in 2 (or 3), 1 (or 2) to check the
validity of slots and axis, and the last would return the pointer,
(maybe return NULL if there is an error in slots or axis)?
Cheers,
Benjamin
> static int
> init_event_queue(struct libevdev *dev)
> {
> @@ -136,6 +151,7 @@ libevdev_reset(struct libevdev *dev)
> free(dev->name);
> free(dev->phys);
> free(dev->uniq);
> + free(dev->mt_slot_vals);
> memset(dev, 0, sizeof(*dev));
> dev->fd = -1;
> dev->initialized = false;
> @@ -187,6 +203,7 @@ libevdev_free(struct libevdev *dev)
> free(dev->name);
> free(dev->phys);
> free(dev->uniq);
> + free(dev->mt_slot_vals);
> queue_free(dev);
> free(dev);
> }
> @@ -369,6 +386,11 @@ libevdev_set_fd(struct libevdev* dev, int fd)
> if (i == ABS_MT_SLOT &&
> !libevdev_has_event_code(dev, EV_ABS, ABS_MT_SLOT - 1)) {
> dev->num_slots = abs_info.maximum + 1;
> + dev->mt_slot_vals = calloc(dev->num_slots * ABS_MT_CNT, sizeof(int));
> + if (!dev->mt_slot_vals) {
> + rc = -ENOMEM;
> + goto out;
> + }
> dev->current_slot = abs_info.value;
> }
>
> @@ -579,14 +601,14 @@ sync_mt_state(struct libevdev *dev, int create_events)
> if (!libevdev_has_event_code(dev, EV_ABS, j))
> continue;
>
> - if (dev->mt_slot_vals[i][jdx] == mt_state[jdx].val[i])
> + if (*slot_value(dev, i, j) == mt_state[jdx].val[i])
> continue;
>
> if (create_events) {
> ev = queue_push(dev);
> init_event(dev, ev, EV_ABS, j, mt_state[jdx].val[i]);
> }
> - dev->mt_slot_vals[i][jdx] = mt_state[jdx].val[i];
> + *slot_value(dev, i, j) = mt_state[jdx].val[i];
> }
> }
>
> @@ -667,14 +689,14 @@ update_mt_state(struct libevdev *dev, const struct input_event *e)
> /* sync abs_info with the current slot values */
> for (i = ABS_MT_SLOT + 1; i <= ABS_MT_MAX; i++) {
> if (libevdev_has_event_code(dev, EV_ABS, i))
> - dev->abs_info[i].value = dev->mt_slot_vals[dev->current_slot][i - ABS_MT_MIN];
> + dev->abs_info[i].value = *slot_value(dev, dev->current_slot, i);
> }
>
> return 0;
> } else if (dev->current_slot == -1)
> return 1;
>
> - dev->mt_slot_vals[dev->current_slot][e->code - ABS_MT_MIN] = e->value;
> + *slot_value(dev, dev->current_slot, e->code) = e->value;
>
> return 0;
> }
> @@ -1070,13 +1092,13 @@ libevdev_get_slot_value(const struct libevdev *dev, unsigned int slot, unsigned
> if (!libevdev_has_event_type(dev, EV_ABS) || !libevdev_has_event_code(dev, EV_ABS, code))
> return 0;
>
> - if (dev->num_slots < 0 || slot >= (unsigned int)dev->num_slots || slot >= MAX_SLOTS)
> + if (dev->num_slots < 0 || slot >= (unsigned int)dev->num_slots)
> return 0;
>
> if (code > ABS_MT_MAX || code < ABS_MT_MIN)
> return 0;
>
> - return dev->mt_slot_vals[slot][code - ABS_MT_MIN];
> + return *slot_value(dev, slot, code);
> }
>
> LIBEVDEV_EXPORT int
> @@ -1085,7 +1107,7 @@ libevdev_set_slot_value(struct libevdev *dev, unsigned int slot, unsigned int co
> if (!libevdev_has_event_type(dev, EV_ABS) || !libevdev_has_event_code(dev, EV_ABS, code))
> return -1;
>
> - if (dev->num_slots == -1 || slot >= (unsigned int)dev->num_slots || slot >= MAX_SLOTS)
> + if (dev->num_slots == -1 || slot >= (unsigned int)dev->num_slots)
> return -1;
>
> if (code > ABS_MT_MAX || code < ABS_MT_MIN)
> @@ -1097,8 +1119,7 @@ libevdev_set_slot_value(struct libevdev *dev, unsigned int slot, unsigned int co
> dev->current_slot = value;
> }
>
> - dev->mt_slot_vals[slot][code - ABS_MT_MIN] = value;
> -
> + *slot_value(dev, slot, code) = value;
>
> return 0;
> }
> @@ -1109,7 +1130,7 @@ libevdev_fetch_slot_value(const struct libevdev *dev, unsigned int slot, unsigne
> if (libevdev_has_event_type(dev, EV_ABS) &&
> libevdev_has_event_code(dev, EV_ABS, code) &&
> dev->num_slots >= 0 &&
> - slot < (unsigned int)dev->num_slots && slot < MAX_SLOTS) {
> + slot < (unsigned int)dev->num_slots) {
> *value = libevdev_get_slot_value(dev, slot, code);
> return 1;
> } else
> --
> 1.8.4.2
>
> _______________________________________________
> Input-tools mailing list
> Input-tools at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/input-tools
More information about the Input-tools
mailing list