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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Monday, August 04, 2014 3:49 AM. Ian Abbott wrote:
> On 2014-08-01 20:50, H Hartley Sweeten wrote:
> [snip]
>> diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
>> index 58e58a3..73e5fd3 100644
>> --- a/drivers/staging/comedi/comedidev.h
>> +++ b/drivers/staging/comedi/comedidev.h
>> @@ -502,7 +502,7 @@ struct pci_driver;
>>   struct pci_dev *comedi_to_pci_dev(struct comedi_device *);
>>
>>   int comedi_pci_enable(struct comedi_device *);
>> -void comedi_pci_disable(struct comedi_device *);
>> +void comedi_pci_detach(struct comedi_device *);
>>
>>   int comedi_pci_auto_config(struct pci_dev *, struct comedi_driver *,
>>   			   unsigned long context);
>> @@ -543,7 +543,7 @@ static inline int comedi_pci_enable(struct comedi_device *dev)
>>   	return -ENOSYS;
>>   }
>>
>> -static inline void comedi_pci_disable(struct comedi_device *dev)
>> +static inline void comedi_pcmcia_disable(struct comedi_device *dev)
>>   {
>>   }
>
> I think you meant to rename it to comedi_pci_detach() there?

I did... Not sure how that slipped.

>> 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().

Ah.. Made an assumption that apci2032_reset() would disable the potential
for an interrupt. But, this is a PCI device so the interrupt is shared...

I'll take another look at this and the other ones and rebase.

Greg has probably closed the staging tree due to the release of 3.16 so I might
have to wait until he has opened it again.

Thanks,
Hartley

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux