[PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds

[PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds

[Navid Emamdoost]

The qpci_iomap() function fails with a fatal g_assert(addr) if it
probes a PCI BAR that has a size of zero. This is expected behavior
for certain devices, like the Q35 PCI Host Bridge, which have valid but
unimplemented BARs.
This assertion blocks the creation of fuzz targets for complex machine
types that include these devices.
Make the check conditional on !CONFIG_FUZZ. In fuzzing builds, a
zero-sized BAR is now handled gracefully by returning an empty BAR
struct, allowing fuzzing to proceed. The original assertion is kept for
all other builds to maintain strict checking for qtest and production
environments.

Signed-off-by: Navid Emamdoost <navidem@google.com>
---
 tests/qtest/libqos/pci.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
index a59197b992..df9e2a3993 100644
--- a/tests/qtest/libqos/pci.c
+++ b/tests/qtest/libqos/pci.c
@@ -541,6 +541,22 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
         addr &= PCI_BASE_ADDRESS_MEM_MASK;
     }
 
+#ifdef CONFIG_FUZZ
+    /*
+     * During fuzzing runs, an unimplemented BAR (addr=0) is not a fatal
+     * error. This occurs when probing devices like the Q35 host bridge. We
+     * return gracefully to allow fuzzing to continue. In non-fuzzing builds,
+     * we retain the original g_assert() to catch unexpected behavior.
+     */
+    if (!addr) {
+        if (sizeptr) {
+            *sizeptr = 0;
+        }
+        memset(&bar, 0, sizeof(bar));
+        return bar;
+    }
+#endif
+
     g_assert(addr); /* Must have *some* size bits */
 
     size = 1U << ctz32(addr);
-- 
2.51.0.710.ga91ca5db03-goog

[PATCH 2/2] tests/qtest/fuzz: Add generic fuzzer for pcie-pci-bridge

[Navid Emamdoost]

Add a new generic fuzz target for the 'pcie-pci-bridge' device. This
target uses a Q35 machine with a multi-level PCI hierarchy to exercise
the bridge's functionality.
This is made possible by the preceding change to handle unimplemented
BARs during fuzzing.

---

This new target significantly improves code coverage for the pcie-pci-bridge
implementation. The baseline coverage shown below was generated by
running all existing fuzz targets with the oss-fuzz corpus.

=== Component: hw/pci  ===
-------------------------------------------------------------------------------
File                           New Target           Baseline             Change
-------------------------------------------------------------------------------
shpc.c                         359/511 (70.3%)      0/511 (0.0%)         +359
pci_bridge.c                   255/304 (83.9%)      12/304 (3.9%)        +243
pcie.c                         390/774 (50.4%)      160/774 (20.7%)      +230
pcie_aer.c                     119/524 (22.7%)      38/524 (7.3%)        +81
pci.c                          1154/2069 (55.8%)    1084/2069 (52.4%)    +70
pcie_port.c                    58/119 (48.7%)       17/119 (14.3%)       +41
pci.h                          86/132 (65.2%)       81/132 (61.4%)       +5

=== Component: hw/pci-bridge  ===
-------------------------------------------------------------------------------
File                           New Target           Baseline             Change
-------------------------------------------------------------------------------
pcie_root_port.c               86/127 (67.7%)       13/127 (10.2%)       +73
pcie_pci_bridge.c              62/94 (66.0%)        20/94 (21.3%)        +42
gen_pcie_root_port.c           45/66 (68.2%)        19/66 (28.8%)        +26

Signed-off-by: Navid Emamdoost <navidem@google.com>
---
 tests/qtest/fuzz/generic_fuzz_configs.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h b/tests/qtest/fuzz/generic_fuzz_configs.h
index ef0ad95712..e025f57a3e 100644
--- a/tests/qtest/fuzz/generic_fuzz_configs.h
+++ b/tests/qtest/fuzz/generic_fuzz_configs.h
@@ -247,6 +247,14 @@ const generic_fuzz_config predefined_configs[] = {
         .args = "-machine q35 -nodefaults "
         "-parallel file:/dev/null",
         .objects = "parallel*",
+    },{
+        .name = "pcie-pci-bridge",
+        .args = "-machine q35 -nodefaults "
+        "-device pcie-root-port,port=0x10,chassis=1,id=pci.1,bus=pcie.0,multifunction=true,addr=0x2 "
+        "-device pcie-pci-bridge,id=pci.2,bus=pci.1,addr=0x0 "
+        "-netdev user,id=net0 "
+        "-device e1000,netdev=net0,id=nic0,bus=pci.2,addr=0x3",
+        .objects = "pci* shpc*"
     }
 };
 
-- 
2.51.0.710.ga91ca5db03-goog

