ixgbe: pci_get_device() call without counterpart call of pci_dev_put()

ixgbe: pci_get_device() call without counterpart call of pci_dev_put()

[Elena Gurevich]

Hi all,
I am pioneer in linux device drivers here and using Intel 82599 NIC as
reference model, 
During investigation to drivers sources I found the suspicious code:  

Is code sequence (1)   and (2) the possible device reference count leakage
???


Thanks a lot in advance
Lena

--------snipped from ixgbe_main.c  file function ixgbe_io_error_detected()
-----------

. . . 
		/* Find the pci device of the offending VF */
		vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, device_id,
NULL);
		while (vfdev) {
			if (vfdev->devfn == (req_id & 0xFF))
				break;
<------------------------------      (1) leaves the loop with successful get
call  !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

			vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
					       device_id, vfdev);
		}
		/*
		 * There's a slim chance the VF could have been hot plugged,
		 * so if it is no longer present we don't need to issue the
		 * VFLR.  Just clean up the AER in that case.
		 */
		if (vfdev) {
			e_dev_err("Issuing VFLR to VF %d\n", vf);
			pci_write_config_dword(vfdev, 0xA8, 0x00008000);
		}

		pci_cleanup_aer_uncorrect_error_status(pdev);
	}

	/*
	 * Even though the error may have occurred on the other port
	 * we still need to increment the vf error reference count for
	 * both ports because the I/O resume function will be called
	 * for both of them.
	 */
	adapter->vferr_refcount++;

	return PCI_ERS_RESULT_RECOVERED;
<--------------------------------------------  (2) leaves the function
without put call !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
----------------------------------  snipped
-----------------------------------

Re: ixgbe: pci_get_device() call without counterpart call of pci_dev_put()

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]

On 12/09/2012 01:47 AM, Elena Gurevich wrote:
> Hi all,
> I am pioneer in linux device drivers here and using Intel 82599 NIC as
> reference model, 
> During investigation to drivers sources I found the suspicious code:  
>
> Is code sequence (1)   and (2) the possible device reference count leakage
> ???
>
>
> Thanks a lot in advance
> Lena
>
> --------snipped from ixgbe_main.c  file function ixgbe_io_error_detected()
> -----------
>
> . . . 
> 		/* Find the pci device of the offending VF */
> 		vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, device_id,
> NULL);
> 		while (vfdev) {
> 			if (vfdev->devfn == (req_id & 0xFF))
> 				break;
> <------------------------------      (1) leaves the loop with successful get
> call  !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
>
> 			vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
> 					       device_id, vfdev);
> 		}
> 		/*
> 		 * There's a slim chance the VF could have been hot plugged,
> 		 * so if it is no longer present we don't need to issue the
> 		 * VFLR.  Just clean up the AER in that case.
> 		 */
> 		if (vfdev) {
> 			e_dev_err("Issuing VFLR to VF %d\n", vf);
> 			pci_write_config_dword(vfdev, 0xA8, 0x00008000);
> 		}
>
> 		pci_cleanup_aer_uncorrect_error_status(pdev);
> 	}
>
> 	/*
> 	 * Even though the error may have occurred on the other port
> 	 * we still need to increment the vf error reference count for
> 	 * both ports because the I/O resume function will be called
> 	 * for both of them.
> 	 */
> 	adapter->vferr_refcount++;
>
> 	return PCI_ERS_RESULT_RECOVERED;
> <--------------------------------------------  (2) leaves the function
> without put call !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
> ----------------------------------  snipped
> -----------------------------------
>
Added Greg Rose (ixgbevf maintainer) & Don Skidmore (ixgbe maintainer)

Since the code you have questions about is dealing with the vf, Greg
will most likely be able to shed some light to your questions.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

RE: ixgbe: pci_get_device() call without counterpart call of pci_dev_put()

[Rose, Gregory V]

> -----Original Message-----
> From: Jeff Kirsher [mailto:tarbal@gmail.com]
> Sent: Sunday, December 09, 2012 11:18 PM
> To: Elena Gurevich
> Cc: netdev; e1000-devel@lists.sourceforge.net; Rose, Gregory V; Skidmore,
> Donald C
> Subject: Re: ixgbe: pci_get_device() call without counterpart call of
> pci_dev_put()
> 
> On 12/09/2012 01:47 AM, Elena Gurevich wrote:
> > Hi all,
> > I am pioneer in linux device drivers here and using Intel 82599 NIC as
> > reference model, During investigation to drivers sources I found the
> > suspicious code:
> >
> > Is code sequence (1)   and (2) the possible device reference count
> leakage
> > ???
> >
> >
> > Thanks a lot in advance
> > Lena
> >
> > --------snipped from ixgbe_main.c  file function
> > ixgbe_io_error_detected()
> > -----------
> >
> > . . .
> > 		/* Find the pci device of the offending VF */
> > 		vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, device_id, NULL);
> > 		while (vfdev) {
> > 			if (vfdev->devfn == (req_id & 0xFF))
> > 				break;
> > <------------------------------      (1) leaves the loop with successful
> get
> > call  !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
> >
> > 			vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
> > 					       device_id, vfdev);
> > 		}
> > 		/*
> > 		 * There's a slim chance the VF could have been hot plugged,
> > 		 * so if it is no longer present we don't need to issue the
> > 		 * VFLR.  Just clean up the AER in that case.
> > 		 */
> > 		if (vfdev) {
> > 			e_dev_err("Issuing VFLR to VF %d\n", vf);
> > 			pci_write_config_dword(vfdev, 0xA8, 0x00008000);
> > 		}
> >
> > 		pci_cleanup_aer_uncorrect_error_status(pdev);
> > 	}
> >
> > 	/*
> > 	 * Even though the error may have occurred on the other port
> > 	 * we still need to increment the vf error reference count for
> > 	 * both ports because the I/O resume function will be called
> > 	 * for both of them.
> > 	 */
> > 	adapter->vferr_refcount++;
> >
> > 	return PCI_ERS_RESULT_RECOVERED;
> > <--------------------------------------------  (2) leaves the function
> > without put call !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
> > ----------------------------------  snipped
> > -----------------------------------
> >
> Added Greg Rose (ixgbevf maintainer) & Don Skidmore (ixgbe maintainer)
> 
> Since the code you have questions about is dealing with the vf, Greg will
> most likely be able to shed some light to your questions.

That's a bug.  I'll generate a patch to add the necessary pci_dev_put().

- Greg