Re: [PATCH] hw/ide: check null block pointer before blk_drain

[P J P]

+-- On Mon, 31 Aug 2020, Philippe Mathieu-Daudé wrote --+
| > +++ b/hw/ide/core.c
| > @@ -718,7 +718,7 @@ void ide_cancel_dma_sync(IDEState *s)
| > -    if (s->bus->dma->aiocb) {
| > +    if (s->blk && s->bus->dma->aiocb) {
| 
| But s->blk mustn't be null here... IMHO we should assert() here and add a
| check earlier.

===
diff --git a/hw/ide/core.c b/hw/ide/core.c
index f76f7e5234..8105187f49 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -718,6 +718,7 @@ void ide_cancel_dma_sync(IDEState *s)
      * whole DMA operation will be submitted to disk with a single
      * aio operation with preadv/pwritev.
      */
+    assert(s->blk);
     if (s->bus->dma->aiocb) {
         trace_ide_cancel_dma_sync_remaining();
         blk_drain(s->blk);
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index b50091b615..51bb9c9abc 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -295,8 +295,11 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val)
     /* Ignore writes to SSBM if it keeps the old value */
     if ((val & BM_CMD_START) != (bm->cmd & BM_CMD_START)) {
         if (!(val & BM_CMD_START)) {
-            ide_cancel_dma_sync(idebus_active_if(bm->bus));
-            bm->status &= ~BM_STATUS_DMAING;
+            IDEState *s = idebus_active_if(bm->bus);
+            if (s->blk) {
+                ide_cancel_dma_sync(s);
+                bm->status &= ~BM_STATUS_DMAING;
+            }
         } else {
             bm->cur_addr = bm->addr;
             if (!(bm->status & BM_STATUS_DMAING)) {
===


Does it look okay?

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D