[PATCH] staging: comedi: comedi_pci: enhance comedi_pci_disable()

[Ian Abbott]

On 04/08/14 18:27, Hartley Sweeten wrote:
> On Monday, August 04, 2014 3:49 AM, Ian Abbott [mailto:abbotti at mev.co.uk] wrote:
>>> diff --git a/drivers/staging/comedi/drivers/addi_apci_2032.c b/drivers/staging/comedi/drivers/addi_apci_2032.c
>>> index be0a8a7..d35998d 100644
>>> --- a/drivers/staging/comedi/drivers/addi_apci_2032.c
>>> +++ b/drivers/staging/comedi/drivers/addi_apci_2032.c
>>> @@ -339,11 +339,9 @@ static void apci2032_detach(struct comedi_device *dev)
>>>    {
>>>    	if (dev->iobase)
>>>    		apci2032_reset(dev);
>>> -	if (dev->irq)
>>> -		free_irq(dev->irq, dev);
>>>    	if (dev->read_subdev)
>>>    		kfree(dev->read_subdev->private);
>>> -	comedi_pci_disable(dev);
>>> +	comedi_pci_detach(dev);
>>>    }
>>
>> That could cause the interrupt handler to access freed memory due to a
>> race condition.  To avoid that, move the kfree() after the call to
>> comedi_pci_detach().
>
> Ian,
>
> I looked over these and I think the correct order in the (*detach) should be:
>
> 1) Do any 'reset' call that the driver has to disable interrupts.
> 2) iounmap() any extra addresses (in the private data)
> 3) call comedi_pci_detach(), which does:
> 	a) free_irq(dev->irq)
> 	b) iounmap(dev->mmio)
> 	c) pci_release_regions(pcidev)
> 	d) pci_disable_device(pcidev)
> 4) then do any additional cleanup (kfree() etc.)
>
> I'm not sure if it matters, but I think all the iounmap() should be done
> before the PCI regions are released and the device is disabled.
>
> I'm rebasing the patch based on this and will post it shortly.

I don't think it's necessary to do the iounmap() before 
pci_disable_device().  It only affects the page tables.  If you call 
pci_disable_device() before iounmapping the region, the virtual address 
might not map to an accessible location any longer, but it's not being 
accessed, it's only being unmapped.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-