Re: [PATCH v3 10/17] esp.c: don't assert() if FIFO empty when executing non-DMA SELATNS

[Philippe Mathieu-Daudé]

On 24/3/24 20:16, Mark Cave-Ayland wrote:
> The current logic assumes that at least 1 byte is present in the FIFO when
> executing a non-DMA SELATNS command, but this may not be the case if the
> guest executes an invalid ESP command sequence.

What is real hardware behavior here?

> Reported-by: Chuhong Yuan <hslester96@gmail.com>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   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 1aac8f5564..f3aa5364cf 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -762,7 +762,8 @@ static void esp_do_nodma(ESPState *s)
>   
>           case CMD_SELATNS:

Alternatively logging the guest abuse:

              len = fifo8_num_used(&s->fifo);
              if (len < 1) {
                  qemu_log_mask(LOG_GUEST_ERROR, ...
                  break;
              }

>               /* Copy one byte from FIFO into cmdfifo */
> -            len = esp_fifo_pop_buf(s, buf, 1);
> +            len = esp_fifo_pop_buf(s, buf,
> +                                   MIN(fifo8_num_used(&s->fifo), 1));
>               len = MIN(fifo8_num_free(&s->cmdfifo), len);
>               fifo8_push_all(&s->cmdfifo, buf, len);
>