Re: [PATCH 3/3] hw/nvme: cancel inflight requests on controller reset

On 7/29/26 13:34, Minwoo Im wrote:
> nvme_ctrl_reset() freed every SQ/CQ right after nvme_ns_drain(), which
> only waits out requests on a per-namespace BlockBackend. That is safe
> as long as the guest first tore down I/O queues gracefully (Delete
> I/O SQ/CQ), since nvme_del_sq() already cancels and waits for
> anything left on a queue before freeing it.
> 
> A reset that happens without that graceful sequence first (e.g. an
> abrupt/asynchronous controller reset) can still have commands
> inflight on blk_aio_*. Freeing sq/cq before those complete leaves
> their completion callbacks (nvme_rw_cb() and friends) to run against
> already-freed NvmeRequest/NvmeSQueue/NvmeCQueue memory via
> nvme_enqueue_req_completion(), causing a use-after-free/segfault.
> 
> Run nvme_sq_cancel_inflight() over every queue in nvme_ctrl_reset()
> before the free loops, so no in-flight blk_aio_* callback can fire
> after sq/cq memory is freed.
> 
> Signed-off-by: Minwoo Im <[email protected]>
> ---
>   hw/nvme/ctrl.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 86b86b2a0fbb..22b1b9636878 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -8035,6 +8035,18 @@ static void nvme_ctrl_reset(NvmeCtrl *n, NvmeResetType rst)
>           nvme_ns_drain(ns);
>       }
>   
> +    /*
> +     * Cancel and wait out every inflight command on every queue first. A
> +     * reset is not required to be preceded by the guest's graceful
> +     * Delete I/O SQ/CQ sequence, so sq/cq must not be freed below while a
> +     * blk_aio_* completion for them could still be in flight.
> +     */
> +    for (i = 0; i < n->num_queues; i++) {
> +        if (n->sq[i] != NULL) {
> +            nvme_sq_cancel_inflight(n->sq[i], NVME_CMD_ABORT_SQ_DEL);
> +        }
> +    }
> +
>       for (i = 0; i < n->num_queues; i++) {
>           if (n->sq[i] != NULL) {
>               nvme_free_sq(n->sq[i], n);

BTW, can't the newly added loop be fold into the existing loop, like

    for (i = 0; i < n->num_queues; i++) {
        if (n->sq[i] != NULL) {
            nvme_sq_cancel_inflight(n->sq[i], NVME_CMD_ABORT_SQ_DEL);
            nvme_free_sq(n->sq[i], n);
        }
    }

Thanks,

/mjt