P J P <ppan...@redhat.com> 于2020年9月4日周五 上午2:34写道: > > From: Prasad J Pandit <p...@fedoraproject.org> > > When cancelling an i/o operation via ide_cancel_dma_sync(), > a block pointer may be null. Add check to avoid null pointer > dereference. > > -> https://ruhr-uni-bochum.sciebo.de/s/NNWP2GfwzYKeKwE?path=%2Fide_nullptr1 > ==1803100==Hint: address points to the zero page. > #0 blk_bs ../block/block-backend.c:714 > #1 blk_drain ../block/block-backend.c:1715 > #2 ide_cancel_dma_sync ../hw/ide/core.c:723 > #3 bmdma_cmd_writeb ../hw/ide/pci.c:298 > #4 bmdma_write ../hw/ide/piix.c:75 > #5 memory_region_write_accessor ../softmmu/memory.c:483 > #6 access_with_adjusted_size ../softmmu/memory.c:544 > #7 memory_region_dispatch_write ../softmmu/memory.c:1465 > #8 flatview_write_continue ../exec.c:3176 > ... > > Reported-by: Ruhr-University <bugs-sys...@rub.de> > Signed-off-by: Prasad J Pandit <p...@fedoraproject.org> > --- > hw/ide/core.c | 1 + > hw/ide/pci.c | 5 ++++- > 2 files changed, 5 insertions(+), 1 deletion(-) > > Update v2: use an assert() call > -> https://lists.nongnu.org/archive/html/qemu-devel/2020-08/msg08336.html > > 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..b47e675456 100644 > --- a/hw/ide/pci.c > +++ b/hw/ide/pci.c > @@ -295,7 +295,10 @@ 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)); > + 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; > --
Hello Prasad, 'bmdma_cmd_writeb' is in the bmdma layer, I think it is better to not touch the IDE layer(check the 's->blk'). I think it is better to defer this check to 'ide_cancel_dma_sync'. 'ide_cancel_dma_sync' is also called by 'cmd_device_reset' and all of the handlers of 'ide_cmd_table' will check whether the 's->blk' is NULL in the beginning of 'ide_exec_cmd'. So I think it is reasonable to check 's->blk' at the begining of 'ide_cancel_dma_sync'. I'm not a blk expert, please correct me if I'm wrong. Thanks, Li Qiang > 2.26.2 > >