[PATCH libinput 1/8] evdev: Don't write out of bounds when mt slot is too large

[Jonas Ådahl]

On Wed, Mar 26, 2014 at 06:29:59PM +1000, Peter Hutterer wrote:
> On Wed, Mar 26, 2014 at 09:20:27AM +0100, Jonas Ådahl wrote:
> > On Wed, Mar 26, 2014 at 06:05:19PM +1000, Peter Hutterer wrote:
> > > On Tue, Mar 25, 2014 at 09:45:52PM +0100, Jonas Ådahl wrote:
> > > > Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> > > 
> > > patch looks good, but I do wonder if it'd be better to just dynamically
> > > allocate slots based on the number of touches. A quick glance shows we don't
> > > really need this a fixed length anyway, MAX_SLOTS is unnecessary.
> > 
> > We probably want a MAX_SLOTS anyway if we want to structure it as slot =
> > index in (dynamically allocated) array, in order to protect from misbehaving drivers.
> 
> the kernel filters slots outside the ranges, so we should be good for the
> normal use-case. using a min macro that caps it to num_slots when we assign
> the slot number should be enough too (that's essentially what libevdev
> does). You'll overwrite stuff in the same slot but by then you already have
> a misbehaving device anyway.

I'd prefer doing dynamic alloc array etc as a follow. Dropping or capping
are both "ok" I guess, although capping would be less code, so maybe
that's better. With the other avoid-double-touch-down it'd get ignored
if it'd collide with some other touch point anything anyway.

Jonas

> 
> Cheers,
>    Peter
> 
> > > > ---
> > > >  src/evdev.c | 13 +++++++++++++
> > > >  1 file changed, 13 insertions(+)
> > > > 
> > > > diff --git a/src/evdev.c b/src/evdev.c
> > > > index 72e4086..ff8b27a 100644
> > > > --- a/src/evdev.c
> > > > +++ b/src/evdev.c
> > > > @@ -132,6 +132,9 @@ evdev_flush_pending_event(struct evdev_device *device, uint32_t time)
> > > >  		if (!(device->seat_caps & EVDEV_DEVICE_TOUCH))
> > > >  			break;
> > > >  
> > > > +		if (slot >= MAX_SLOTS)
> > > > +			break;
> > > > +
> > > >  		seat_slot = ffs(~seat->slot_map) - 1;
> > > >  		device->mt.slots[slot].seat_slot = seat_slot;
> > > >  
> > > > @@ -148,6 +151,9 @@ evdev_flush_pending_event(struct evdev_device *device, uint32_t time)
> > > >  		if (!(device->seat_caps & EVDEV_DEVICE_TOUCH))
> > > >  			break;
> > > >  
> > > > +		if (slot >= MAX_SLOTS)
> > > > +			break;
> > > > +
> > > >  		seat_slot = device->mt.slots[slot].seat_slot;
> > > >  		x = li_fixed_from_int(device->mt.slots[slot].x);
> > > >  		y = li_fixed_from_int(device->mt.slots[slot].y);
> > > > @@ -161,6 +167,9 @@ evdev_flush_pending_event(struct evdev_device *device, uint32_t time)
> > > >  		if (!(device->seat_caps & EVDEV_DEVICE_TOUCH))
> > > >  			break;
> > > >  
> > > > +		if (slot >= MAX_SLOTS)
> > > > +			break;
> > > > +
> > > >  		seat_slot = device->mt.slots[slot].seat_slot;
> > > >  
> > > >  		if (seat_slot == -1)
> > > > @@ -300,11 +309,15 @@ evdev_process_touch(struct evdev_device *device,
> > > >  			device->pending_event = EVDEV_ABSOLUTE_MT_UP;
> > > >  		break;
> > > >  	case ABS_MT_POSITION_X:
> > > > +		if (device->mt.slot >= MAX_SLOTS)
> > > > +			break;
> > > >  		device->mt.slots[device->mt.slot].x = e->value;
> > > >  		if (device->pending_event == EVDEV_NONE)
> > > >  			device->pending_event = EVDEV_ABSOLUTE_MT_MOTION;
> > > >  		break;
> > > >  	case ABS_MT_POSITION_Y:
> > > > +		if (device->mt.slot >= MAX_SLOTS)
> > > > +			break;
> > > >  		device->mt.slots[device->mt.slot].y = e->value;
> > > >  		if (device->pending_event == EVDEV_NONE)
> > > >  			device->pending_event = EVDEV_ABSOLUTE_MT_MOTION;
> > > > -- 
> > > > 1.8.3.2
> > > > 
> > > > _______________________________________________
> > > > wayland-devel mailing list
> > > > wayland-devel at lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > > >