On 04/08/14 18:27, Hartley Sweeten wrote:
On Monday, August 04, 2014 3:49 AM, Ian Abbott [mailto:abbotti@xxxxxxxxx] 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@xxxxxxxxx> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
