Testing result shows that the driver may start processing data before all the tools associated with the same logical port are enabled or after one of the tools is removed during hotplugging process. This causes unenabled tools to be accessed sometime, which leads to a driver crash.
Added a counter (wcmNumTools) to keep track of the enabled the tools so we don't let the driver read the data if not all the tools are enabled. The fact that all tools associated with the same logical port are initialized first before they are enabled one by one is the foundation that we could use a counter to cover both static defined (in xorg.conf) and hotplugged/dynamic allocated devices with the code. Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net> Signed-off-by: Ping Cheng <pingli...@gmail.com> --- Peter, I added your "if (!pInfo->dev || !pInfo->dev->enabled)" check as a safeguard for the crash issue. Since it was intrduced to deal with the same issue, I merged it in this patch. I also added your Reviewed-by as you made privately yesterday. Let me know if you do not agree. --- src/wcmCommon.c | 5 ++--- src/xf86Wacom.c | 19 +++++++++++++++---- src/xf86WacomDefs.h | 5 +++++ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/wcmCommon.c b/src/wcmCommon.c index dcf792c..8c3afd8 100644 --- a/src/wcmCommon.c +++ b/src/wcmCommon.c @@ -1226,10 +1226,9 @@ static void commonDispatchDevice(WacomCommonPtr common, unsigned int channel, /* Tool on the tablet when driver starts. This sometime causes * access errors to the device */ - if (!miPointerGetScreen(pDev->dev)) + if (!pDev || !pDev->dev || !pDev->dev->enabled) { - xf86Msg(X_ERROR, "wcmEvent: Wacom driver can not get Current Screen ID\n"); - xf86Msg(X_ERROR, "Please remove Wacom tool from the tablet and bring it back again.\n"); + xf86Msg(X_ERROR, "wcmEvent: tool not initialized yet. Skipping event. \n"); return; } diff --git a/src/xf86Wacom.c b/src/xf86Wacom.c index e8c4710..03eb8df 100644 --- a/src/xf86Wacom.c +++ b/src/xf86Wacom.c @@ -740,9 +740,17 @@ static int wcmReady(InputInfoPtr pInfo) static void wcmDevReadInput(InputInfoPtr pInfo) { + WacomDevicePtr priv = (WacomDevicePtr)pInfo->private; + WacomCommonPtr common = priv->common; int loop=0; #define MAX_READ_LOOPS 10 + /* don't read the data before all tools are enabled, or at + * least one tool has been removed on the same logical port. + */ + if (common->fd_refs != common->wcmNumTools) + return; + /* move data until we exhaust the device */ for (loop=0; loop < MAX_READ_LOOPS; ++loop) { @@ -757,8 +765,6 @@ static void wcmDevReadInput(InputInfoPtr pInfo) /* report how well we're doing */ if (loop > 0) { - WacomDevicePtr priv = (WacomDevicePtr)pInfo->private; - if (loop >= MAX_READ_LOOPS) DBG(1, priv, "Can't keep up!!!\n"); else @@ -880,9 +886,9 @@ static void wcmDevClose(InputInfoPtr pInfo) static int wcmDevProc(DeviceIntPtr pWcm, int what) { InputInfoPtr pInfo = (InputInfoPtr)pWcm->public.devicePrivate; -#ifdef DEBUG WacomDevicePtr priv = (WacomDevicePtr)pInfo->private; -#endif + WacomCommonPtr common = priv->common; + Status rc = !Success; DBG(2, priv, "BEGIN dev=%p priv=%p " @@ -900,9 +906,14 @@ static int wcmDevProc(DeviceIntPtr pWcm, int what) case DEVICE_INIT: if (!wcmDevInit(pWcm)) goto out; + + /* All tools on the same port are initialized one by one + * here before any of them are enabled */ + common->wcmNumTools++; break; case DEVICE_ON: + /* common->fd_refs is updated for each tool in wcmDevOpen */ if (!wcmDevOpen(pWcm)) goto out; xf86AddEnabledDevice(pInfo); diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h index 91adf72..5d7a70f 100644 --- a/src/xf86WacomDefs.h +++ b/src/xf86WacomDefs.h @@ -477,6 +477,11 @@ struct _WacomCommonRec void *private; /* backend-specific information */ + int wcmNumTools; /* Number of tools associated with a logical port. + * It is a counter to prevent the process of data before + * the last tool associated with the same port is enabled + */ + WacomToolPtr wcmTool; /* List of unique tools */ /* DO NOT TOUCH THIS. use wcmRefCommon() instead */ -- 1.7.4 ------------------------------------------------------------------------------ The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE: Pinpoint memory and threading errors before they happen. Find and fix more than 250 security defects in the development cycle. Locate bottlenecks in serial and parallel code that limit performance. http://p.sf.net/sfu/intel-dev2devfeb _______________________________________________ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel