On Wed, Mar 2, 2011 at 10:06 PM, Peter Hutterer <[email protected]>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.
>
What's the plan for this patchset? Is it possible to make a common->enabled
so we can disable the Read in xf86Wacom.c or at least stop process the data
as what you did in this patch for all tools on the same port while
initializing the tools?
I am willing to ack the patchset. But I hope we have considered all existing
issues that we could cover. priv->enabled feels weak here.
Another thing that I didn't want to bring up was: for those devices that
support both touch and pen, we probably will have a bigger challenge. Pen
and touch are on different logical port. But they are still the same
physical device. To stop tools on one port accessing the tablet while the
tools on the other port initializing is a job beyond common->disabled. We
will need to link the two logical ports together by product ID plus
something else, in case there are more than one identical tablets connected
to the system.
Hopefully we can come up with a solution when we need to worry about it
later.
Ping
> Signed-off-by: Peter Hutterer <[email protected]>
> ---
> 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
>
------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
