On 6/13/23 06:32, Volodymyr Babchuk wrote:
...
> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> index 652807a4a4..1270174e78 100644
> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -38,20 +38,32 @@ extern vpci_register_init_t *const __end_vpci_array[];
>
> void vpci_remove_device(struct pci_dev *pdev)
> {
> - if ( !has_vpci(pdev->domain) || !pdev->vpci )
> + struct vpci *vpci;
> +
> + if ( !has_vpci(pdev->domain) )
> return;
>
> - spin_lock(&pdev->vpci->lock);
> + write_lock(&pdev->domain->vpci_rwlock);
> + if ( !pdev->vpci )
> + {
> + write_unlock(&pdev->domain->vpci_rwlock);
> + return;
> + }
> +
> + vpci = pdev->vpci;
> + pdev->vpci = NULL;
Here we set pdev->vpci to NULL, yet there are still a few uses remaining after
this in vpci_remove_device. I suspect they were missed during a
s/pdev->vpci/vpci/ search and replace.
> + write_unlock(&pdev->domain->vpci_rwlock);
> +
> while ( !list_empty(&pdev->vpci->handlers) )
pdev->vpci dereferenced here
> {
> - struct vpci_register *r = list_first_entry(&pdev->vpci->handlers,
> + struct vpci_register *r = list_first_entry(&vpci->handlers,
> struct vpci_register,
> node);
>
> list_del(&r->node);
> xfree(r);
> }
> - spin_unlock(&pdev->vpci->lock);
> +
> if ( pdev->vpci->msix )
pdev->vpci dereferenced here
> {
> unsigned int i;
> @@ -61,29 +73,33 @@ void vpci_remove_device(struct pci_dev *pdev)
> if ( pdev->vpci->msix->table[i] )
pdev->vpci dereferenced here, and two more above not shown in the diff context
> iounmap(pdev->vpci->msix->table[i]);
pdev->vpci dereferenced here
> }
> - xfree(pdev->vpci->msix);
> - xfree(pdev->vpci->msi);
> - xfree(pdev->vpci);
> - pdev->vpci = NULL;
> + xfree(vpci->msix);
> + xfree(vpci->msi);
> + xfree(vpci);
> }
