On Thu, 27 Nov 2025 at 00:12, Navid Emamdoost <navidem@google.com> 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 <navidem@google.com>
> ---
> 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 <peter.maydell@linaro.org>
thanks
-- PMM