Commit 5bdc12fed5 for qemu.org

commit 5bdc12fed592a1673f64ca1e8243046632f83a26
Author: Denis V. Lunev <den@openvz.org>
Date:   Tue Aug 18 09:55:48 2026 +0200

    hw/ide/ahci: drain the ports on teardown

    ahci_uninit() frees s->dev without touching the requests still in flight.
    The only blk_aio_cancel() for them lives in ahci_reset_port(), which the
    unplug path does not run, and the ide-hd child's own drain is deferred
    through call_rcu so it happens after the free. A guest that powers the
    root port slot off through SLTCTL, or writes the ACPI ejection register,
    while a read is outstanding therefore leaves the completion to run
    against freed memory.

    A plain device_del is not affected: the pciehp attention-button flow
    resets the secondary bus first, which cancels through the reset path.
    Surprise removal is what skips it.

    Cancelling the NCQ requests alone is not enough. IDEDMA and IDEBus are
    embedded in AHCIDevice, so a plain DMA read reaches the freed array
    through dma_blk_cb() and a PIO read through ide_buffered_readv_cb(),
    neither of which the NCQ bookkeeping covers. ide_exit() drains nothing
    and frees io_buffer, which an outstanding request may still target.

    Move the NCQ cancel loop into a helper, run it from ahci_uninit() too,
    and drain each port before ide_exit() so no class of request can outlive
    the allocation. Delete check_bh there as well; qemu_bh_new_guarded() in
    check_cmd() has no counterpart on this path either.

    Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4069
    Cc: John Snow <jsnow@redhat.com>
    Cc: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
    Signed-off-by: Denis V. Lunev <den@openvz.org>

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 2b2ef873e0..6b04762c4a 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -619,12 +619,37 @@ static void ahci_set_signature(AHCIDevice *ad, uint32_t sig)
                              s->lcyl, s->hcyl, sig);
 }

+static void ahci_cancel_ncq_requests(AHCIDevice *ad)
+{
+    int i;
+
+    for (i = 0; i < AHCI_MAX_CMDS; i++) {
+        NCQTransferState *ncq_tfs = &ad->ncq_tfs[i];
+        ncq_tfs->halt = false;
+        if (!ncq_tfs->used) {
+            continue;
+        }
+
+        if (ncq_tfs->aiocb) {
+            blk_aio_cancel(ncq_tfs->aiocb);
+            ncq_tfs->aiocb = NULL;
+        }
+
+        /* Maybe we just finished the request thanks to blk_aio_cancel() */
+        if (!ncq_tfs->used) {
+            continue;
+        }
+
+        qemu_sglist_destroy(&ncq_tfs->sglist);
+        ncq_tfs->used = 0;
+    }
+}
+
 static void ahci_reset_port(AHCIState *s, int port, IDEResetKind kind)
 {
     AHCIDevice *d = &s->dev[port];
     AHCIPortRegs *pr = &d->port_regs;
     IDEState *ide_state = &d->port.ifs[0];
-    int i;

     trace_ahci_reset_port(s, port);

@@ -645,27 +670,7 @@ static void ahci_reset_port(AHCIState *s, int port, IDEResetKind kind)
         return;
     }

-    /* reset ncq queue */
-    for (i = 0; i < AHCI_MAX_CMDS; i++) {
-        NCQTransferState *ncq_tfs = &s->dev[port].ncq_tfs[i];
-        ncq_tfs->halt = false;
-        if (!ncq_tfs->used) {
-            continue;
-        }
-
-        if (ncq_tfs->aiocb) {
-            blk_aio_cancel(ncq_tfs->aiocb);
-            ncq_tfs->aiocb = NULL;
-        }
-
-        /* Maybe we just finished the request thanks to blk_aio_cancel() */
-        if (!ncq_tfs->used) {
-            continue;
-        }
-
-        qemu_sglist_destroy(&ncq_tfs->sglist);
-        ncq_tfs->used = 0;
-    }
+    ahci_cancel_ncq_requests(d);

     s->dev[port].port_state = STATE_RUN;
     if (ide_state->drive_kind == IDE_CD) {
@@ -1659,8 +1664,29 @@ void ahci_uninit(AHCIState *s)
     for (i = 0; i < s->ports; i++) {
         AHCIDevice *ad = &s->dev[i];

+        /*
+         * Unplug does not go through a reset, so this is the only chance to
+         * detach the requests and the bottom half that would otherwise walk
+         * s->dev after it is freed below.
+         */
+        ahci_cancel_ncq_requests(ad);
+        if (ad->check_bh) {
+            qemu_bh_delete(ad->check_bh);
+            ad->check_bh = NULL;
+        }
+
         for (j = 0; j < 2; j++) {
-            ide_exit(&ad->port.ifs[j]);
+            IDEState *ide_state = &ad->port.ifs[j];
+
+            /*
+             * Everything the port still owns points into the allocation this
+             * function frees, io_buffer included, so nothing may be left in
+             * flight once ide_exit() has run.
+             */
+            if (ide_state->blk) {
+                blk_drain(ide_state->blk);
+            }
+            ide_exit(ide_state);
         }
         object_unparent(OBJECT(&ad->port));
     }