From 36ec1a829a07cd9a926b2f0ddfa5079c8dc9dae5 Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Thu, 25 Sep 2025 13:28:45 +0100 Subject: [PATCH] esp.c: fix esp_cdb_ready() FIFO wraparound limit calculation MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The original calculation in commit 3cc70889a3 ("esp.c: prevent cmdfifo overflow in esp_cdb_ready()") subtracted cmdfifo_cdb_offset from fifo8_num_used() to calculate the outstanding cmdfifo length, but this is incorrect because fifo8_num_used() can also include wraparound data. Instead calculate the maximum offset used by scsi_cdb_length() which is just the first byte after cmdfifo_cdb_offset, and then peek the entire content of the cmdfifo. The fifo8_peek_bufptr() result will then return the maximum length of remaining data up to the end of the internal cmdfifo array, which can then be used for the overflow check. Signed-off-by: Mark Cave-Ayland Fixes: 3cc70889a3 ("esp.c: prevent cmdfifo overflow in esp_cdb_ready()") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3082 Reviewed-by: Philippe Mathieu-Daudé Link: https://lore.kernel.org/r/20250925122846.527615-2-mark.cave-ayland@ilande.co.uk Signed-off-by: Paolo Bonzini --- 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 1d264c40e5..2809fcdee0 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -447,7 +447,9 @@ static void write_response(ESPState *s) static bool esp_cdb_ready(ESPState *s) { - int len = fifo8_num_used(&s->cmdfifo) - s->cmdfifo_cdb_offset; + /* scsi_cdb_length() only reads the first byte */ + int limit = s->cmdfifo_cdb_offset + 1; + int len = fifo8_num_used(&s->cmdfifo); const uint8_t *pbuf; uint32_t n; int cdblen; @@ -457,7 +459,7 @@ static bool esp_cdb_ready(ESPState *s) } pbuf = fifo8_peek_bufptr(&s->cmdfifo, len, &n); - if (n < len) { + if (n < limit) { /* * In normal use the cmdfifo should never wrap, but include this check * to prevent a malicious guest from reading past the end of the -- 2.47.3