[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v3 12/17] esp.c: prevent cmdfifo overflow in esp_cdb_ready()
From: |
Mark Cave-Ayland |
Subject: |
[PATCH v3 12/17] esp.c: prevent cmdfifo overflow in esp_cdb_ready() |
Date: |
Sun, 24 Mar 2024 19:17:01 +0000 |
During normal use the cmdfifo will never wrap internally and cmdfifo_cdb_offset
will always indicate the start of the SCSI CDB. However it is possible that a
malicious guest could issue an invalid ESP command sequence such that cmdfifo
wraps internally and cmdfifo_cdb_offset could point beyond the end of the FIFO
data buffer.
Add an extra check to fifo8_peek_buf() to ensure that if the cmdfifo has wrapped
internally then esp_cdb_ready() will exit rather than allow scsi_cdb_length() to
access data outside the cmdfifo data buffer.
Reported-by: Chuhong Yuan <hslester96@gmail.com>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/scsi/esp.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f47abc36d6..d8db33b921 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -429,13 +429,23 @@ static bool esp_cdb_ready(ESPState *s)
{
int len = fifo8_num_used(&s->cmdfifo) - s->cmdfifo_cdb_offset;
const uint8_t *pbuf;
+ uint32_t n;
int cdblen;
if (len <= 0) {
return false;
}
- pbuf = fifo8_peek_buf(&s->cmdfifo, len, NULL);
+ pbuf = fifo8_peek_buf(&s->cmdfifo, len, &n);
+ if (n < len) {
+ /*
+ * In normal use the cmdfifo should never wrap, but include this check
+ * to prevent a malicious guest from reading past the end of the
+ * cmdfifo data buffer below
+ */
+ return false;
+ }
+
cdblen = scsi_cdb_length((uint8_t *)&pbuf[s->cmdfifo_cdb_offset]);
return cdblen < 0 ? false : (len >= cdblen);
--
2.39.2
- Re: [PATCH v3 03/17] esp.c: replace esp_fifo_pop_buf() with esp_fifo8_pop_buf() in do_message_phase(), (continued)
- [PATCH v3 04/17] esp.c: replace cmdfifo use of esp_fifo_pop() in do_message_phase(), Mark Cave-Ayland, 2024/03/24
- [PATCH v3 05/17] esp.c: change esp_fifo_push() to take ESPState, Mark Cave-Ayland, 2024/03/24
- [PATCH v3 07/17] esp.c: use esp_fifo_push() instead of fifo8_push(), Mark Cave-Ayland, 2024/03/24
- [PATCH v3 06/17] esp.c: change esp_fifo_pop() to take ESPState, Mark Cave-Ayland, 2024/03/24
- [PATCH v3 08/17] esp.c: change esp_fifo_pop_buf() to take ESPState, Mark Cave-Ayland, 2024/03/24
- [PATCH v3 09/17] esp.c: introduce esp_fifo_push_buf() function for pushing to the FIFO, Mark Cave-Ayland, 2024/03/24
- [PATCH v3 10/17] esp.c: don't assert() if FIFO empty when executing non-DMA SELATNS, Mark Cave-Ayland, 2024/03/24
- [PATCH v3 11/17] esp.c: rework esp_cdb_length() into esp_cdb_ready(), Mark Cave-Ayland, 2024/03/24
- [PATCH v3 12/17] esp.c: prevent cmdfifo overflow in esp_cdb_ready(),
Mark Cave-Ayland <=
- [PATCH v3 13/17] esp.c: move esp_set_phase() and esp_get_phase() towards the beginning of the file, Mark Cave-Ayland, 2024/03/24
- [PATCH v3 14/17] esp.c: introduce esp_update_drq() and update esp_fifo_{push, pop}_buf() to use it, Mark Cave-Ayland, 2024/03/24
- [PATCH v3 15/17] esp.c: update esp_fifo_{push, pop}() to call esp_update_drq(), Mark Cave-Ayland, 2024/03/24
- [PATCH v3 16/17] esp.c: ensure esp_pdma_write() always calls esp_fifo_push(), Mark Cave-Ayland, 2024/03/24
- [PATCH v3 17/17] esp.c: remove explicit setting of DRQ within ESP state machine, Mark Cave-Ayland, 2024/03/24