| Message ID | 20240516174831.2257970-1-alex.williamson@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio/pci: Restore zero affected bus reset devices warning | expand |
On 5/16/24 19:48, Alex Williamson wrote: > Yi notes relative to commit f6944d4a0b87 ("vfio/pci: Collect hot-reset > devices to local buffer") that we previously tested the resulting > device count with a WARN_ON, which was removed when we switched to > the in-loop user copy in commit b56b7aabcf3c ("vfio/pci: Copy hot-reset > device info to userspace in the devices loop"). Finding no devices in > the bus/slot would be an unexpected condition, so let's restore the > warning and trigger a -ERANGE error here as success with no devices > would be an unexpected result to userspace as well. > > Suggested-by: Yi Liu <yi.l.liu@intel.com> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > drivers/vfio/pci/vfio_pci_core.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c > index d8c95cc16be8..80cae87fff36 100644 > --- a/drivers/vfio/pci/vfio_pci_core.c > +++ b/drivers/vfio/pci/vfio_pci_core.c > @@ -1281,6 +1281,9 @@ static int vfio_pci_ioctl_get_pci_hot_reset_info( > if (ret) > return ret; > > + if (WARN_ON(!count)) /* Should always be at least one */ > + return -ERANGE; > + > if (count > (hdr.argsz - sizeof(hdr)) / sizeof(*devices)) { > hdr.count = count; > ret = -ENOSPC;
> From: Alex Williamson <alex.williamson@redhat.com> > Sent: Friday, May 17, 2024 1:49 AM > > Yi notes relative to commit f6944d4a0b87 ("vfio/pci: Collect hot-reset > devices to local buffer") that we previously tested the resulting > device count with a WARN_ON, which was removed when we switched to > the in-loop user copy in commit b56b7aabcf3c ("vfio/pci: Copy hot-reset > device info to userspace in the devices loop"). Finding no devices in > the bus/slot would be an unexpected condition, so let's restore the > warning and trigger a -ERANGE error here as success with no devices > would be an unexpected result to userspace as well. > > Suggested-by: Yi Liu <yi.l.liu@intel.com> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > --- > drivers/vfio/pci/vfio_pci_core.c | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Yi Liu <yi.l.liu@intel.com> > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c > index d8c95cc16be8..80cae87fff36 100644 > --- a/drivers/vfio/pci/vfio_pci_core.c > +++ b/drivers/vfio/pci/vfio_pci_core.c > @@ -1281,6 +1281,9 @@ static int vfio_pci_ioctl_get_pci_hot_reset_info( > if (ret) > return ret; > > + if (WARN_ON(!count)) /* Should always be at least one */ > + return -ERANGE; > + > if (count > (hdr.argsz - sizeof(hdr)) / sizeof(*devices)) { > hdr.count = count; > ret = -ENOSPC; > -- > 2.44.0 Regards, Yi Liu
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index d8c95cc16be8..80cae87fff36 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1281,6 +1281,9 @@ static int vfio_pci_ioctl_get_pci_hot_reset_info( if (ret) return ret; + if (WARN_ON(!count)) /* Should always be at least one */ + return -ERANGE; + if (count > (hdr.argsz - sizeof(hdr)) / sizeof(*devices)) { hdr.count = count; ret = -ENOSPC;
Yi notes relative to commit f6944d4a0b87 ("vfio/pci: Collect hot-reset devices to local buffer") that we previously tested the resulting device count with a WARN_ON, which was removed when we switched to the in-loop user copy in commit b56b7aabcf3c ("vfio/pci: Copy hot-reset device info to userspace in the devices loop"). Finding no devices in the bus/slot would be an unexpected condition, so let's restore the warning and trigger a -ERANGE error here as success with no devices would be an unexpected result to userspace as well. Suggested-by: Yi Liu <yi.l.liu@intel.com> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- drivers/vfio/pci/vfio_pci_core.c | 3 +++ 1 file changed, 3 insertions(+)