Commit 61e90817 authored by Jason Gunthorpe's avatar Jason Gunthorpe Committed by Alex Williamson
Browse files

vfio/pci: Move VGA and VF initialization to functions 



vfio_pci_probe() is quite complicated, with optional VF and VGA sub
components. Move these into clear init/uninit functions and have a linear
flow in probe/remove.

This fixes a few little buglets:
 - vfio_pci_remove() is in the wrong order, vga_client_register() removes
   a notifier and is after kfree(vdev), but the notifier refers to vdev,
   so it can use after free in a race.
 - vga_client_register() can fail but was ignored

Organize things so destruction order is the reverse of creation order.

Fixes: ecaa1f6a ("vfio-pci: Add VGA arbiter client")
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarKevin Tian <kevin.tian@intel.com>
Reviewed-by: default avatarMax Gurtovoy <mgurtovoy@nvidia.com>
Reviewed-by: default avatarCornelia Huck <cohuck@redhat.com>
Reviewed-by: default avatarEric Auger <eric.auger@redhat.com>
Signed-off-by: default avatarJason Gunthorpe <jgg@nvidia.com>
Message-Id: <7-v3-225de1400dfc+4e074-vfio1_jgg@nvidia.com>
Signed-off-by: default avatarAlex Williamson <alex.williamson@redhat.com>
parent 0ca78666

drivers/vfio/pci/vfio_pci.c

+74 −42
Original line number Diff line number Diff line
@@ -1922,6 +1922,68 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb, 
	return 0;

}



static int vfio_pci_vf_init(struct vfio_pci_device *vdev)

{

	struct pci_dev *pdev = vdev->pdev;

	int ret;



	if (!pdev->is_physfn)

		return 0;



	vdev->vf_token = kzalloc(sizeof(*vdev->vf_token), GFP_KERNEL);

	if (!vdev->vf_token)

		return -ENOMEM;



	mutex_init(&vdev->vf_token->lock);

	uuid_gen(&vdev->vf_token->uuid);



	vdev->nb.notifier_call = vfio_pci_bus_notifier;

	ret = bus_register_notifier(&pci_bus_type, &vdev->nb);

	if (ret) {

		kfree(vdev->vf_token);

		return ret;

	}

	return 0;

}



static void vfio_pci_vf_uninit(struct vfio_pci_device *vdev)

{

	if (!vdev->vf_token)

		return;



	bus_unregister_notifier(&pci_bus_type, &vdev->nb);

	WARN_ON(vdev->vf_token->users);

	mutex_destroy(&vdev->vf_token->lock);

	kfree(vdev->vf_token);

}



static int vfio_pci_vga_init(struct vfio_pci_device *vdev)

{

	struct pci_dev *pdev = vdev->pdev;

	int ret;



	if (!vfio_pci_is_vga(pdev))

		return 0;



	ret = vga_client_register(pdev, vdev, NULL, vfio_pci_set_vga_decode);

	if (ret)

		return ret;

	vga_set_legacy_decoding(pdev, vfio_pci_set_vga_decode(vdev, false));

	return 0;

}



static void vfio_pci_vga_uninit(struct vfio_pci_device *vdev)

{

	struct pci_dev *pdev = vdev->pdev;



	if (!vfio_pci_is_vga(pdev))

		return;

	vga_client_register(pdev, NULL, NULL, NULL);

	vga_set_legacy_decoding(pdev, VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM |

					      VGA_RSRC_LEGACY_IO |

					      VGA_RSRC_LEGACY_MEM);

}



static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)

{

	struct vfio_pci_device *vdev;
@@ -1975,28 +2037,12 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) 
	ret = vfio_pci_reflck_attach(vdev);

	if (ret)

		goto out_del_group_dev;



	if (pdev->is_physfn) {

		vdev->vf_token = kzalloc(sizeof(*vdev->vf_token), GFP_KERNEL);

		if (!vdev->vf_token) {

			ret = -ENOMEM;

	ret = vfio_pci_vf_init(vdev);

	if (ret)

		goto out_reflck;

		}



		mutex_init(&vdev->vf_token->lock);

		uuid_gen(&vdev->vf_token->uuid);



		vdev->nb.notifier_call = vfio_pci_bus_notifier;

		ret = bus_register_notifier(&pci_bus_type, &vdev->nb);

	ret = vfio_pci_vga_init(vdev);

	if (ret)

			goto out_vf_token;

	}



	if (vfio_pci_is_vga(pdev)) {

		vga_client_register(pdev, vdev, NULL, vfio_pci_set_vga_decode);

		vga_set_legacy_decoding(pdev,

					vfio_pci_set_vga_decode(vdev, false));

	}

		goto out_vf;



	vfio_pci_probe_power_state(vdev);
@@ -2016,8 +2062,8 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) 


	return ret;



out_vf_token:

	kfree(vdev->vf_token);

out_vf:

	vfio_pci_vf_uninit(vdev);

out_reflck:

	vfio_pci_reflck_put(vdev->reflck);

out_del_group_dev:
@@ -2039,33 +2085,19 @@ static void vfio_pci_remove(struct pci_dev *pdev) 
	if (!vdev)

		return;



	if (vdev->vf_token) {

		WARN_ON(vdev->vf_token->users);

		mutex_destroy(&vdev->vf_token->lock);

		kfree(vdev->vf_token);

	}



	if (vdev->nb.notifier_call)

		bus_unregister_notifier(&pci_bus_type, &vdev->nb);



	vfio_pci_vf_uninit(vdev);

	vfio_pci_reflck_put(vdev->reflck);

	vfio_pci_vga_uninit(vdev);



	vfio_iommu_group_put(pdev->dev.iommu_group, &pdev->dev);

	kfree(vdev->region);

	mutex_destroy(&vdev->ioeventfds_lock);



	if (!disable_idle_d3)

		vfio_pci_set_power_state(vdev, PCI_D0);



	mutex_destroy(&vdev->ioeventfds_lock);

	kfree(vdev->region);

	kfree(vdev->pm_save);

	kfree(vdev);



	if (vfio_pci_is_vga(pdev)) {

		vga_client_register(pdev, NULL, NULL, NULL);

		vga_set_legacy_decoding(pdev,

				VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM |

				VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM);

	}

}



static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,