tests/qtest: Rework libqos PCI BAR handling to support fuzzing

[PATCH v2 1/5] libqos: pci: Handle zero-sized BARs gracefully

Posted by Navid Emamdoost 2 days, 10 hours ago

The qpci_iomap() function would previously fail with a fatal assertion
if it probed a PCI BAR that had a size of zero. This is, however,
expected behavior for some devices like the Q35 host bridge, and the
assertion blocked the creation of new fuzzing targets.

Instead of asserting at map time, modify the QPCIBar struct to store
the BAR's size. Defer the safety check to the accessor functions
(qpci_io_readb, qpci_memread, etc.), which now assert that any
access is within the BAR's bounds.

Signed-off-by: Navid Emamdoost navidem@google.com
---
 tests/qtest/libqos/pci.c | 25 ++++++++++++++++++++++++-
 tests/qtest/libqos/pci.h |  1 +
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
index a59197b992..70caf382cc 100644
--- a/tests/qtest/libqos/pci.c
+++ b/tests/qtest/libqos/pci.c
@@ -396,6 +396,7 @@ void qpci_config_writel(QPCIDevice *dev, uint8_t offset, uint32_t value)
 
 uint8_t qpci_io_readb(QPCIDevice *dev, QPCIBar token, uint64_t off)
 {
+	g_assert(off + 1 <= token.size);
     QPCIBus *bus = dev->bus;
 
     if (token.is_io) {
@@ -410,6 +411,7 @@ uint8_t qpci_io_readb(QPCIDevice *dev, QPCIBar token, uint64_t off)
 
 uint16_t qpci_io_readw(QPCIDevice *dev, QPCIBar token, uint64_t off)
 {
+	g_assert(off + 2 <= token.size);
     QPCIBus *bus = dev->bus;
 
     if (token.is_io) {
@@ -424,6 +426,7 @@ uint16_t qpci_io_readw(QPCIDevice *dev, QPCIBar token, uint64_t off)
 
 uint32_t qpci_io_readl(QPCIDevice *dev, QPCIBar token, uint64_t off)
 {
+	g_assert(off + 4 <= token.size);
     QPCIBus *bus = dev->bus;
 
     if (token.is_io) {
@@ -438,6 +441,7 @@ uint32_t qpci_io_readl(QPCIDevice *dev, QPCIBar token, uint64_t off)
 
 uint64_t qpci_io_readq(QPCIDevice *dev, QPCIBar token, uint64_t off)
 {
+	g_assert(off + 8 <= token.size);
     QPCIBus *bus = dev->bus;
 
     if (token.is_io) {
@@ -453,6 +457,7 @@ uint64_t qpci_io_readq(QPCIDevice *dev, QPCIBar token, uint64_t off)
 void qpci_io_writeb(QPCIDevice *dev, QPCIBar token, uint64_t off,
                     uint8_t value)
 {
+	g_assert(off + 1 <= token.size);
     QPCIBus *bus = dev->bus;
 
     if (token.is_io) {
@@ -465,6 +470,7 @@ void qpci_io_writeb(QPCIDevice *dev, QPCIBar token, uint64_t off,
 void qpci_io_writew(QPCIDevice *dev, QPCIBar token, uint64_t off,
                     uint16_t value)
 {
+	g_assert(off + 2 <= token.size);
     QPCIBus *bus = dev->bus;
 
     if (token.is_io) {
@@ -478,6 +484,7 @@ void qpci_io_writew(QPCIDevice *dev, QPCIBar token, uint64_t off,
 void qpci_io_writel(QPCIDevice *dev, QPCIBar token, uint64_t off,
                     uint32_t value)
 {
+	g_assert(off + 4 <= token.size);
     QPCIBus *bus = dev->bus;
 
     if (token.is_io) {
@@ -491,6 +498,7 @@ void qpci_io_writel(QPCIDevice *dev, QPCIBar token, uint64_t off,
 void qpci_io_writeq(QPCIDevice *dev, QPCIBar token, uint64_t off,
                     uint64_t value)
 {
+	g_assert(off + 8 <= token.size);
     QPCIBus *bus = dev->bus;
 
     if (token.is_io) {
@@ -500,10 +508,10 @@ void qpci_io_writeq(QPCIDevice *dev, QPCIBar token, uint64_t off,
         bus->memwrite(bus, token.addr + off, &value, sizeof(value));
     }
 }
-
 void qpci_memread(QPCIDevice *dev, QPCIBar token, uint64_t off,
                   void *buf, size_t len)
 {
+	g_assert(off + len <= token.size);
     g_assert(!token.is_io);
     dev->bus->memread(dev->bus, token.addr + off, buf, len);
 }
@@ -511,6 +519,7 @@ void qpci_memread(QPCIDevice *dev, QPCIBar token, uint64_t off,
 void qpci_memwrite(QPCIDevice *dev, QPCIBar token, uint64_t off,
                    const void *buf, size_t len)
 {
+	g_assert(off + len <= token.size);
     g_assert(!token.is_io);
     dev->bus->memwrite(dev->bus, token.addr + off, buf, len);
 }
@@ -541,6 +550,19 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
         addr &= PCI_BASE_ADDRESS_MEM_MASK;
     }
 
+    if (!addr){
+        /*
+         * This is an unimplemented BAR. It is not a fatal error.
+         * We model it as a BAR with a size of zero. Any attempt to
+         * access it will be caught by assertions in the accessors.
+         */
+        if (sizeptr) {
+            *sizeptr = 0;
+        }
+        memset(&bar, 0, sizeof(bar));
+        return bar;
+    }
+
     g_assert(addr); /* Must have *some* size bits */
 
     size = 1U << ctz32(addr);
@@ -572,6 +594,7 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
     }
 
     bar.addr = loc;
+    bar.size = size;
     return bar;
 }
 
diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h
index 8389614523..e790e5293d 100644
--- a/tests/qtest/libqos/pci.h
+++ b/tests/qtest/libqos/pci.h
@@ -58,6 +58,7 @@ struct QPCIBus {
 
 struct QPCIBar {
     uint64_t addr;
+    uint64_t size;
     bool is_io;
 };
 
-- 
2.52.0.158.g65b55ccf14-goog

Re: [PATCH v2 1/5] libqos: pci: Handle zero-sized BARs gracefully

Posted by Peter Maydell 1 day, 21 hours ago

On Thu, 27 Nov 2025 at 00:12, Navid Emamdoost <navidem@google.com> wrote:
>
> The qpci_iomap() function would previously fail with a fatal assertion
> if it probed a PCI BAR that had a size of zero. This is, however,
> expected behavior for some devices like the Q35 host bridge, and the
> assertion blocked the creation of new fuzzing targets.
>
> Instead of asserting at map time, modify the QPCIBar struct to store
> the BAR's size. Defer the safety check to the accessor functions
> (qpci_io_readb, qpci_memread, etc.), which now assert that any
> access is within the BAR's bounds.
>
> Signed-off-by: Navid Emamdoost navidem@google.com
> ---
>  tests/qtest/libqos/pci.c | 25 ++++++++++++++++++++++++-
>  tests/qtest/libqos/pci.h |  1 +
>  2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
> index a59197b992..70caf382cc 100644
> --- a/tests/qtest/libqos/pci.c
> +++ b/tests/qtest/libqos/pci.c
> @@ -396,6 +396,7 @@ void qpci_config_writel(QPCIDevice *dev, uint8_t offset, uint32_t value)
>
>  uint8_t qpci_io_readb(QPCIDevice *dev, QPCIBar token, uint64_t off)
>  {
> +       g_assert(off + 1 <= token.size);
>      QPCIBus *bus = dev->bus;

The indent seems to be wrong for all your changes to these functions?

Also, we need "make check" to pass for every commit in the
patchset, not just after it has all been applied. So we need
to make the fixes that you have in patches 2-4 before we
can start enforcing the size limits with assertions.

> @@ -541,6 +550,19 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
>          addr &= PCI_BASE_ADDRESS_MEM_MASK;
>      }
>
> +    if (!addr){

Missing space before "{". (scripts/checkpatch.pl will
probably catch this kind of style error.)

> +        /*
> +         * This is an unimplemented BAR. It is not a fatal error.
> +         * We model it as a BAR with a size of zero. Any attempt to
> +         * access it will be caught by assertions in the accessors.
> +         */
> +        if (sizeptr) {
> +            *sizeptr = 0;
> +        }
> +        memset(&bar, 0, sizeof(bar));
> +        return bar;
> +    }
> +
>      g_assert(addr); /* Must have *some* size bits */

We can drop this assert now, because we just dealt with
the addr == 0 case.

>      size = 1U << ctz32(addr);
> @@ -572,6 +594,7 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
>      }
>
>      bar.addr = loc;
> +    bar.size = size;
>      return bar;
>  }

thanks
-- PMM

[PATCH v2 1/5] libqos: pci: Handle zero-sized BARs gracefully
[PATCH v2 2/5] libqos: pci: Require size for legacy I/O port mapping
[PATCH v2 3/5] tests/qtest: ahci-test: Check only implemented ports in verify_state
[PATCH v2 4/5] tests/qtest: Rework nvmetest_oob_cmb_test for BAR check
[PATCH v2 5/5] tests/qtest/fuzz: Add generic fuzzer for pcie-pci-bridge