[libvirt PATCH] util: validate pcie_cap_pos != 0 in virDeviceHasPCIExpressLink()

Laine Stump posted 1 patch 2 weeks, 4 days ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210106235102.880922-1-laine@redhat.com
src/util/virpci.c | 5 +++++
1 file changed, 5 insertions(+)

[libvirt PATCH] util: validate pcie_cap_pos != 0 in virDeviceHasPCIExpressLink()

Posted by Laine Stump 2 weeks, 4 days ago
virDeviceHasPCIExpressLink() wasn't checking that pcie_cap_pos was
valid before attempting to use it, which could lead to reading the
byte at offset 0+PCI_CAP_ID_EXP instead of [valid
offset]+PCI_CAP_ID_EXP. In particular, this could happen for
"integrated" PCI devices (those that are on the PCIe root complex). If
it happened that the byte from the wrong address had the "right" bit
set, then it would lead to us innappropriately believing that Express
Link info was available when it wasn't, and the node device driver
would log an error like this:

  virPCIDeviceGetLinkCapSta:2754 :
  internal error: pci device 0000:00:18.0 is not a PCI-Express device

during a libvirtd restart. (this didn't ever occur until after
virPCIDeviceIsPCIExpress() was made more intelligent in commit
c00b6b1ae, which hasn't yet been in any official release)

Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/util/virpci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 9bfc743fbd..50fd5ef7ea 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2722,6 +2722,11 @@ virPCIDeviceHasPCIExpressLink(virPCIDevicePtr dev)
     if (virPCIDeviceInit(dev, fd) < 0)
         goto cleanup;
 
+    if (dev->pcie_cap_pos == 0) {
+        ret = 0;
+        goto cleanup;
+    }
+
     cap = virPCIDeviceRead16(dev, fd, dev->pcie_cap_pos + PCI_CAP_FLAGS);
     type = (cap & PCI_EXP_FLAGS_TYPE) >> 4;
 
-- 
2.29.2

Re: [libvirt PATCH] util: validate pcie_cap_pos != 0 in virDeviceHasPCIExpressLink()

Posted by Michal Privoznik 2 weeks, 4 days ago
On 1/7/21 12:51 AM, Laine Stump wrote:
> virDeviceHasPCIExpressLink() wasn't checking that pcie_cap_pos was
> valid before attempting to use it, which could lead to reading the
> byte at offset 0+PCI_CAP_ID_EXP instead of [valid
> offset]+PCI_CAP_ID_EXP. In particular, this could happen for
> "integrated" PCI devices (those that are on the PCIe root complex). If
> it happened that the byte from the wrong address had the "right" bit
> set, then it would lead to us innappropriately believing that Express
> Link info was available when it wasn't, and the node device driver
> would log an error like this:
> 
>    virPCIDeviceGetLinkCapSta:2754 :
>    internal error: pci device 0000:00:18.0 is not a PCI-Express device
> 
> during a libvirtd restart. (this didn't ever occur until after
> virPCIDeviceIsPCIExpress() was made more intelligent in commit
> c00b6b1ae, which hasn't yet been in any official release)
> 
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>   src/util/virpci.c | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal