On Wed, Mar 2, 2011 at 10:06 PM, Peter Hutterer <peter.hutte...@who-t.net>wrote:

> There is a small time window where a device may try to send an event even
> though it is not fully setup to send events yet, causing a server crash.
>
> This window opens when the tool is added to the list of devices in
> wcmParseOptions() and closes with the server calling xf86ActivateDevice().
> If an event for a dependent device is processed during that time, the tool
> will be available but the device pointer is still invalid.
>
> Crash can be reproduced by putting a breakpoint after wcmParseOptions() for
> the eraser, then generating events with the eraser. These will cause the
> tool to dereference tool->device->dev, which is uninitialized.
>
> Work around this with a simple "enabled" flag that is set whenever the tool
> is actually enabled.
>

I didn't test anything yet.  As I've shared with you before, I don't think a
local enabled flag is enough. Something common to all the tools associated
with the same port is needed since we should not access the fd before all
tools are ready.

+       if (!tool->enabled) {
need to be changed to something like

+       if (!common->enabled) {

And commmon->enabled is set to true oly when the last hotplugged tool is
enabled. It will be set to false as soon as one tool is disabled.

Let me know if I've lost you.

Ping


>
> Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
> ---
>  src/wcmCommon.c     |    9 ++++-----
>  src/xf86Wacom.c     |   19 +++++++++++++++++++
>  src/xf86WacomDefs.h |    1 +
>  3 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index 591a20e..2491e4b 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -1163,17 +1163,16 @@ static void commonDispatchDevice(WacomCommonPtr
> common, unsigned int channel,
>                return;
>        }
>
> -       pInfo = tool->device;
> -       DBG(11, common, "tool id=%d for %s\n", ds->device_type,
> pInfo->name);
> -
>        /* Tool on the tablet when driver starts. This sometime causes
>         * access errors to the device */
> -       if (!pInfo || !pInfo->dev || !pInfo->dev->enabled)
> -       {
> +       if (!tool->enabled) {
>                xf86Msg(X_ERROR, "wcmEvent: tool not initialized yet.
> Skipping event. \n");
>                return;
>        }
>
> +       pInfo = tool->device;
> +       DBG(11, common, "tool id=%d for %s\n", ds->device_type,
> pInfo->name);
> +
>        filtered = pChannel->valid.state;
>
>        /* Device transformations come first */
> diff --git a/src/xf86Wacom.c b/src/xf86Wacom.c
> index 0962e1d..43d64a4 100644
> --- a/src/xf86Wacom.c
> +++ b/src/xf86Wacom.c
> @@ -755,6 +755,23 @@ static void wcmDevClose(InputInfoPtr pInfo)
>        }
>  }
>
> +static void wcmEnableDisableTool(DeviceIntPtr dev, Bool enable)
> +{
> +       InputInfoPtr    pInfo   = dev->public.devicePrivate;
> +       WacomDevicePtr  priv    = pInfo->private;
> +       WacomToolPtr    tool    = priv->tool;
> +
> +       tool->enabled = enable;
> +}
> +
> +static void wcmEnableTool(DeviceIntPtr dev)
> +{
> +       wcmEnableDisableTool(dev, TRUE);
> +}
> +static void wcmDisableTool(DeviceIntPtr dev)
> +{
> +       wcmEnableDisableTool(dev, FALSE);
> +}
>
>  
> /*****************************************************************************
>  * wcmDevProc --
>  *   Handle the initialization, etc. of a wacom tablet. Called by the
> server
> @@ -791,12 +808,14 @@ static int wcmDevProc(DeviceIntPtr pWcm, int what)
>                case DEVICE_ON:
>                        if (!wcmDevOpen(pWcm))
>                                goto out;
> +                       wcmEnableTool(pWcm);
>                        xf86AddEnabledDevice(pInfo);
>                        pWcm->public.on = TRUE;
>                        break;
>
>                case DEVICE_OFF:
>                case DEVICE_CLOSE:
> +                       wcmDisableTool(pWcm);
>                        if (pInfo->fd >= 0)
>                        {
>                                xf86RemoveEnabledDevice(pInfo);
> diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
> index db83ddb..2a6e4ff 100644
> --- a/src/xf86WacomDefs.h
> +++ b/src/xf86WacomDefs.h
> @@ -492,6 +492,7 @@ struct _WacomTool
>
>        int typeid; /* Tool type */
>        int serial; /* Serial id, 0 == no serial id */
> +       Bool enabled;
>
>        InputInfoPtr device; /* The InputDevice connected to this tool */
>  };
> --
> 1.7.4
>
>
>
> ------------------------------------------------------------------------------
> Free Software Download: Index, Search & Analyze Logs and other IT data in
> Real-Time with Splunk. Collect, index and harness all the fast moving IT
> data
> generated by your applications, servers and devices whether physical,
> virtual
> or in the cloud. Deliver compliance at lower cost and gain new business
> insights. http://p.sf.net/sfu/splunk-dev2dev
> _______________________________________________
> Linuxwacom-devel mailing list
> Linuxwacom-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
>
------------------------------------------------------------------------------
Free Software Download: Index, Search & Analyze Logs and other IT data in 
Real-Time with Splunk. Collect, index and harness all the fast moving IT data 
generated by your applications, servers and devices whether physical, virtual
or in the cloud. Deliver compliance at lower cost and gain new business 
insights. http://p.sf.net/sfu/splunk-dev2dev 
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to