From: "Denis V. Lunev" <[email protected]> vfio_pci_read_config() signals a failed host-side read by returning (uint32_t)-1, regardless of the requested length. vfio_intx_enable() and vfio_pci_pre_reset() both narrowed that return value straight into a uint8_t/uint16_t local before checking anything, which truncates -1 into 0xff or 0xffff - values a real 1- or 2-byte register read can legitimately produce. From that point on, a failed read and real all-ones content are indistinguishable.
Keep the full uint32_t result and check it against (uint32_t)-1 before narrowing. In vfio_pci_pre_reset(), skip the corresponding write-back on a failed read instead of writing back constructed garbage to the device. Resolves: Coverity CID 1663684 Resolves: Coverity CID 1663688 Signed-off-by: Denis V. Lunev <[email protected]> CC: Alex Williamson <[email protected]> CC: Cédric Le Goater <[email protected]> Link: https://lore.kernel.org/qemu-devel/[email protected] [ clg: Added Coverity IDs ] Reviewed-by: Cédric Le Goater <[email protected]> Signed-off-by: Cédric Le Goater <[email protected]> --- hw/vfio/pci.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index dcfc92aae1c78d3a803665bcaea1e83f66a6d030..b8c937d4be50091691875f2253c4d03ba83df704 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -323,10 +323,16 @@ static void vfio_irqchip_change(Notifier *notify, void *data) static bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp) { PCIDevice *pdev = PCI_DEVICE(vdev); - uint8_t pin = vfio_pci_read_config(pdev, PCI_INTERRUPT_PIN, 1); + uint32_t val = vfio_pci_read_config(pdev, PCI_INTERRUPT_PIN, 1); + uint8_t pin; Error *err = NULL; int32_t fd; + if (val == (uint32_t)-1) { + error_setg(errp, "failed to read PCI_INTERRUPT_PIN"); + return false; + } + pin = val; if (!pin) { return true; @@ -2766,6 +2772,7 @@ bool vfio_pci_add_capabilities(VFIOPCIDevice *vdev, Error **errp) void vfio_pci_pre_reset(VFIOPCIDevice *vdev) { PCIDevice *pdev = PCI_DEVICE(vdev); + uint32_t val; uint16_t cmd; vfio_disable_interrupts(vdev); @@ -2774,23 +2781,34 @@ void vfio_pci_pre_reset(VFIOPCIDevice *vdev) * Stop any ongoing DMA by disconnecting I/O, MMIO, and bus master. * Also put INTx Disable in known state. */ - cmd = vfio_pci_read_config(pdev, PCI_COMMAND, 2); - cmd &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | - PCI_COMMAND_INTX_DISABLE); - vfio_pci_write_config(pdev, PCI_COMMAND, cmd, 2); + val = vfio_pci_read_config(pdev, PCI_COMMAND, 2); + if (val != (uint32_t)-1) { + cmd = val; + cmd &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | + PCI_COMMAND_INTX_DISABLE); + vfio_pci_write_config(pdev, PCI_COMMAND, cmd, 2); + } /* Make sure the device is in D0 */ if (pdev->pm_cap) { uint16_t pmcsr; uint8_t state; - pmcsr = vfio_pci_read_config(pdev, pdev->pm_cap + PCI_PM_CTRL, 2); + val = vfio_pci_read_config(pdev, pdev->pm_cap + PCI_PM_CTRL, 2); + if (val == (uint32_t)-1) { + return; + } + pmcsr = val; state = pmcsr & PCI_PM_CTRL_STATE_MASK; if (state) { pmcsr &= ~PCI_PM_CTRL_STATE_MASK; vfio_pci_write_config(pdev, pdev->pm_cap + PCI_PM_CTRL, pmcsr, 2); /* vfio handles the necessary delay here */ - pmcsr = vfio_pci_read_config(pdev, pdev->pm_cap + PCI_PM_CTRL, 2); + val = vfio_pci_read_config(pdev, pdev->pm_cap + PCI_PM_CTRL, 2); + if (val == (uint32_t)-1) { + return; + } + pmcsr = val; state = pmcsr & PCI_PM_CTRL_STATE_MASK; if (state) { error_report("vfio: Unable to power on device, stuck in D%d", -- 2.55.0
