[PATCH synaptics] Avoid out-of-bounds access by running num_active_touches < 0 (#49439)

[Peter Hutterer]

On Mon, May 14, 2012 at 10:02:24AM -0700, Chase Douglas wrote:
> On 05/10/2012 07:30 PM, Peter Hutterer wrote:
> > If a touch is active during driver init, the slot will be set to
> > SLOTSTATE_CLOSE when it finishes. That could decrease num_active_touches to
> > less than 0, causing out-of-bounds access.
> > 
> > X.Org Bug 49439 <http://bugs.freedesktop.org/show_bug.cgi?id=49439>
> > 
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> >  src/synaptics.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/synaptics.c b/src/synaptics.c
> > index e792977..d98ba71 100644
> > --- a/src/synaptics.c
> > +++ b/src/synaptics.c
> > @@ -2624,7 +2624,8 @@ UpdateTouchState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
> >                      priv->open_slots[j] = priv->open_slots[j + 1];
> >              }
> >  
> > -            priv->num_active_touches--;
> > +            if (priv->num_active_touches > 0)
> > +                priv->num_active_touches--;
> >          }
> >      }
> >  
> 
> Please add an error message if priv->num_active_touches <= 0. It will
> tell us when there is a bug, because that should never happen. With that
> change:

I've squashed this in:

diff --git a/src/synaptics.c b/src/synaptics.c
index d98ba71..ff47857 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -2624,6 +2624,7 @@ UpdateTouchState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
                     priv->open_slots[j] = priv->open_slots[j + 1];
             }
 
+            BUG_WARN(priv->num_active_touches == 0);
             if (priv->num_active_touches > 0)
                 priv->num_active_touches--;
         }

Cheers,
  Peter

> 
> Reviewed-by: Chase Douglas <chase.douglas at canonical.com>
> 
> I was able to reproduce this with a synaptics trackpad, but not with
> Apple trackpads. mtdev gracefully handles pre-existing touches and does
> not send any events for them. The bcm5974 and hid-magicmouse kernel
> drivers for Apple trackpads use the evdev MT protocol A, so mtdev is
> cleaning the events. However, the synaptics trackpad driver uses
> protocol B, and mtdev is just passing the events through.
> 
> I have a fix for the real bug in eventcomm.c. Please see the patch I
> will send out shortly.
> 
> -- Chase