[PATCH v2] Allow driver to call DeleteInputDeviceRequest during UnInit
Simon Thum
simon.thum at gmx.de
Fri Jan 15 01:40:02 PST 2010
Oldřich Jedlička wrote:
> When the input driver (like xf86-input-wacom) removes it's devices
> during a call to UnInit, the CloseDownDevices() cannot handle it. The
> "next" variable can become a pointer to freed memory.
>
> The patch introduces order-independent device freeing mechanism by
> remembering the already freed device ids. The devices can reorder any
> time during freeing. No device will be double-freed - if the removing
> failed for any reason; some implementations of DeleteInputDeviceRequest
> don't free the devices already.
>
> Signed-off-by: Oldřich Jedlička <oldium.pro at seznam.cz>
> ---
> dix/devices.c | 42 +++++++++++++++++++++++++++++++-----------
> 1 files changed, 31 insertions(+), 11 deletions(-)
>
> diff --git dix/devices.c dix/devices.c
> index 245a95b..dce0f12 100644
> --- dix/devices.c
> +++ dix/devices.c
> @@ -878,13 +878,41 @@ CloseDevice(DeviceIntPtr dev)
> }
>
> /**
> + * Shut down all devices of one list and free all resources.
> + */
> +static
> +void
> +CloseListDevices(DeviceIntPtr *listHead)
> +{
> + /* Used to mark devices that we tried to free */
> + Bool freedIds[MAXDEVICES] = { 0 };
This would only work if UnInit()s removals are somehow forced onto
freedIds. In any case the freed array is a sort of context that's not
easy to get right. I'm CC'ing Peter, maybe he's got a better idea.
> + DeviceIntPtr dev;
> +
> + if (listHead == NULL)
> + return;
> +
> + dev = *listHead;
> + while (dev != NULL)
> + {
> + freedIds[dev->id] = TRUE;
> + DeleteInputDeviceRequest(dev);
> +
> + dev = *listHead;
> + while (dev != NULL && freedIds[dev->id])
> + {
> + dev = dev->next;
> + }
> + }
> +}
> +
> +/**
> * Shut down all devices, free all resources, etc.
> * Only useful if you're shutting down the server!
> */
> void
> CloseDownDevices(void)
> {
> - DeviceIntPtr dev, next;
> + DeviceIntPtr dev;
>
> /* Float all SDs before closing them. Note that at this point resources
> * (e.g. cursors) have been freed already, so we can't just call
> @@ -897,16 +925,8 @@ CloseDownDevices(void)
> dev->u.master = NULL;
> }
>
> - for (dev = inputInfo.devices; dev; dev = next)
> - {
> - next = dev->next;
> - DeleteInputDeviceRequest(dev);
> - }
> - for (dev = inputInfo.off_devices; dev; dev = next)
> - {
> - next = dev->next;
> - DeleteInputDeviceRequest(dev);
> - }
> + CloseListDevices(&inputInfo.devices);
> + CloseListDevices(&inputInfo.off_devices);
>
> CloseDevice(inputInfo.pointer);
> CloseDevice(inputInfo.keyboard);
More information about the xorg-devel
mailing list