On Thu, 27 Nov 2025 at 00:12, Navid Emamdoost <[email protected]> wrote: > > The verify_state helper function in ahci-test.c incorrectly > assumed that all 32 potential AHCI ports were implemented. During post- > migration checks, it would loop through all 32 ports, attempting to > read registers for non-existent ones. > This resulted in an out-of-bounds access on the main HBA BAR. This > latent bug was exposed by the recent introduction of strict bounds > checking in the libqos PCI accessors, which now correctly triggers a > fatal assertion. > Fix this by modifying the loop in verify_state to first read the > AHCI_PI (Ports Implemented) register and then only check the state > for ports that the device reports as present. > > Signed-off-by: Navid Emamdoost <[email protected]> > --- > tests/qtest/ahci-test.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c > index e8aabfc13f..06c5bd08d8 100644 > --- a/tests/qtest/ahci-test.c > +++ b/tests/qtest/ahci-test.c > @@ -81,6 +81,7 @@ static void string_bswap16(uint16_t *s, size_t bytes) > static void verify_state(AHCIQState *ahci, uint64_t hba_old) > { > int i, j; > + uint32_t ports_impl; > uint32_t ahci_fingerprint; > uint64_t hba_base; > AHCICommandHeader cmd; > @@ -99,7 +100,14 @@ static void verify_state(AHCIQState *ahci, uint64_t > hba_old) > g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP), ==, ahci->cap); > g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP2), ==, ahci->cap2); > > + ports_impl = ahci_rreg(ahci, AHCI_PI); > + > for (i = 0; i < 32; i++) { > + > + if (!(ports_impl & (1 << i))) {
We should use "1U << i" here, because coverity will probably complain about shifting into the sign bit of a signed integer otherwise. > + continue; /* Skip unimplemented ports */ > + } > + > g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_FB), ==, > ahci->port[i].fb); > g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_CLB), ==, Otherwise Reviewed-by: Peter Maydell <[email protected]> thanks -- PMM
