Re: [dpdk-dev] [PATCH] bus/pci: fix use after free on unplug

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 11/3/21 12:16, David Marchand wrote:
> rte_pci_unmap_device() needs intr_handle objects to unregister
> callbacks.
> 
> Bugzilla ID: 845
> Fixes: d61138d4f0e2 ("drivers: remove direct access to interrupt handle")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>   drivers/bus/pci/pci_common.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> index f8fff2c98e..4a3a87f24f 100644
> --- a/drivers/bus/pci/pci_common.c
> +++ b/drivers/bus/pci/pci_common.c
> @@ -271,10 +271,6 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
>   		return ret; /* no rollback if already succeeded earlier */
>   	if (ret) {
>   		dev->driver = NULL;
> -		rte_intr_instance_free(dev->vfio_req_intr_handle);
> -		dev->vfio_req_intr_handle = NULL;
> -		rte_intr_instance_free(dev->intr_handle);
> -		dev->intr_handle = NULL;
>   		if ((dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) &&
>   			/* Don't unmap if device is unsupported and
>   			 * driver needs mapped resources.
> @@ -282,6 +278,10 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
>   			!(ret > 0 &&
>   				(dr->drv_flags & RTE_PCI_DRV_KEEP_MAPPED_RES)))
>   			rte_pci_unmap_device(dev);
> +		rte_intr_instance_free(dev->vfio_req_intr_handle);
> +		dev->vfio_req_intr_handle = NULL;
> +		rte_intr_instance_free(dev->intr_handle);
> +		dev->intr_handle = NULL;
>   	} else {
>   		dev->device.driver = &dr->driver;
>   	}
> @@ -322,15 +322,16 @@ rte_pci_detach_dev(struct rte_pci_device *dev)
>   	/* clear driver structure */
>   	dev->driver = NULL;
>   	dev->device.driver = NULL;
> -	rte_intr_instance_free(dev->intr_handle);
> -	dev->intr_handle = NULL;
> -	rte_intr_instance_free(dev->vfio_req_intr_handle);
> -	dev->vfio_req_intr_handle = NULL;
>   
>   	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
>   		/* unmap resources for devices that use igb_uio */
>   		rte_pci_unmap_device(dev);
>   
> +	rte_intr_instance_free(dev->intr_handle);
> +	dev->intr_handle = NULL;
> +	rte_intr_instance_free(dev->vfio_req_intr_handle);
> +	dev->vfio_req_intr_handle = NULL;
> +
>   	return 0;
>   }
>   
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime