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

[John Snow <jsnow@redhat.com>]

On 5/14/21 3:23 PM, Thomas Huth wrote:
> On 23/01/2021 11.03, 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));
>>
> 
> Ping again!
> 
> Could anybody review / pick this up?
> 
>   Thomas
> 

Yep. Not forgotten, despite appearances. Clearing my Python review 
backlog, then onto FDC/IDE.

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.