[PATCH] Allow driver to call DeleteInputDeviceRequest during UnInit

[Oldřich Jedlička]

Hi Simon,

Dne St 13. ledna 2010 20:47:29 Simon Thum napsal(a):
> Oldřich Jedlička wrote:
> > When an 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 fixes the problem by introducing a pointer to the value
> > holding the reference to the driver that is currently being freed.
> 
> I see the problem, but I don't see why prev is not prone to pointing
> into the void. What if DIDR ends up freeing *prev's storage? Not that
> it's likely but I also don't see what might rule it out.

I agree with you that it looks like there should be better handling. But when 
I look at implementations of DeleteInputDeviceRequests in hw/ directory, they 
either does nothing (devices are never freed), or the device is always removed 
(RemoveDevice -> remove from the device list).

So in theory there could be problem, but in practice the prev will always 
point to list head or it will move to the list end without freeing devices.

The question is only the wanted complexity of the solution. I've chosen to 
start with the simplest one :-)

Cheers,
Oldřich.

> 
> > Signed-off-by: Oldřich Jedlička <oldium.pro at seznam.cz>
> > ---
> > 
> >  dix/devices.c |   18 +++++++++++++-----
> >  1 files changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/dix/devices.c b/dix/devices.c
> > index 245a95b..e4bd908 100644
> > --- a/dix/devices.c
> > +++ b/dix/devices.c
> > @@ -884,7 +884,7 @@ CloseDevice(DeviceIntPtr dev)
> > 
> >  void
> >  CloseDownDevices(void)
> >  {
> > 
> > -    DeviceIntPtr dev, next;
> > +    DeviceIntPtr dev, *prev;
> > 
> >      /* 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,15 +897,23 @@ CloseDownDevices(void)
> > 
> >              dev->u.master = NULL;
> >      
> >      }
> > 
> > -    for (dev = inputInfo.devices; dev; dev = next)
> > +    for (prev = &inputInfo.devices, dev = *prev; dev; dev = *prev)
> > 
> >      {
> > 
> > -	next = dev->next;
> > 
> >          DeleteInputDeviceRequest(dev);
> > 
> > +        if (*prev == dev)
> > +        {
> > +            /* Device not freed, move to the next one */
> > +            prev = &dev->next;
> > +        }
> > 
> >      }
> > 
> > -    for (dev = inputInfo.off_devices; dev; dev = next)
> > +    for (prev = &inputInfo.off_devices, dev = *prev; dev; dev = *prev)
> > 
> >      {
> > 
> > -	next = dev->next;
> > 
> >          DeleteInputDeviceRequest(dev);
> > 
> > +        if (*prev == dev)
> > +        {
> > +            /* Device not freed, move to the next one */
> > +            prev = &dev->next;
> > +        }
> > 
> >      }
> >      
> >      CloseDevice(inputInfo.pointer);
> 
> _______________________________________________
> xorg-devel mailing list
> xorg-devel at lists.x.org
> http://lists.x.org/mailman/listinfo/xorg-devel