[PATCH] Possible AMD8111e free irq issue

[PATCH] Possible AMD8111e free irq issue

[Panagiotis Issaris]

Hi,

It seems to me that if in the amd8111e_open() fuction dev->irq isn't
zero and the irq request succeeds it might not get released anymore.

Specifically, on failure of the amd8111e_restart() call the function
returns -ENOMEM without releasing the irq. The amd8111e_restart()
function can fail because of various pci_alloc_consistent() and
dev_alloc_skb() calls in amd8111e_init_ring() which is being
called by amd8111e_restart.

1374     if(dev->irq ==0 || request_irq(dev->irq, amd8111e_interrupt, SA_SHIRQ,
1375                      dev->name, dev))
1376         return -EAGAIN;
	
The patch applies to 2.6.11-rc5-bk2. 

If I'm right about the above, I'm not I'm not sure if the free_irq() should
happen before or after releasing the spinlock.

With friendly regards,
Takis

diff -uprN linux-2.6.11-rc5-bk2/drivers/net/amd8111e.c linux-2.6.11-rc5-bk2-pi/drivers/net/amd8111e.c
--- linux-2.6.11-rc5-bk2/drivers/net/amd8111e.c	2005-02-28 13:44:46.000000000 +0100
+++ linux-2.6.11-rc5-bk2-pi/drivers/net/amd8111e.c	2005-02-28 13:45:09.000000000 +0100
@@ -1381,6 +1381,8 @@ static int amd8111e_open(struct net_devi
 
 	if(amd8111e_restart(dev)){
 		spin_unlock_irq(&lp->lock);
+		if (dev->irq)
+			free_irq(dev->irq, dev);
 		return -ENOMEM;
 	}
 	/* Start ipg timer */


-- 
OpenPGP key: http://lumumba.luc.ac.be/takis/takis_public_key.txt
fingerprint: 6571 13A3 33D9 3726 F728  AA98 F643 B12E ECF3 E029

Re: [PATCH] Possible AMD8111e free irq issue

[Jeff Garzik]

Panagiotis Issaris wrote:
> Hi,
> 
> It seems to me that if in the amd8111e_open() fuction dev->irq isn't
> zero and the irq request succeeds it might not get released anymore.
> 
> Specifically, on failure of the amd8111e_restart() call the function
> returns -ENOMEM without releasing the irq. The amd8111e_restart()
> function can fail because of various pci_alloc_consistent() and
> dev_alloc_skb() calls in amd8111e_init_ring() which is being
> called by amd8111e_restart.
> 
> 1374     if(dev->irq ==0 || request_irq(dev->irq, amd8111e_interrupt, SA_SHIRQ,
> 1375                      dev->name, dev))
> 1376         return -EAGAIN;
> 	
> The patch applies to 2.6.11-rc5-bk2. 
> 
> If I'm right about the above, I'm not I'm not sure if the free_irq() should
> happen before or after releasing the spinlock.
> 
> With friendly regards,
> Takis
> 
> diff -uprN linux-2.6.11-rc5-bk2/drivers/net/amd8111e.c linux-2.6.11-rc5-bk2-pi/drivers/net/amd8111e.c
> --- linux-2.6.11-rc5-bk2/drivers/net/amd8111e.c	2005-02-28 13:44:46.000000000 +0100
> +++ linux-2.6.11-rc5-bk2-pi/drivers/net/amd8111e.c	2005-02-28 13:45:09.000000000 +0100
> @@ -1381,6 +1381,8 @@ static int amd8111e_open(struct net_devi
>  
>  	if(amd8111e_restart(dev)){
>  		spin_unlock_irq(&lp->lock);
> +		if (dev->irq)
> +			free_irq(dev->irq, dev);
>  		return -ENOMEM;

Yes, this is a needed fix.  Thanks.

	Jeff

Re: [PATCH] Possible AMD8111e free irq issue

[Panagiotis Issaris]

Hi,

Jeff Garzik wrote:

>> diff -uprN linux-2.6.11-rc5-bk2/drivers/net/amd8111e.c 
>> linux-2.6.11-rc5-bk2-pi/drivers/net/amd8111e.c
>> --- linux-2.6.11-rc5-bk2/drivers/net/amd8111e.c    2005-02-28 
>> 13:44:46.000000000 +0100
>> +++ linux-2.6.11-rc5-bk2-pi/drivers/net/amd8111e.c    2005-02-28 
>> 13:45:09.000000000 +0100
>> @@ -1381,6 +1381,8 @@ static int amd8111e_open(struct net_devi
>>  
>>      if(amd8111e_restart(dev)){
>>          spin_unlock_irq(&lp->lock);
>> +        if (dev->irq)
>> +            free_irq(dev->irq, dev);
>>          return -ENOMEM;
>
>
> Yes, this is a needed fix.  Thanks.

Should the release of the irq happen before or after unlocking the 
spinlock? I wasn't really
sure about it.

With friendly regards,
Takis

-- 
  K.U.Leuven, Mechanical Eng.,  Mechatronics & Robotics Research Group
  http://people.mech.kuleuven.ac.be/~pissaris/