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

Reply via email to