hw/vfio/pci-quirks.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
Coverity reports a tained scalar when traversing the capabilities
chain (CID 1516589). In practice I've never seen a device with a
chain so broken as to cause an issue, but it's also pretty easy to
sanitize.
Fixes: f6b30c1984f7 ("hw/vfio/pci-quirks: Support alternate offset for GPUDirect Cliques")
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/vfio/pci-quirks.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 0ed2fcd53152..f4ff83680572 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1530,6 +1530,12 @@ const PropertyInfo qdev_prop_nv_gpudirect_clique = {
.set = set_nv_gpudirect_clique_id,
};
+static bool is_valid_std_cap_offset(uint8_t pos)
+{
+ return (pos >= PCI_STD_HEADER_SIZEOF &&
+ pos <= (PCI_CFG_SPACE_SIZE - PCI_CAP_SIZEOF));
+}
+
static int vfio_add_nv_gpudirect_cap(VFIOPCIDevice *vdev, Error **errp)
{
PCIDevice *pdev = &vdev->pdev;
@@ -1563,7 +1569,7 @@ static int vfio_add_nv_gpudirect_cap(VFIOPCIDevice *vdev, Error **errp)
*/
ret = pread(vdev->vbasedev.fd, &tmp, 1,
vdev->config_offset + PCI_CAPABILITY_LIST);
- if (ret != 1 || !tmp) {
+ if (ret != 1 || !is_valid_std_cap_offset(tmp)) {
error_setg(errp, "NVIDIA GPUDirect Clique ID: error getting cap list");
return -EINVAL;
}
@@ -1575,7 +1581,7 @@ static int vfio_add_nv_gpudirect_cap(VFIOPCIDevice *vdev, Error **errp)
d4_conflict = true;
}
tmp = pdev->config[tmp + PCI_CAP_LIST_NEXT];
- } while (tmp);
+ } while (is_valid_std_cap_offset(tmp));
if (!c8_conflict) {
pos = 0xC8;
--
2.40.1
On 7/1/23 00:36, Alex Williamson wrote: > Coverity reports a tained scalar when traversing the capabilities > chain (CID 1516589). In practice I've never seen a device with a > chain so broken as to cause an issue, but it's also pretty easy to > sanitize. > > Fixes: f6b30c1984f7 ("hw/vfio/pci-quirks: Support alternate offset for GPUDirect Cliques") > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Nice. I was just looking at the coverity report ! You beat me at it. is_valid_std_cap_offset() could be reused in other places where QEMU scans the PCI capabilities I think. Thanks, C. > --- > hw/vfio/pci-quirks.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c > index 0ed2fcd53152..f4ff83680572 100644 > --- a/hw/vfio/pci-quirks.c > +++ b/hw/vfio/pci-quirks.c > @@ -1530,6 +1530,12 @@ const PropertyInfo qdev_prop_nv_gpudirect_clique = { > .set = set_nv_gpudirect_clique_id, > }; > > +static bool is_valid_std_cap_offset(uint8_t pos) > +{ > + return (pos >= PCI_STD_HEADER_SIZEOF && > + pos <= (PCI_CFG_SPACE_SIZE - PCI_CAP_SIZEOF)); > +} > + > static int vfio_add_nv_gpudirect_cap(VFIOPCIDevice *vdev, Error **errp) > { > PCIDevice *pdev = &vdev->pdev; > @@ -1563,7 +1569,7 @@ static int vfio_add_nv_gpudirect_cap(VFIOPCIDevice *vdev, Error **errp) > */ > ret = pread(vdev->vbasedev.fd, &tmp, 1, > vdev->config_offset + PCI_CAPABILITY_LIST); > - if (ret != 1 || !tmp) { > + if (ret != 1 || !is_valid_std_cap_offset(tmp)) { > error_setg(errp, "NVIDIA GPUDirect Clique ID: error getting cap list"); > return -EINVAL; > } > @@ -1575,7 +1581,7 @@ static int vfio_add_nv_gpudirect_cap(VFIOPCIDevice *vdev, Error **errp) > d4_conflict = true; > } > tmp = pdev->config[tmp + PCI_CAP_LIST_NEXT]; > - } while (tmp); > + } while (is_valid_std_cap_offset(tmp)); > > if (!c8_conflict) { > pos = 0xC8;
© 2016 - 2024 Red Hat, Inc.