On 5/14/21 9:26 PM, John Snow wrote: > On 5/14/21 3:23 PM, Thomas Huth wrote: >> On 23/01/2021 11.03, P J P wrote: >>> From: Prasad J Pandit <p...@fedoraproject.org> >>> >>> While processing ioport command in 'fdctrl_write_dor', device >>> controller may select a drive which is not initialised with a >>> block device. This may result in a NULL pointer dereference. >>> Add checks to avoid it. >>> >>> Fixes: CVE-2021-20196 >>> Reported-by: Gaoning Pan <p...@zju.edu.cn> >>> Buglink: https://bugs.launchpad.net/qemu/+bug/1912780 >>> Signed-off-by: Prasad J Pandit <p...@fedoraproject.org> >>> --- >>> hw/block/fdc.c | 11 +++++++++-- >>> 1 file changed, 9 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c >>> index 3636874432..13a9470d19 100644 >>> --- a/hw/block/fdc.c >>> +++ b/hw/block/fdc.c >>> @@ -1429,7 +1429,9 @@ static void fdctrl_write_dor(FDCtrl *fdctrl, >>> uint32_t value) >>> } >>> } >>> /* Selected drive */ >>> - fdctrl->cur_drv = value & FD_DOR_SELMASK; >>> + if (fdctrl->drives[value & FD_DOR_SELMASK].blk) { >>> + fdctrl->cur_drv = value & FD_DOR_SELMASK; >>> + } >>> fdctrl->dor = value; >>> } >>> @@ -1894,6 +1896,10 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl) >>> uint32_t pos; >>> cur_drv = get_cur_drv(fdctrl); >>> + if (!cur_drv->blk) { >>> + FLOPPY_DPRINTF("No drive connected\n"); >>> + return 0; >>> + } >>> fdctrl->dsr &= ~FD_DSR_PWRDOWN; >>> if (!(fdctrl->msr & FD_MSR_RQM) || !(fdctrl->msr & FD_MSR_DIO)) { >>> FLOPPY_DPRINTF("error: controller not ready for reading\n"); >>> @@ -2420,7 +2426,8 @@ static void fdctrl_write_data(FDCtrl *fdctrl, >>> uint32_t value) >>> if (pos == FD_SECTOR_LEN - 1 || >>> fdctrl->data_pos == fdctrl->data_len) { >>> cur_drv = get_cur_drv(fdctrl); >>> - if (blk_pwrite(cur_drv->blk, fd_offset(cur_drv), >>> fdctrl->fifo, >>> + if (cur_drv->blk == NULL >>> + || blk_pwrite(cur_drv->blk, fd_offset(cur_drv), >>> fdctrl->fifo, >>> BDRV_SECTOR_SIZE, 0) < 0) { >>> FLOPPY_DPRINTF("error writing sector %d\n", >>> fd_sector(cur_drv)); >>> >> >> Ping again! >> >> Could anybody review / pick this up?
This patch misses the qtest companion with the reproducer provided by Alexander. > Yep. Not forgotten, despite appearances. Clearing my Python review > backlog, then onto FDC/IDE. Yeah \o/ > > In the meantime, anything anyone else happens to feel comfortable > staging won't upset me any. I don't insist they go through my tree right > now.