On Thu, 4 Jun 2020, P J P wrote:
From: Prasad J Pandit <p...@fedoraproject.org>
While reading PCI configuration bytes, a guest may send an
address towards the end of the configuration space. It may lead
to an OOB access issue. Assert that 'address + len' is within
PCI configuration space.
Suggested-by: Philippe Mathieu-Daudé <phi...@redhat.com>
Signed-off-by: Prasad J Pandit <p...@fedoraproject.org>
---
hw/pci/pci.c | 2 ++
1 file changed, 2 insertions(+)
Update v2: assert PCI configuration access is within bounds
-> https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00711.html
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 70c66965f5..173bec4fd5 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1381,6 +1381,8 @@ uint32_t pci_default_read_config(PCIDevice *d,
{
uint32_t val = 0;
+ assert(address + len <= pci_config_size(d));
Does this allow guest now to crash QEMU? I think it was suggested that
assert should only be used for cases that can only arise from a
programming error and not from values set by the guest. If this is
considered to be an error now to call this function with wrong parameters
did you check other callers? I've found a few such as:
hw/scsi/esp-pci.c
hw/watchdog/wdt_i6300esb.c
hw/ide/cmd646.c
hw/vfio/pci.c
and maybe others. Would it be better to not crash just log invalid access
and either fix up parameters or return some garbage like 0?
Regards,
BALATON Zoltan
+
if (pci_is_express_downstream_port(d) &&
ranges_overlap(address, len, d->exp.exp_cap + PCI_EXP_LNKSTA, 2)) {
pcie_sync_bridge_lnk(d);
