vfio/pci: Copy hot-reset device info to userspace in the devices loop

This copies the vfio_pci_dependent_device to userspace during looping each
affected device for reporting vfio_pci_hot_reset_info. This avoids counting
the affected devices and allocating a potential large buffer to store the
vfio_pci_dependent_device of all the affected devices before copying them
to userspace.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Tested-by: Yanting Jiang <yanting.jiang@intel.com>
Tested-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/r/20230718105542.4138-10-yi.l.liu@intel.com
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 32506c5..f4a0ea0 100644 (file)
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -777,19 +777,25 @@ static int vfio_pci_count_devs(struct pci_dev *pdev, void *data)
 }
 
 struct vfio_pci_fill_info {
-       int max;
-       int cur;
-       struct vfio_pci_dependent_device *devices;
+       struct vfio_pci_dependent_device __user *devices;
+       struct vfio_pci_dependent_device __user *devices_end;
        struct vfio_device *vdev;
+       u32 count;
        u32 flags;
 };
 
 static int vfio_pci_fill_devs(struct pci_dev *pdev, void *data)
 {
+       struct vfio_pci_dependent_device info = {
+               .segment = pci_domain_nr(pdev->bus),
+               .bus = pdev->bus->number,
+               .devfn = pdev->devfn,
+       };
        struct vfio_pci_fill_info *fill = data;
 
-       if (fill->cur == fill->max)
-               return -EAGAIN; /* Something changed, try again */
+       fill->count++;
+       if (fill->devices >= fill->devices_end)
+               return 0;
 
        if (fill->flags & VFIO_PCI_HOT_RESET_FLAG_DEV_ID) {
                struct iommufd_ctx *iommufd = vfio_iommufd_device_ictx(fill->vdev);
@@ -802,19 +808,19 @@ static int vfio_pci_fill_devs(struct pci_dev *pdev, void *data)
                 */
                vdev = vfio_find_device_in_devset(dev_set, &pdev->dev);
                if (!vdev) {
-                       fill->devices[fill->cur].devid = VFIO_PCI_DEVID_NOT_OWNED;
+                       info.devid = VFIO_PCI_DEVID_NOT_OWNED;
                } else {
                        int id = vfio_iommufd_get_dev_id(vdev, iommufd);
 
                        if (id > 0)
-                               fill->devices[fill->cur].devid = id;
+                               info.devid = id;
                        else if (id == -ENOENT)
-                               fill->devices[fill->cur].devid = VFIO_PCI_DEVID_OWNED;
+                               info.devid = VFIO_PCI_DEVID_OWNED;
                        else
-                               fill->devices[fill->cur].devid = VFIO_PCI_DEVID_NOT_OWNED;
+                               info.devid = VFIO_PCI_DEVID_NOT_OWNED;
                }
                /* If devid is VFIO_PCI_DEVID_NOT_OWNED, clear owned flag. */
-               if (fill->devices[fill->cur].devid == VFIO_PCI_DEVID_NOT_OWNED)
+               if (info.devid == VFIO_PCI_DEVID_NOT_OWNED)
                        fill->flags &= ~VFIO_PCI_HOT_RESET_FLAG_DEV_ID_OWNED;
        } else {
                struct iommu_group *iommu_group;
@@ -823,13 +829,13 @@ static int vfio_pci_fill_devs(struct pci_dev *pdev, void *data)
                if (!iommu_group)
                        return -EPERM; /* Cannot reset non-isolated devices */
 
-               fill->devices[fill->cur].group_id = iommu_group_id(iommu_group);
+               info.group_id = iommu_group_id(iommu_group);
                iommu_group_put(iommu_group);
        }
-       fill->devices[fill->cur].segment = pci_domain_nr(pdev->bus);
-       fill->devices[fill->cur].bus = pdev->bus->number;
-       fill->devices[fill->cur].devfn = pdev->devfn;
-       fill->cur++;
+
+       if (copy_to_user(fill->devices, &info, sizeof(info)))
+               return -EFAULT;
+       fill->devices++;
        return 0;
 }
 
@@ -1259,8 +1265,7 @@ static int vfio_pci_ioctl_get_pci_hot_reset_info(
        unsigned long minsz =
                offsetofend(struct vfio_pci_hot_reset_info, count);
        struct vfio_pci_hot_reset_info hdr;
-       struct vfio_pci_fill_info fill = { 0 };
-       struct vfio_pci_dependent_device *devices = NULL;
+       struct vfio_pci_fill_info fill = {};
        bool slot = false;
        int ret = 0;
 
@@ -1278,29 +1283,9 @@ static int vfio_pci_ioctl_get_pci_hot_reset_info(
        else if (pci_probe_reset_bus(vdev->pdev->bus))
                return -ENODEV;
 
-       /* How many devices are affected? */
-       ret = vfio_pci_for_each_slot_or_bus(vdev->pdev, vfio_pci_count_devs,
-                                           &fill.max, slot);
-       if (ret)
-               return ret;
-
-       WARN_ON(!fill.max); /* Should always be at least one */
-
-       /*
-        * If there's enough space, fill it now, otherwise return -ENOSPC and
-        * the number of devices affected.
-        */
-       if (hdr.argsz < sizeof(hdr) + (fill.max * sizeof(*devices))) {
-               ret = -ENOSPC;
-               hdr.count = fill.max;
-               goto reset_info_exit;
-       }
-
-       devices = kcalloc(fill.max, sizeof(*devices), GFP_KERNEL);
-       if (!devices)
-               return -ENOMEM;
-
-       fill.devices = devices;
+       fill.devices = arg->devices;
+       fill.devices_end = arg->devices +
+                          (hdr.argsz - sizeof(hdr)) / sizeof(arg->devices[0]);
        fill.vdev = &vdev->vdev;
 
        if (vfio_device_cdev_opened(&vdev->vdev))
@@ -1311,29 +1296,17 @@ static int vfio_pci_ioctl_get_pci_hot_reset_info(
        ret = vfio_pci_for_each_slot_or_bus(vdev->pdev, vfio_pci_fill_devs,
                                            &fill, slot);
        mutex_unlock(&vdev->vdev.dev_set->lock);
+       if (ret)
+               return ret;
 
-       /*
-        * If a device was removed between counting and filling, we may come up
-        * short of fill.max.  If a device was added, we'll have a return of
-        * -EAGAIN above.
-        */
-       if (!ret) {
-               hdr.count = fill.cur;
-               hdr.flags = fill.flags;
-       }
-
-reset_info_exit:
+       hdr.count = fill.count;
+       hdr.flags = fill.flags;
        if (copy_to_user(arg, &hdr, minsz))
-               ret = -EFAULT;
-
-       if (!ret) {
-               if (copy_to_user(&arg->devices, devices,
-                                hdr.count * sizeof(*devices)))
-                       ret = -EFAULT;
-       }
+               return -EFAULT;
 
-       kfree(devices);
-       return ret;
+       if (fill.count > fill.devices - arg->devices)
+               return -ENOSPC;
+       return 0;
 }
 
 static int