Re: [PATCH] fdc: check drive block device before usage (CVE-2021-20196)

[Alexander Bulekov]

On 210123 1247, Alexander Bulekov wrote:
> Hello,
> Here is a reproducer for this bug found by the OSS-Fuzz fuzzer:
> 
> cat << EOF | qemu-system-i386 -display none -machine accel=qtest \
> -m 512M -machine pc -device floppy,unit=1,id=floppy0,drive=disk0 \
> -drive id=disk0,file=null-co://,file.read-zeroes=on,if=none,format=raw \
> -qtest stdio
> outw 0x3f4 0x2500
> outb 0x3f5 0x81
> outb 0x3f5 0x0
> outb 0x3f5 0x0
> outb 0x3f5 0x0
> outw 0x3f4 0x0
> outw 0x3f4 0x4000
> outw 0x3f4 0x13
> outb 0x3f5 0x1
> outw 0x3f2 0x1405
> outw 0x3f4 0x0
> EOF
> 
Oops there's a mistake there. Fixed:

cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest \
-m 512M -machine pc -device floppy,unit=1,id=floppy0,drive=disk0 \
-drive id=disk0,file=null-co://,file.read-zeroes=on,if=none,format=raw \
-qtest stdio
outw 0x3f4 0x2500
outb 0x3f5 0x81
outb 0x3f5 0x0
outb 0x3f5 0x0
outb 0x3f5 0x0
outw 0x3f2 0x14
outw 0x3f4 0x0
outw 0x3f4 0x4000
outw 0x3f4 0x13
outb 0x3f5 0x1
outw 0x3f2 0x1405
outw 0x3f4 0x0
EOF

> -Alex
> 
> On 210123 1533, P J P wrote:
> > From: Prasad J Pandit <pjp@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 <pgn@zju.edu.cn>
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1912780
> > Signed-off-by: Prasad J Pandit <pjp@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));
> > -- 
> > 2.29.2
> > 
> >