|
 |
fadd50c |
------------------------------------------------------------------------
|
|
|
fadd50c |
*From*: Paolo Bonzini
|
|
|
fadd50c |
*Subject*: Re: [Qemu-devel] [PATCH] scsi: check buffer length before
|
|
|
fadd50c |
reading scsi command
|
|
|
fadd50c |
*Date*: Wed, 1 Jun 2016 15:10:16 +0200
|
|
|
fadd50c |
*User-agent*: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
|
|
|
fadd50c |
Thunderbird/45.1.0
|
|
|
fadd50c |
|
|
|
fadd50c |
------------------------------------------------------------------------
|
|
|
fadd50c |
|
|
|
fadd50c |
|
|
|
fadd50c |
On 31/05/2016 19:53, P J P wrote:
|
|
|
fadd50c |
>/ From: Prasad J Pandit <address@hidden>/
|
|
|
fadd50c |
>/ /
|
|
|
fadd50c |
>/ The 53C9X Fast SCSI Controller(FSC) comes with an internal 16-byte/
|
|
|
fadd50c |
>/ FIFO buffer. It is used to handle command and data transfer./
|
|
|
fadd50c |
>/ Routine get_cmd() in non-DMA mode, uses 'ti_size' to read scsi/
|
|
|
fadd50c |
>/ command into a buffer. Add check to validate command length against/
|
|
|
fadd50c |
>/ buffer size to avoid any overrun./
|
|
|
fadd50c |
>/ /
|
|
|
fadd50c |
>/ Reported-by: Li Qiang <address@hidden>/
|
|
|
fadd50c |
>/ Signed-off-by: Prasad J Pandit <address@hidden>/
|
|
|
fadd50c |
>/ ---/
|
|
|
fadd50c |
>/ hw/scsi/esp.c | 3 +++/
|
|
|
fadd50c |
>/ 1 file changed, 3 insertions(+)/
|
|
|
fadd50c |
>/ /
|
|
|
fadd50c |
>/ diff --git a/tools/qemu-xen-traditional/hw/esp.c b/tools/qemu-xen-traditional/hw/esp.c/
|
|
|
fadd50c |
>/ index 60c1b28..953027a 100644/
|
|
|
fadd50c |
>/ --- a/tools/qemu-xen-traditional/hw/esp.c/
|
|
|
fadd50c |
>/ +++ b/tools/qemu-xen-traditional/hw/esp.c/
|
|
|
fadd50c |
>/ @@ -98,6 +98,9 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf, uint8_t /
|
|
|
fadd50c |
>/ buflen)/
|
|
|
fadd50c |
>/ s->dma_memory_read(s->dma_opaque, buf, dmalen);/
|
|
|
fadd50c |
>/ } else {/
|
|
|
fadd50c |
>/ dmalen = s->ti_size;/
|
|
|
fadd50c |
>/ + if (dmalen > TI_BUFSZ) {/
|
|
|
fadd50c |
>/ + return 0;/
|
|
|
fadd50c |
>/ + }/
|
|
|
fadd50c |
>/ memcpy(buf, s->ti_buf, dmalen);/
|
|
|
fadd50c |
>/ buf[0] = buf[2] >> 5;/
|
|
|
fadd50c |
>/ }/
|
|
|
fadd50c |
>/ /
|
|
|
fadd50c |
|
|
|
fadd50c |
In theory this shouldn't happen, but I agree that it is better to be
|
|
|
fadd50c |
defensive. I'm queuing this patch.
|
|
|
fadd50c |
|
|
|
fadd50c |
At least the following patch is needed to ensure that ti_size always
|
|
|
fadd50c |
matches ti_rptr/ti_wptr (Hervé, what do you think about it? should I
|
|
|
fadd50c |
resubmit it formally?). Also, things are more complicated than
|
|
|
fadd50c |
necessary due to ti_size being used for both DMA and FIFO transfers.
|
|
|
fadd50c |
|
|
|
fadd50c |
diff --git a/tools/qemu-xen-traditional/hw/esp.c b/tools/qemu-xen-traditional/hw/esp.c
|
|
|
fadd50c |
index c2f6f8f..6407844 100644
|
|
|
fadd50c |
--- a/tools/qemu-xen-traditional/hw/esp.c
|
|
|
fadd50c |
+++ b/tools/qemu-xen-traditional/hw/esp.c
|
|
|
fadd50c |
@@ -222,7 +222,7 @@ static void write_response(ESPState *s)
|
|
|
fadd50c |
} else {
|
|
|
fadd50c |
s->ti_size = 2;
|
|
|
fadd50c |
s->ti_rptr = 0;
|
|
|
fadd50c |
- s->ti_wptr = 0;
|
|
|
fadd50c |
+ s->ti_wptr = 2;
|
|
|
fadd50c |
s->rregs[ESP_RFLAGS] = 2;
|
|
|
fadd50c |
}
|
|
|
fadd50c |
esp_raise_irq(s);
|
|
|
fadd50c |
|