[libvirt] [PATCH] Fix a potential race in pciInitDevice.

[Don Dutile]

Chris Lalancette wrote:
> If detecting the FLR flag of a pci device fails, then we
> could run into the situation of trying to close a file
> descriptor twice, once in pciInitDevice() and once in pciFreeDevice().
> Fix that by removing the pciCloseConfig() in pciInitDevice() and
> just letting pciFreeDevice() handle it.
> 
> Thanks to Chris Wright for pointing out this problem.
> 
> While we are at it, fix an error check.  While it would actually
> work as-is (since success returns 0), it's still more clear to
> check for < 0 (as the rest of the code does).
> 
> Signed-off-by: Chris Lalancette <clalance at redhat.com>
> ---
>  src/qemu/qemu_driver.c |    2 +-
>  src/util/pci.c         |    8 ++++----
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index b4c468a..9436e2a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8010,7 +8010,7 @@ static int qemudDomainAttachHostPciDevice(struct qemud_driver *driver,
>          return -1;
>      }
>  
> -    if (qemuPrepareHostdevPCIDevices(driver, &hostdev, 1))
> +    if (qemuPrepareHostdevPCIDevices(driver, &hostdev, 1) < 0)
>          return -1;
>  
>      if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
> diff --git a/src/util/pci.c b/src/util/pci.c
> index 14ef058..26d55b8 100644
> --- a/src/util/pci.c
> +++ b/src/util/pci.c
> @@ -189,8 +189,10 @@ pciCloseConfig(pciDevice *dev)
>      if (!dev)
>          return;
>  
> -    if (dev->fd >= 0)
> +    if (dev->fd >= 0) {
>          close(dev->fd);
> +        dev->fd = -1;
> +    }
>  }
>  
>  static int
> @@ -673,10 +675,8 @@ pciInitDevice(pciDevice *dev)
>      dev->pcie_cap_pos   = pciFindCapabilityOffset(dev, PCI_CAP_ID_EXP);
>      dev->pci_pm_cap_pos = pciFindCapabilityOffset(dev, PCI_CAP_ID_PM);
>      flr = pciDetectFunctionLevelReset(dev);
> -    if (flr < 0) {
> -        pciCloseConfig(dev);
> +    if (flr < 0)
>          return flr;
> -    }
>      dev->has_flr        = flr;
>      dev->has_pm_reset   = pciDetectPowerManagementReset(dev);
>      dev->initted        = 1;

seems simple & logical enough...

Acked-by: Donald Dutile <ddutile at redhat.com>