vfio/pci: Fix vf_token mechanism when device-specific VF drivers are used

BugLink: https://bugs.launchpad.net/bugs/1969857
[ Upstream commit 1ef3342a934e235aca72b4bcc0d6854d80a65077 ]

get_pf_vdev() tries to check if a PF is a VFIO PF by looking at the driver:

       if (pci_dev_driver(physfn) != pci_dev_driver(vdev->pdev)) {

However now that we have multiple VF and PF drivers this is no longer
reliable.

This means that security tests realted to vf_token can be skipped by
mixing and matching different VFIO PCI drivers.

Instead of trying to use the driver core to find the PF devices maintain a
linked list of all PF vfio_pci_core_device's that we have called
pci_enable_sriov() on.

When registering a VF just search the list to see if the PF is present and
record the match permanently in the struct. PCI core locking prevents a PF
from passing pci_disable_sriov() while VF drivers are attached so the VFIO
owned PF becomes a static property of the VF.

In common cases where vfio does not own the PF the global list remains
empty and the VF's pointer is statically NULL.

This also fixes a lockdep splat from recursive locking of the
vfio_group::device_lock between vfio_device_get_from_name() and
vfio_device_get_from_dev(). If the VF and PF share the same group this
would deadlock.

Fixes: ff53edf6d6ab ("vfio/pci: Split the pci_driver code out of vfio_pci_core.c")
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/r/0-v3-876570980634+f2e8-vfio_vf_token_jgg@nvidia.com
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 15d158bdcde0197926225801bc5547eca002ded6..f3916e6b16b9dd28eab0560c7aec0b14f531801f 100644 (file)
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -36,6 +36,10 @@ static bool nointxmask;
 static bool disable_vga;
 static bool disable_idle_d3;
 
+/* List of PF's that vfio_pci_core_sriov_configure() has been called on */
+static DEFINE_MUTEX(vfio_pci_sriov_pfs_mutex);
+static LIST_HEAD(vfio_pci_sriov_pfs);
+
 static inline bool vfio_vga_disabled(void)
 {
 #ifdef CONFIG_VFIO_PCI_VGA
@@ -434,47 +438,17 @@ out:
 }
 EXPORT_SYMBOL_GPL(vfio_pci_core_disable);
 
-static struct vfio_pci_core_device *get_pf_vdev(struct vfio_pci_core_device *vdev)
-{
-       struct pci_dev *physfn = pci_physfn(vdev->pdev);
-       struct vfio_device *pf_dev;
-
-       if (!vdev->pdev->is_virtfn)
-               return NULL;
-
-       pf_dev = vfio_device_get_from_dev(&physfn->dev);
-       if (!pf_dev)
-               return NULL;
-
-       if (pci_dev_driver(physfn) != pci_dev_driver(vdev->pdev)) {
-               vfio_device_put(pf_dev);
-               return NULL;
-       }
-
-       return container_of(pf_dev, struct vfio_pci_core_device, vdev);
-}
-
-static void vfio_pci_vf_token_user_add(struct vfio_pci_core_device *vdev, int val)
-{
-       struct vfio_pci_core_device *pf_vdev = get_pf_vdev(vdev);
-
-       if (!pf_vdev)
-               return;
-
-       mutex_lock(&pf_vdev->vf_token->lock);
-       pf_vdev->vf_token->users += val;
-       WARN_ON(pf_vdev->vf_token->users < 0);
-       mutex_unlock(&pf_vdev->vf_token->lock);
-
-       vfio_device_put(&pf_vdev->vdev);
-}
-
 void vfio_pci_core_close_device(struct vfio_device *core_vdev)
 {
        struct vfio_pci_core_device *vdev =
                container_of(core_vdev, struct vfio_pci_core_device, vdev);
 
-       vfio_pci_vf_token_user_add(vdev, -1);
+       if (vdev->sriov_pf_core_dev) {
+               mutex_lock(&vdev->sriov_pf_core_dev->vf_token->lock);
+               WARN_ON(!vdev->sriov_pf_core_dev->vf_token->users);
+               vdev->sriov_pf_core_dev->vf_token->users--;
+               mutex_unlock(&vdev->sriov_pf_core_dev->vf_token->lock);
+       }
        vfio_spapr_pci_eeh_release(vdev->pdev);
        vfio_pci_core_disable(vdev);
 
@@ -495,7 +469,12 @@ void vfio_pci_core_finish_enable(struct vfio_pci_core_device *vdev)
 {
        vfio_pci_probe_mmaps(vdev);
        vfio_spapr_pci_eeh_open(vdev->pdev);
-       vfio_pci_vf_token_user_add(vdev, 1);
+
+       if (vdev->sriov_pf_core_dev) {
+               mutex_lock(&vdev->sriov_pf_core_dev->vf_token->lock);
+               vdev->sriov_pf_core_dev->vf_token->users++;
+               mutex_unlock(&vdev->sriov_pf_core_dev->vf_token->lock);
+       }
 }
 EXPORT_SYMBOL_GPL(vfio_pci_core_finish_enable);
 
@@ -1603,11 +1582,8 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
         *
         * If the VF token is provided but unused, an error is generated.
         */
-       if (!vdev->pdev->is_virtfn && !vdev->vf_token && !vf_token)
-               return 0; /* No VF token provided or required */
-
        if (vdev->pdev->is_virtfn) {
-               struct vfio_pci_core_device *pf_vdev = get_pf_vdev(vdev);
+               struct vfio_pci_core_device *pf_vdev = vdev->sriov_pf_core_dev;
                bool match;
 
                if (!pf_vdev) {
@@ -1620,7 +1596,6 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
                }
 
                if (!vf_token) {
-                       vfio_device_put(&pf_vdev->vdev);
                        pci_info_ratelimited(vdev->pdev,
                                "VF token required to access device\n");
                        return -EACCES;
@@ -1630,8 +1605,6 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
                match = uuid_equal(uuid, &pf_vdev->vf_token->uuid);
                mutex_unlock(&pf_vdev->vf_token->lock);
 
-               vfio_device_put(&pf_vdev->vdev);
-
                if (!match) {
                        pci_info_ratelimited(vdev->pdev,
                                "Incorrect VF token provided for device\n");
@@ -1752,8 +1725,30 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
 static int vfio_pci_vf_init(struct vfio_pci_core_device *vdev)
 {
        struct pci_dev *pdev = vdev->pdev;
+       struct vfio_pci_core_device *cur;
+       struct pci_dev *physfn;
        int ret;
 
+       if (pdev->is_virtfn) {
+               /*
+                * If this VF was created by our vfio_pci_core_sriov_configure()
+                * then we can find the PF vfio_pci_core_device now, and due to
+                * the locking in pci_disable_sriov() it cannot change until
+                * this VF device driver is removed.
+                */
+               physfn = pci_physfn(vdev->pdev);
+               mutex_lock(&vfio_pci_sriov_pfs_mutex);
+               list_for_each_entry(cur, &vfio_pci_sriov_pfs, sriov_pfs_item) {
+                       if (cur->pdev == physfn) {
+                               vdev->sriov_pf_core_dev = cur;
+                               break;
+                       }
+               }
+               mutex_unlock(&vfio_pci_sriov_pfs_mutex);
+               return 0;
+       }
+
+       /* Not a SRIOV PF */
        if (!pdev->is_physfn)
                return 0;
 
@@ -1825,6 +1820,7 @@ void vfio_pci_core_init_device(struct vfio_pci_core_device *vdev,
        INIT_LIST_HEAD(&vdev->ioeventfds_list);
        mutex_init(&vdev->vma_lock);
        INIT_LIST_HEAD(&vdev->vma_list);
+       INIT_LIST_HEAD(&vdev->sriov_pfs_item);
        init_rwsem(&vdev->memory_lock);
 }
 EXPORT_SYMBOL_GPL(vfio_pci_core_init_device);
@@ -1923,7 +1919,7 @@ void vfio_pci_core_unregister_device(struct vfio_pci_core_device *vdev)
 {
        struct pci_dev *pdev = vdev->pdev;
 
-       pci_disable_sriov(pdev);
+       vfio_pci_core_sriov_configure(pdev, 0);
 
        vfio_unregister_group_dev(&vdev->vdev);
 
@@ -1963,21 +1959,49 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
 
 int vfio_pci_core_sriov_configure(struct pci_dev *pdev, int nr_virtfn)
 {
+       struct vfio_pci_core_device *vdev;
        struct vfio_device *device;
        int ret = 0;
 
+       device_lock_assert(&pdev->dev);
+
        device = vfio_device_get_from_dev(&pdev->dev);
        if (!device)
                return -ENODEV;
 
-       if (nr_virtfn == 0)
-               pci_disable_sriov(pdev);
-       else
+       vdev = container_of(device, struct vfio_pci_core_device, vdev);
+
+       if (nr_virtfn) {
+               mutex_lock(&vfio_pci_sriov_pfs_mutex);
+               /*
+                * The thread that adds the vdev to the list is the only thread
+                * that gets to call pci_enable_sriov() and we will only allow
+                * it to be called once without going through
+                * pci_disable_sriov()
+                */
+               if (!list_empty(&vdev->sriov_pfs_item)) {
+                       ret = -EINVAL;
+                       goto out_unlock;
+               }
+               list_add_tail(&vdev->sriov_pfs_item, &vfio_pci_sriov_pfs);
+               mutex_unlock(&vfio_pci_sriov_pfs_mutex);
                ret = pci_enable_sriov(pdev, nr_virtfn);
+               if (ret)
+                       goto out_del;
+               ret = nr_virtfn;
+               goto out_put;
+       }
 
-       vfio_device_put(device);
+       pci_disable_sriov(pdev);
 
-       return ret < 0 ? ret : nr_virtfn;
+out_del:
+       mutex_lock(&vfio_pci_sriov_pfs_mutex);
+       list_del_init(&vdev->sriov_pfs_item);
+out_unlock:
+       mutex_unlock(&vfio_pci_sriov_pfs_mutex);
+out_put:
+       vfio_device_put(device);
+       return ret;
 }
 EXPORT_SYMBOL_GPL(vfio_pci_core_sriov_configure);
 

diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index ae6f4838ab75588ade80a15622b508e447ed0007..6e5db4edc33593607821c7a7c32fa0f75ddced96 100644 (file)
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -133,6 +133,8 @@ struct vfio_pci_core_device {
        struct mutex            ioeventfds_lock;
        struct list_head        ioeventfds_list;
        struct vfio_pci_vf_token        *vf_token;
+       struct list_head                sriov_pfs_item;
+       struct vfio_pci_core_device     *sriov_pf_core_dev;
        struct notifier_block   nb;
        struct mutex            vma_lock;
        struct list_head        vma_list;