[PATCH v2 synaptics] Fix stack smash in clickpad_guess_clickfingers()

[Peter Hutterer]

On Fri, Apr 26, 2013 at 09:13:30AM +0200, walter harms wrote:
> 
> 
> Am 26.04.2013 02:12, schrieb Peter Hutterer:
> > Apple Magic Trackpad can report 16 slots. In clickpad_guess_clickfingers()
> > the array allocated on the stack contains only 10 slots.
> > As (.num_mt_mask == .num_slots), the function writes out of the bounds
> > of close_point.
> > 
> > Use a size 32 bitmask instead and warn if we ever get past 32 touchpoints.
> > 
> > This fixes:
> > https://bugzilla.redhat.com/show_bug.cgi?id=952221
> > 
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > Reported-by: Benjamin Tissoires <benjamin.tissoires at redhat.com>
> > ---
> > close_point only has values 1 or 0, so we can just use a bitmask here. and
> > uint32_t gives us up to 32 touchpoints, which should cover us for a while.
> > 
> >  src/synaptics.c | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/synaptics.c b/src/synaptics.c
> > index f0a8269..2de05d4 100644
> > --- a/src/synaptics.c
> > +++ b/src/synaptics.c
> > @@ -2453,10 +2453,11 @@ clickpad_guess_clickfingers(SynapticsPrivate * priv,
> >                              struct SynapticsHwState *hw)
> >  {
> >      int nfingers = 0;
> > -    char close_point[SYNAPTICS_MAX_TOUCHES] = { 0 };    /* 1 for each point close
> > -                                                           to another one */
> > +    uint32_t close_point = 0; /* 1 bit for each point close to another one */
> >      int i, j;
> >  
> > +    BUG_RETURN_VAL(hw->num_mt_mask > sizeof(close_point) * 8, 0);
> > +
> 
> nitpicking:
> To make that perfect you could use  CHAR_BIT from limits.h here.

heh, didn't know about that one, thanks. though if sizeof(char) is ever
anything != 8 then we are screwed in more ways than clickfinger not working
correctly...

Cheers,
  Peter

> 
> re,
>  wh
> 
> >      for (i = 0; i < hw->num_mt_mask - 1; i++) {
> >          ValuatorMask *f1;
> >  
> > @@ -2488,14 +2489,16 @@ clickpad_guess_clickfingers(SynapticsPrivate * priv,
> >               * size. Good luck. */
> >              if (abs(x1 - x2) < (priv->maxx - priv->minx) * .3 &&
> >                  abs(y1 - y2) < (priv->maxy - priv->miny) * .3) {
> > -                close_point[j] = 1;
> > -                close_point[i] = 1;
> > +                close_point |= (1 << j);
> > +                close_point |= (1 << i);
> >              }
> >          }
> >      }
> >  
> > -    for (i = 0; i < SYNAPTICS_MAX_TOUCHES; i++)
> > -        nfingers += close_point[i];
> > +    while (close_point > 0) {
> > +        nfingers += close_point & 0x1;
> > +        close_point >>= 1;
> > +    }
> >  
> >      return nfingers;
> >  }
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>