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 <hsleste...@gmail.com>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@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);