On Thu, 27 Nov 2025 at 00:12, Navid Emamdoost <[email protected]> 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 [email protected]
> ---
> 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
