qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 07/11] esp: don't underflow cmdfifo in do_cmd()


From: Mark Cave-Ayland
Subject: Re: [PATCH v3 07/11] esp: don't underflow cmdfifo in do_cmd()
Date: Thu, 1 Apr 2021 09:51:51 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.9.0

On 01/04/2021 09:19, Philippe Mathieu-Daudé wrote:

On 4/1/21 9:49 AM, Mark Cave-Ayland wrote:
If the guest tries to execute a CDB when cmdfifo is not empty before the start
of the message out phase then clearing the message out phase data will cause
cmdfifo to underflow due to cmdfifo_cdb_offset being larger than the amount of
data within.

Since this can only occur by issuing deliberately incorrect instruction
sequences, ensure that the maximum length of esp_fifo_pop_buf() is limited to
the size of the data within cmdfifo.

Buglink: https://bugs.launchpad.net/qemu/+bug/1909247
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
  hw/scsi/esp.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 4decbbfc29..7f49522e1d 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -319,13 +319,15 @@ static void do_busid_cmd(ESPState *s, uint8_t busid)
static void do_cmd(ESPState *s)
  {
-    uint8_t busid = fifo8_pop(&s->cmdfifo);
+    uint8_t busid = esp_fifo_pop(&s->cmdfifo);
+    int len;
s->cmdfifo_cdb_offset--; /* Ignore extended messages for now */
      if (s->cmdfifo_cdb_offset) {
-        esp_fifo_pop_buf(&s->cmdfifo, NULL, s->cmdfifo_cdb_offset);
+        len = MIN(s->cmdfifo_cdb_offset, fifo8_num_used(&s->cmdfifo));

Do we want to log(GUEST_ERRORS) this?

Not for this case, since a guest OS may legitimately try a SDTR negotiation using extended messages which the ESP implementation currently ignores.

+        esp_fifo_pop_buf(&s->cmdfifo, NULL, len);
          s->cmdfifo_cdb_offset = 0;
      }

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


ATB,

Mark.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]