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