From: Thomas Huth <[email protected]> ide_cancel_dma_sync() is called with a "IDEState *s" for one of the two IDE drives on a bus (primary or secondary drive) to cancel all pending DMA transfers on the drive. The code then checks s->bus->dma->aiocb to see whether there is any IO in flight on the *bus* and then calls blk_drain(s->blk) to wait for its completion. However, s->bus->dma->aiocb might belong to the other drive on the bus, and if there is no disk attached to the current drive, s->blk is NULL. Since blk_drain() does not check its parameter for a NULL pointer, QEMU can crash in such a case.
To fix the problem, we have to check that "blk" is not NULL before calling blk_drain(). And we have to call blk_drain() for both drives, otherwise the assert(s->bus->dma->aiocb == NULL) statement after the blk_drain() might trigger if the IO in flight belongs to the the other drive. Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/905 Reported-by: Alexander Bulekov <[email protected]> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4052 Reported-by: dong ling Signed-off-by: Thomas Huth <[email protected]> Message-ID: <[email protected]> --- hw/ide/core.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 4c1ee19d8e8..fb9bf11b455 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -741,10 +741,17 @@ void ide_cancel_dma_sync(IDEState *s) * In the future we'll be able to safely cancel the I/O if the * whole DMA operation will be submitted to disk with a single * aio operation with preadv/pwritev. + * + * Note: s->bus->dma->aiocb might belong to the adjacent IDEState, + * so we have to drain both drives to get it cleared. */ if (s->bus->dma->aiocb) { trace_ide_cancel_dma_sync_remaining(); - blk_drain(s->blk); + for (int i = 0; i < 2; i++) { + if (s->bus->ifs[i].blk) { + blk_drain(s->bus->ifs[i].blk); + } + } assert(s->bus->dma->aiocb == NULL); } } -- 2.55.0
