From 54a53a006ed9c1fe027fd89045d6de1e9128d7f4 Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Sat, 30 Jul 2022 13:26:55 +0100 Subject: scsi-disk: fix overflow when block size is not a multiple of BDRV_SECTOR_SIZE In scsi_disk_emulate_write_same() the number of host sectors to transfer is calculated as (s->qdev.blocksize / BDRV_SECTOR_SIZE) which is then used to copy data in block size chunks to the iov buffer. Since the loop copying the data to the iov buffer uses a fixed increment of s->qdev.blocksize then using a block size that isn't a multiple of BDRV_SECTOR_SIZE introduces a rounding error in the iov buffer size calculation such that the iov buffer copy overflows the space allocated. Update the iov buffer copy for() loop so that it will use the smallest of either the current block size or the remaining transfer count to prevent the overflow. Signed-off-by: Mark Cave-Ayland Message-Id: <20220730122656.253448-2-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'hw') diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index f5cdb9ad4b..3027ac3b1e 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -1849,7 +1849,7 @@ static void scsi_disk_emulate_write_same(SCSIDiskReq *r, uint8_t *inbuf) uint32_t nb_sectors = scsi_data_cdb_xfer(r->req.cmd.buf); WriteSameCBData *data; uint8_t *buf; - int i; + int i, l; /* Fail if PBDATA=1 or LBDATA=1 or ANCHOR=1. */ if (nb_sectors == 0 || (req->cmd.buf[1] & 0x16)) { @@ -1891,8 +1891,9 @@ static void scsi_disk_emulate_write_same(SCSIDiskReq *r, uint8_t *inbuf) data->iov.iov_len); qemu_iovec_init_external(&data->qiov, &data->iov, 1); - for (i = 0; i < data->iov.iov_len; i += s->qdev.blocksize) { - memcpy(&buf[i], inbuf, s->qdev.blocksize); + for (i = 0; i < data->iov.iov_len; i += l) { + l = MIN(s->qdev.blocksize, data->iov.iov_len - i); + memcpy(&buf[i], inbuf, l); } scsi_req_ref(&r->req); -- cgit v1.2.3 From 55794c904df723109b228da28b5db778e0df3110 Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Sat, 30 Jul 2022 13:26:56 +0100 Subject: scsi-disk: ensure block size is non-zero and changes limited to bits 8-15 The existing code assumes that the block size can be generated from p[1] << 8 in multiple places which ignores the top and bottom 8 bits. If the block size is allowed to be set to an arbitrary value then this causes a mismatch between the value written by the guest in the block descriptor and the value subsequently read back using READ CAPACITY causing the guest to generate requests that can crash QEMU. For now restrict block size changes to bits 8-15 and also ignore requests to set the block size to 0 which causes the SCSI emulation to crash in at least one place with a divide by zero error. Fixes: 356c4c441e ("scsi-disk: allow MODE SELECT block descriptor to set the block size") Closes: https://gitlab.com/qemu-project/qemu/-/issues/1112 Signed-off-by: Mark Cave-Ayland Message-Id: <20220730122656.253448-3-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'hw') diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 3027ac3b1e..efee6739f9 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -1591,7 +1591,7 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf) int cmd = r->req.cmd.buf[0]; int len = r->req.cmd.xfer; int hdr_len = (cmd == MODE_SELECT ? 4 : 8); - int bd_len; + int bd_len, bs; int pass; if ((r->req.cmd.buf[1] & 0x11) != 0x10) { @@ -1617,9 +1617,19 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf) } /* Allow changing the block size */ - if (bd_len && p[6] != (s->qdev.blocksize >> 8)) { - s->qdev.blocksize = p[6] << 8; - trace_scsi_disk_mode_select_set_blocksize(s->qdev.blocksize); + if (bd_len) { + bs = p[5] << 16 | p[6] << 8 | p[7]; + + /* + * Since the existing code only checks/updates bits 8-15 of the block + * size, restrict ourselves to the same requirement for now to ensure + * that a block size set by a block descriptor and then read back by + * a subsequent SCSI command will be the same + */ + if (bs && !(bs & ~0xff00) && bs != s->qdev.blocksize) { + s->qdev.blocksize = bs; + trace_scsi_disk_mode_select_set_blocksize(s->qdev.blocksize); + } } len -= bd_len; -- cgit v1.2.3