Re: [PATCH 2/3] esp: restrict non-DMA transfer length to that of available data

[Thomas Huth <thuth@redhat.com>]

On 13/09/2023 22.44, Mark Cave-Ayland wrote:
> In the case where a SCSI layer transfer is incorrectly terminated, it is
> possible for a TI command to cause a SCSI buffer overflow due to the
> expected transfer data length being less than the available data in the
> FIFO. When this occurs the unsigned async_len variable underflows and
> becomes a large offset which writes past the end of the allocated SCSI
> buffer.
> 
> Restrict the non-DMA transfer length to be the smallest of the expected
> transfer length and the available FIFO data to ensure that it is no longer
> possible for the SCSI buffer overflow to occur.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1810
> ---
>   hw/scsi/esp.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 4218a6a960..9b11d8c573 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -759,7 +759,8 @@ static void esp_do_nodma(ESPState *s)
>       }
>   
>       if (to_device) {
> -        len = MIN(fifo8_num_used(&s->fifo), ESP_FIFO_SZ);
> +        len = MIN(s->async_len, ESP_FIFO_SZ);
> +        len = MIN(len, fifo8_num_used(&s->fifo));
>           esp_fifo_pop_buf(&s->fifo, s->async_buf, len);
>           s->async_buf += len;
>           s->async_len -= len;

Thanks for taking care of this!

Reviewed-by: Thomas Huth <thuth@redhat.com>