| Message ID | 20170717130130.29211-1-ppandit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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); + } } }