On Mon, Mar 15, 2010 at 10:19:29PM -0700, Ping Cheng wrote: > On Sun, Mar 14, 2010 at 11:55 PM, Peter Hutterer > <[email protected]>wrote: > > > 5780e59c4 "Take scroll buttons into accounts when allocating buttons." > > added > > a conditional to always have at least 7 buttons for devices with more than > > 3 > > buttons. The exact number is nbuttons + 4, but for serial devices nbuttons > > is always WCM_MAX_MOUSE_BUTTONS - resulting in a segfault later on when > > arrays are accessed OOB. > > > > Reported-by: Ping Cheng <[email protected]> > > Signed-off-by: Peter Hutterer <[email protected]> > > --- > > Ping, I think this should fix your crash with the isdv4 branch. > > > > > Your patch (with "if (nbuttons < WCM_MAX_BUTTONS)" instead of "<=" though) > does fix the crash issue on my ISDV4 with touch system. However, I made a > new one with the following commit comments: > > ------------------- > The workaround that supports at least 7 buttons could cause > nbbuttons larger than WCM_MAX_BUTTONS, the maximum number of > buttons that is supported by the driver. This patch keeps > nbbuttons in its range. > > This patch also fixes a driver crash issue on ISDV4 with touch > systems that may be caused by nbbuttons being larger than > WCM_MAX_BUTTONS. The "may be" is backed up by the following testing result: > > 1. linuxwacom-0.8.5-11 has the same block of code. However, > running linuxwacom-0.8.5-11 on the same ISDV4 system (Xorg 1.6.0) > without this patch doesn't have the issue. > > 2. Running xf86-input-wacom without this patch on a pen only > ISDV4 system doesn't have the issue either (Thomas Jaeger reported > the same positive testing result too). > > So, there must be some other issues behind the crash that we don't > understand yet. > ------------------------ > > Patch is attached. Please let me know if it makes sense to you or not. > > Ping > > > > > > src/xf86Wacom.c | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/src/xf86Wacom.c b/src/xf86Wacom.c > > index 8656e63..3809dab 100644 > > --- a/src/xf86Wacom.c > > +++ b/src/xf86Wacom.c > > @@ -608,7 +608,8 @@ static int wcmRegisterX11Devices (LocalDevicePtr local) > > > > /* if more than 3 buttons, offset by the four scroll buttons, > > * otherwise, alloc 7 buttons for scroll wheel. */ > > - nbbuttons = (nbbuttons > 3) ? nbbuttons + 4 : 7; > > + if (nbuttons <= WCM_MAX_BUTTONS) > > + nbbuttons = (nbbuttons > 3) ? nbbuttons + 4 : 7; > > nbkeys = nbbuttons; /* Same number of keys since any button > > may be > > * configured as an either mouse button > > or key */ > > > > -- > > 1.6.6.1 > > > >
> From aa1aa20942cac09c4ce84a57a6d840caeaddf521 Mon Sep 17 00:00:00 2001 > From: Ping Cheng <[email protected]> > Date: Mon, 15 Mar 2010 21:14:41 -0700 > Subject: [PATCH] nbbuttons should be <= WCM_MAX_BUTTONS > > The workaround that supports at least 7 buttons could cause > nbbuttons larger than WCM_MAX_BUTTONS, the maximum number of > buttons that is supported by the driver. This patch keeps > nbbuttons in its range. > > This patch also fixes a driver crash issue on ISDV4 with touch > systems that may be caused by nbbuttons being larger than > WCM_MAX_BUTTONS. The "may be" is backed up by the following > testing result: > > 1. linuxwacom-0.8.5-11 has the same block of code. However, > running linuxwacom-0.8.5-11 on the same ISDV4 system (Xorg 1.6.0) > without this patch doesn't have the issue. > > 2. Running xf86-input-wacom without this patch on a pen only > ISDV4 system doesn't have the issue either (Thomas Jaeger reported > the same positive testing result too). > > So, there must be some other issues behind the crash that we don't > understand yet. > > Signed-off-by: Ping Cheng <[email protected]> > --- > src/xf86Wacom.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/src/xf86Wacom.c b/src/xf86Wacom.c > index 8685805..1f61503 100644 > --- a/src/xf86Wacom.c > +++ b/src/xf86Wacom.c > @@ -606,6 +606,11 @@ static int wcmRegisterX11Devices (LocalDevicePtr local) > /* if more than 3 buttons, offset by the four scroll buttons, > * otherwise, alloc 7 buttons for scroll wheel. */ > nbbuttons = (nbbuttons > 3) ? nbbuttons + 4 : 7; > + > + /* make sure nbbuttons stays in the range */ > + if (nbbuttons > WCM_MAX_BUTTONS) > + nbbuttons = WCM_MAX_BUTTONS; > + > nbkeys = nbbuttons; /* Same number of keys since any button may > be > * configured as an either mouse button or > key */ > > -- > 1.6.6.1 merged, thanks. Cheers, Peter ------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev _______________________________________________ Linuxwacom-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
