Re: [Qemu-devel] [PATCH] hw/pci: fix error flow in pci multifunction init

[Michael S. Tsirkin]

On Tue, Jan 21, 2014 at 06:37:51PM +0200, Marcel Apfelbaum wrote:
> Scenario:
>   - There is a non multifunction pci device A on 00:0X.0.
>   - Hot-plug another multifunction pci device B at 00:0X.1.
>   - The operation will fail of course.
>   - Try to hot-plug the B device 2-3 more times, qemu will crash.
> 
> Reason: The error flow leaves the B's address space into global address spaces
> list, but the device object is freed. Fixed that.
> 
> Signed-off-by: Marcel Apfelbaum <address@hidden>

Thanks, applied.

> ---
>  hw/pci/pci.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index aa2a395..5f454dd 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -793,6 +793,15 @@ static void pci_config_free(PCIDevice *pci_dev)
>      g_free(pci_dev->used);
>  }
>  
> +static void do_pci_unregister_device(PCIDevice *pci_dev)
> +{
> +    pci_dev->bus->devices[pci_dev->devfn] = NULL;
> +    pci_config_free(pci_dev);
> +
> +    address_space_destroy(&pci_dev->bus_master_as);
> +    memory_region_destroy(&pci_dev->bus_master_enable_region);
> +}
> +
>  /* -1 for devfn means auto assign */
>  static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>                                           const char *name, int devfn)
> @@ -858,7 +867,7 @@ static PCIDevice *do_pci_register_device(PCIDevice 
> *pci_dev, PCIBus *bus,
>          pci_init_mask_bridge(pci_dev);
>      }
>      if (pci_init_multifunction(bus, pci_dev)) {
> -        pci_config_free(pci_dev);
> +        do_pci_unregister_device(pci_dev);
>          return NULL;
>      }
>  
> @@ -873,15 +882,6 @@ static PCIDevice *do_pci_register_device(PCIDevice 
> *pci_dev, PCIBus *bus,
>      return pci_dev;
>  }
>  
> -static void do_pci_unregister_device(PCIDevice *pci_dev)
> -{
> -    pci_dev->bus->devices[pci_dev->devfn] = NULL;
> -    pci_config_free(pci_dev);
> -
> -    address_space_destroy(&pci_dev->bus_master_as);
> -    memory_region_destroy(&pci_dev->bus_master_enable_region);
> -}
> -
>  static void pci_unregister_io_regions(PCIDevice *pci_dev)
>  {
>      PCIIORegion *r;
> -- 
> 1.8.3.1