bus/pci: fix use after free on unplug

Message ID 20211103111615.6271-1-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series bus/pci: fix use after free on unplug |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS

Commit Message

David Marchand Nov. 3, 2021, 11:16 a.m. UTC
  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(-)
  

Comments

Maxime Coquelin Nov. 4, 2021, 2:15 p.m. UTC | #1
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
  
David Marchand Nov. 4, 2021, 2:21 p.m. UTC | #2
On Thu, Nov 4, 2021 at 3:16 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
> 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>
Tested-by: Yan Xia <yanx.xia@intel.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Applied, thanks.
  

Patch

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;
 }