[Qemu-devel] [PATCH v1] ide: check BlockBackend object in ide_cancel_dma_sync

[Qemu-devel] [PATCH v1] ide: check BlockBackend object in ide_cancel_dma_sync

[P J P]

From: Prasad J Pandit <pjp@fedoraproject.org>

When cancelling pending DMA requests in ide_cancel_dma_sync,
the s->blk object could be null, leading to a null dereference.
Add check to avoid it.

  blk_bs (blk=0x0) at block/block-backend.c:389
  blk_drain (blk=0x0) at block/block-backend.c:1232
  ide_cancel_dma_sync (s=0x7f203241c1a8) at hw/ide/core.c:684
  bmdma_cmd_writeb (bm=0x7f203241cf20, val=104) at hw/ide/pci.c:237
  bmdma_write (opaque=0x7f203241cf20, addr=0, val=104, size=1) at hw/ide/piix.c:77

Reported-by: Chensongnian <chensongnian@huawei.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 hw/ide/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Update: add call stack trace leading the null dereference.

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 0b48b64..04474b3 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -681,8 +681,10 @@ void ide_cancel_dma_sync(IDEState *s)
 #ifdef DEBUG_IDE
         printf("%s: draining all remaining requests", __func__);
 #endif
-        blk_drain(s->blk);
-        assert(s->bus->dma->aiocb == NULL);
+        if (s->blk) {
+            blk_drain(s->blk);
+            assert(s->bus->dma->aiocb == NULL);
+        }
     }
 }
 
-- 
2.9.4

Re: [Qemu-devel] [Qemu-block] [PATCH v1] ide: check BlockBackend object in ide_cancel_dma_sync

[Kevin Wolf]

Am 17.07.2017 um 15:01 hat P J P geschrieben:
> From: Prasad J Pandit <pjp@fedoraproject.org>
> 
> When cancelling pending DMA requests in ide_cancel_dma_sync,
> the s->blk object could be null, leading to a null dereference.
> Add check to avoid it.
> 
>   blk_bs (blk=0x0) at block/block-backend.c:389
>   blk_drain (blk=0x0) at block/block-backend.c:1232
>   ide_cancel_dma_sync (s=0x7f203241c1a8) at hw/ide/core.c:684
>   bmdma_cmd_writeb (bm=0x7f203241cf20, val=104) at hw/ide/pci.c:237
>   bmdma_write (opaque=0x7f203241cf20, addr=0, val=104, size=1) at hw/ide/piix.c:77
> 
> Reported-by: Chensongnian <chensongnian@huawei.com>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/ide/core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Update: add call stack trace leading the null dereference.

I think Stefan didn't only mean a stack trace, but an actual instruction
how to reproduce this. VM configuration, what actions to take, etc.

In fact, I will add that we will probably want a qtest case as a
regression test anyway, and tests are always great for describing how to
reproduce a problem, too.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH v1] ide: check BlockBackend object in ide_cancel_dma_sync

[P J P]

  Hello Kevin,

+-- On Mon, 17 Jul 2017, Kevin Wolf wrote --+
| I think Stefan didn't only mean a stack trace, but an actual instruction
| how to reproduce this. VM configuration, what actions to take, etc.
| 
| In fact, I will add that we will probably want a qtest case as a
| regression test anyway, and tests are always great for describing how to
| reproduce a problem, too.

It was found via a fuzzing exercise; I've requested a reproducer from original 
reporter, will try to create a qtest once it's available.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F