@samitouri / QOSamiQemu / commits / d149a11c6c

scsi-disk: protect against guest sending truncated data for MODE SELECT commands

scsi-disk has a MODE SELECT path where a truncated mode page can be allowed by a compatibility quirk, but the parser continues to use the page's declared length rather than the number of bytes actually remaining in the request buffer. This means that scsi_disk_check_mode_select() and scsi_disk_apply_mode_select() can read beyond the valid part of inbuf[], potentially up to the emulated age's length. Clamping page_len (the size of the page) to len (whatever the guest provided) ensures that scsi_disk_check_mode_select() and scsi_disk_apply_mode_select() do not access anything beyond bounds; however, this requires care to accept and handle truncated input in those two functions. In particular, until scsi_disk_check_mode_select()'s first call to mode_sense_page() the number of bytes to be cleared in mode_current[] is unknown, so zero it completely. And for everything else, be conservative and use len when providing inputs to other functions; but at the same time, ensure all accesses to inbuf[] are bound by expected_len. Note that pages longer than the emulated one are still rejected. Fixes: 389e18eb9aa4 ("scsi-disk: add SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED quirk for Macintosh") Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4051 Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo Bonzini committed Jul 21, 2026 at 17:22 UTC d149a11c6c825e182cfb5db7d1d1492a049e82ab
1 file changed +13 -8
hw/scsi/scsi-disk.c
+13 -8
@@ -1524,7 +1524,7 @@ static void scsi_disk_emulate_read_data(SCSIRequest *req)
1524 static int scsi_disk_check_mode_select(SCSIDiskState *s, int page,
1525 uint8_t *inbuf, int inlen)
1526 {
1527 - uint8_t mode_current[SCSI_MAX_MODE_LEN];
1527 + uint8_t mode_current[SCSI_MAX_MODE_LEN] = { 0 };
1528 uint8_t mode_changeable[SCSI_MAX_MODE_LEN];
1529 uint8_t *p;
1530 int len, expected_len, changeable_len, i;
@@ -1543,21 +1543,21 @@ static int scsi_disk_check_mode_select(SCSIDiskState *s, int page,
1543 }
1544
1545 p = mode_current;
1546 - memset(mode_current, 0, inlen + 2);
1546 len = mode_sense_page(s, page, &p, 0);
1548 - if (len < 0 || len != expected_len) {
1547 + /* The guest may send a truncated page, but not a longer one. */
1548 + if (len < 0 || expected_len > len) {
1549 return -1;
1550 }
1551
1552 p = mode_changeable;
1553 - memset(mode_changeable, 0, inlen + 2);
1553 + memset(mode_changeable, 0, len);
1554 changeable_len = mode_sense_page(s, page, &p, 1);
1555 assert(changeable_len == len);
1556
1557 /* Check that unchangeable bits are the same as what MODE SENSE
1558 * would return.
1559 */
1560 - for (i = 2; i < len; i++) {
1560 + for (i = 2; i < expected_len; i++) {
1561 if (((mode_current[i] ^ inbuf[i - 2]) & ~mode_changeable[i]) != 0) {
1562 return -1;
1563 }
@@ -1565,11 +1565,15 @@ static int scsi_disk_check_mode_select(SCSIDiskState *s, int page,
1565 return 0;
1566 }
1567
1568 -static void scsi_disk_apply_mode_select(SCSIDiskState *s, int page, uint8_t *p)
1568 +/* Note p may be truncated, so check any bytes you access against len. */
1569 +static void scsi_disk_apply_mode_select(SCSIDiskState *s, int page,
1570 + uint8_t *p, int len)
1571 {
1572 switch (page) {
1573 case MODE_PAGE_CACHING:
1572 - blk_set_enable_write_cache(s->qdev.conf.blk, (p[0] & 4) != 0);
1574 + if (len > 0) {
1575 + blk_set_enable_write_cache(s->qdev.conf.blk, (p[0] & 4) != 0);
1576 + }
1577 break;
1578
1579 default:
@@ -1612,6 +1616,7 @@ static int mode_select_pages(SCSIDiskReq *r, uint8_t *p, int len, bool change)
1616 goto invalid_param_len;
1617 }
1618 trace_scsi_disk_mode_select_page_truncated(page, page_len, len);
1619 + page_len = len;
1620 }
1621
1622 if (!change) {
@@ -1619,7 +1624,7 @@ static int mode_select_pages(SCSIDiskReq *r, uint8_t *p, int len, bool change)
1624 goto invalid_param;
1625 }
1626 } else {
1622 - scsi_disk_apply_mode_select(s, page, p);
1627 + scsi_disk_apply_mode_select(s, page, p, page_len);
1628 }
1629
1630 p += page_len;