Message ID | 20230630223608.650555-1-alex.williamson@redhat.com |
---|---|
State | New |
Headers | show |
Series | hw/vfio/pci-quirks: Sanitize capability pointer | expand |
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;
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;
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(-)