| From 580a34d73f9dc699b343764b07397b296ca2c292 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 13 Apr 2022 10:10:36 -0300 |
| Subject: vfio/pci: Fix vf_token mechanism when device-specific VF drivers are |
| used |
| |
| From: Jason Gunthorpe <jgg@nvidia.com> |
| |
| [ 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> |
| --- |
| drivers/vfio/pci/vfio_pci_core.c | 124 ++++++++++++++++++------------- |
| include/linux/vfio_pci_core.h | 2 + |
| 2 files changed, 76 insertions(+), 50 deletions(-) |
| |
| diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c |
| index 2e6409cc11ad..ef54ef11af55 100644 |
| --- 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 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev) |
| } |
| 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); |
| @@ -1916,7 +1912,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); |
| |
| @@ -1954,21 +1950,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 ae6f4838ab75..6e5db4edc335 100644 |
| --- 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; |
| -- |
| 2.35.1 |
| |