Re: [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds

[Alexander Bulekov]

On 251008 1919, Navid Emamdoost wrote:
> The qpci_iomap() function fails with a fatal g_assert(addr) if it
> probes a PCI BAR that has a size of zero. This is expected behavior
> for certain devices, like the Q35 PCI Host Bridge, which have valid but
> unimplemented BARs.
> This assertion blocks the creation of fuzz targets for complex machine
> types that include these devices.
> Make the check conditional on !CONFIG_FUZZ. In fuzzing builds, a
> zero-sized BAR is now handled gracefully by returning an empty BAR
> struct, allowing fuzzing to proceed. The original assertion is kept for
> all other builds to maintain strict checking for qtest and production
> environments.

Is there a way to determine whether a BAR is unimplememnted from the
PCIDev in generic_fuzz.c:pci_enum so that we can skip the call to iomap?

>
> Signed-off-by: Navid Emamdoost <navidem@google.com>
> ---
>  tests/qtest/libqos/pci.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
> index a59197b992..df9e2a3993 100644
> --- a/tests/qtest/libqos/pci.c
> +++ b/tests/qtest/libqos/pci.c
> @@ -541,6 +541,22 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
>          addr &= PCI_BASE_ADDRESS_MEM_MASK;
>      }
>
> +#ifdef CONFIG_FUZZ
> +    /*
> +     * During fuzzing runs, an unimplemented BAR (addr=0) is not a fatal
> +     * error. This occurs when probing devices like the Q35 host bridge. We
> +     * return gracefully to allow fuzzing to continue. In non-fuzzing builds,
> +     * we retain the original g_assert() to catch unexpected behavior.
> +     */
> +    if (!addr) {
> +        if (sizeptr) {
> +            *sizeptr = 0;
> +        }
> +        memset(&bar, 0, sizeof(bar));
> +        return bar;
> +    }
> +#endif
> +
>      g_assert(addr); /* Must have *some* size bits */
>
>      size = 1U << ctz32(addr);
> --
> 2.51.0.710.ga91ca5db03-goog
>
>

Re: [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds

[Navid Emamdoost]

Hi Alexander,

On Fri, Oct 10, 2025 at 8:59 AM Alexander Bulekov <alxndr@bu.edu> wrote:
>
> On 251008 1919, Navid Emamdoost wrote:
> > The qpci_iomap() function fails with a fatal g_assert(addr) if it
> > probes a PCI BAR that has a size of zero. This is expected behavior
> > for certain devices, like the Q35 PCI Host Bridge, which have valid but
> > unimplemented BARs.
> > This assertion blocks the creation of fuzz targets for complex machine
> > types that include these devices.
> > Make the check conditional on !CONFIG_FUZZ. In fuzzing builds, a
> > zero-sized BAR is now handled gracefully by returning an empty BAR
> > struct, allowing fuzzing to proceed. The original assertion is kept for
> > all other builds to maintain strict checking for qtest and production
> > environments.
>
> Is there a way to determine whether a BAR is unimplememnted from the
> PCIDev in generic_fuzz.c:pci_enum so that we can skip the call to iomap?
>

Fair point. I don't think we have a reliable way to determine if a BAR
is truly unimplemented from the PCIDevice model without probing it. If
we moved that hardware probe into pci_enum, it would become
inefficient for all the BARs that are implemented, as they would be
probed twice: once in pci_enum just to check, and then again inside
qpci_iomap to do the actual mapping. That's why I think delegating
this check to qpci_iomap is the cleaner approach.

> >
> > Signed-off-by: Navid Emamdoost <navidem@google.com>
> > ---
> >  tests/qtest/libqos/pci.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
> > index a59197b992..df9e2a3993 100644
> > --- a/tests/qtest/libqos/pci.c
> > +++ b/tests/qtest/libqos/pci.c
> > @@ -541,6 +541,22 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
> >          addr &= PCI_BASE_ADDRESS_MEM_MASK;
> >      }
> >
> > +#ifdef CONFIG_FUZZ
> > +    /*
> > +     * During fuzzing runs, an unimplemented BAR (addr=0) is not a fatal
> > +     * error. This occurs when probing devices like the Q35 host bridge. We
> > +     * return gracefully to allow fuzzing to continue. In non-fuzzing builds,
> > +     * we retain the original g_assert() to catch unexpected behavior.
> > +     */
> > +    if (!addr) {
> > +        if (sizeptr) {
> > +            *sizeptr = 0;
> > +        }
> > +        memset(&bar, 0, sizeof(bar));
> > +        return bar;
> > +    }
> > +#endif
> > +
> >      g_assert(addr); /* Must have *some* size bits */
> >
> >      size = 1U << ctz32(addr);
> > --
> > 2.51.0.710.ga91ca5db03-goog
> >
> >



-- 
Thank you,
Navid.

