[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4] scsi: esp: check length before dma read
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v4] scsi: esp: check length before dma read |
Date: |
Fri, 17 Jun 2016 10:21:23 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 |
On 17/06/2016 06:19, Amit Shah wrote:
> Hi,
>
> On (Wed) 15 Jun 2016 [23:06:19], P J P wrote:
>> From: Prasad J Pandit <address@hidden>
>>
>> While doing DMA read into ESP command buffer 's->cmdbuf', it could
>> write past the 's->cmdbuf' area, if it was partially filled;
>> ie. 's->cmdlen' wasn't set at the start of the buffer.
>> Check 'len' to avoid OOB access. Also increase the command buffer
>> size to 32, which is maximum when 's->do_cmd' is set.
>>
>> Reported-by: Li Qiang <address@hidden>
>> Signed-off-by: Prasad J Pandit <address@hidden>
>
> [...]
>
>> diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
>> index 6c79527..d2c4886 100644
>> --- a/include/hw/scsi/esp.h
>> +++ b/include/hw/scsi/esp.h
>> @@ -14,6 +14,7 @@ void esp_init(hwaddr espaddr, int it_shift,
>>
>> #define ESP_REGS 16
>> #define TI_BUFSZ 16
>> +#define ESP_CMDBUF_SZ 32
>>
>> typedef struct ESPState ESPState;
>>
>> @@ -31,7 +32,7 @@ struct ESPState {
>> SCSIBus bus;
>> SCSIDevice *current_dev;
>> SCSIRequest *current_req;
>> - uint8_t cmdbuf[TI_BUFSZ];
>> + uint8_t cmdbuf[ESP_CMDBUF_SZ];
>
> This was flagged as an incompatibility in the vmstate by a nightly run
> of the vmstate checker:
>
> Section "esp" Description "esp" Field "cmdbuf" size mismatch: 16 , 32
> Section "dc390" Description "esp" Field "cmdbuf" size mismatch: 16 , 32
> Section "am53c974" Description "esp" Field "cmdbuf" size mismatch: 16 , 32
Oh, good catch. But from reading the spec the implementation was
completely busted. Let's just drop handle_satn_stop, cmdbuf, cmdlen and
do_cmd.
Paolo