Re: [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds

[Navid Emamdoost]

On Mon, Oct 13, 2025 at 6:14 PM Navid Emamdoost <navidem@google.com> wrote:
>
> Hi Alexander,
>
> On Fri, Oct 10, 2025 at 8:59 AM Alexander Bulekov <alxndr@bu.edu> wrote:
> >
> > On 251008 1919, Navid Emamdoost wrote:
> > > The qpci_iomap() function fails with a fatal g_assert(addr) if it
> > > probes a PCI BAR that has a size of zero. This is expected behavior
> > > for certain devices, like the Q35 PCI Host Bridge, which have valid but
> > > unimplemented BARs.
> > > This assertion blocks the creation of fuzz targets for complex machine
> > > types that include these devices.
> > > Make the check conditional on !CONFIG_FUZZ. In fuzzing builds, a
> > > zero-sized BAR is now handled gracefully by returning an empty BAR
> > > struct, allowing fuzzing to proceed. The original assertion is kept for
> > > all other builds to maintain strict checking for qtest and production
> > > environments.
> >
> > Is there a way to determine whether a BAR is unimplememnted from the
> > PCIDev in generic_fuzz.c:pci_enum so that we can skip the call to iomap?
> >
>
> Fair point. I don't think we have a reliable way to determine if a BAR
> is truly unimplemented from the PCIDevice model without probing it. If
> we moved that hardware probe into pci_enum, it would become
> inefficient for all the BARs that are implemented, as they would be
> probed twice: once in pci_enum just to check, and then again inside
> qpci_iomap to do the actual mapping. That's why I think delegating
> this check to qpci_iomap is the cleaner approach.
>

Friendly ping.

> > >
> > > Signed-off-by: Navid Emamdoost <navidem@google.com>
> > > ---
> > >  tests/qtest/libqos/pci.c | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > >
> > > diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
> > > index a59197b992..df9e2a3993 100644
> > > --- a/tests/qtest/libqos/pci.c
> > > +++ b/tests/qtest/libqos/pci.c
> > > @@ -541,6 +541,22 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
> > >          addr &= PCI_BASE_ADDRESS_MEM_MASK;
> > >      }
> > >
> > > +#ifdef CONFIG_FUZZ
> > > +    /*
> > > +     * During fuzzing runs, an unimplemented BAR (addr=0) is not a fatal
> > > +     * error. This occurs when probing devices like the Q35 host bridge. We
> > > +     * return gracefully to allow fuzzing to continue. In non-fuzzing builds,
> > > +     * we retain the original g_assert() to catch unexpected behavior.
> > > +     */
> > > +    if (!addr) {
> > > +        if (sizeptr) {
> > > +            *sizeptr = 0;
> > > +        }
> > > +        memset(&bar, 0, sizeof(bar));
> > > +        return bar;
> > > +    }
> > > +#endif
> > > +
> > >      g_assert(addr); /* Must have *some* size bits */
> > >
> > >      size = 1U << ctz32(addr);
> > > --
> > > 2.51.0.710.ga91ca5db03-goog
> > >
> > >
>
>
>
> --
> Thank you,
> Navid.



-- 
Thank you,
Navid.

Re: [PATCH 1/2] libqos: pci: Avoid fatal assert on zero-sized BARs in fuzz builds

[Peter Maydell]

On Wed, 8 Oct 2025 at 20:21, Navid Emamdoost <navidem@google.com> wrote:
>
> The qpci_iomap() function fails with a fatal g_assert(addr) if it
> probes a PCI BAR that has a size of zero. This is expected behavior
> for certain devices, like the Q35 PCI Host Bridge, which have valid but
> unimplemented BARs.
> This assertion blocks the creation of fuzz targets for complex machine
> types that include these devices.
> Make the check conditional on !CONFIG_FUZZ. In fuzzing builds, a
> zero-sized BAR is now handled gracefully by returning an empty BAR
> struct, allowing fuzzing to proceed. The original assertion is kept for
> all other builds to maintain strict checking for qtest and production
> environments.
>
> Signed-off-by: Navid Emamdoost <navidem@google.com>
> ---
>  tests/qtest/libqos/pci.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
> index a59197b992..df9e2a3993 100644
> --- a/tests/qtest/libqos/pci.c
> +++ b/tests/qtest/libqos/pci.c
> @@ -541,6 +541,22 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
>          addr &= PCI_BASE_ADDRESS_MEM_MASK;
>      }
>
> +#ifdef CONFIG_FUZZ
> +    /*
> +     * During fuzzing runs, an unimplemented BAR (addr=0) is not a fatal
> +     * error. This occurs when probing devices like the Q35 host bridge. We
> +     * return gracefully to allow fuzzing to continue. In non-fuzzing builds,
> +     * we retain the original g_assert() to catch unexpected behavior.
> +     */
> +    if (!addr) {
> +        if (sizeptr) {
> +            *sizeptr = 0;
> +        }
> +        memset(&bar, 0, sizeof(bar));
> +        return bar;
> +    }
> +#endif

Personally I think I'd prefer it if we didn't make this
dependent on CONFIG_FUZZ. If BARs with no size are
a valid thing then the libqos API should handle them
(e.g. in theory one should be able to write a test that the
Q35 host bridge provides the addr=0 BARs it is supposed to).

I think if we added the size to the QPCIBar struct then we
could assert in the accessors like qpci_io_readb() and
friends that the offset provided was in range. That would
catch both the unlikely "we implemented the BAR with no
size" case and the rather more likely "we got the size too
small" case (and also the "bug in the test and it got the
offset too big" case), and would mean that we don't lose
anything by not asserting that we have a non-zero-size BAR here.

What do you think?

thanks
-- PMM