scsi: Reject commands if the CDB length exceeds buf_len

In scsi_req_parse_cdb(), if the CDB length implied by the command type
exceeds the initialized portion of the command buffer, reject the request.

Rejected requests are recorded by the `scsi_req_parse_bad` trace event.

On example of a bug detected by this check is SunOS's use of interleaved
DMA and non-DMA commands. This guest behavior currently causes QEMU to
parse uninitialized memory as a SCSI command, with unpredictable
outcomes.

With the new check in place:

  * QEMU consistently creates a trace event and rejects the request.

  * SunOS retries the request(s) and is able to successfully boot from
    disk.

Signed-off-by: John Millikin <john@john-millikin.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1127
Message-Id: <20220817053458.698416-2-john@john-millikin.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
John Millikin 2022-08-17 14:35:00 +09:00 committed by Paolo Bonzini
parent fe9d8927e2
commit 6d1511cea0

View file

@ -712,6 +712,11 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
SCSICommand cmd = { .len = 0 };
int ret;
if (buf_len == 0) {
trace_scsi_req_parse_bad(d->id, lun, tag, 0);
goto invalid_opcode;
}
if ((d->unit_attention.key == UNIT_ATTENTION ||
bus->unit_attention.key == UNIT_ATTENTION) &&
(buf[0] != INQUIRY &&
@ -741,6 +746,7 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
if (ret != 0) {
trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
invalid_opcode:
req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private);
} else {
assert(cmd.len != 0);
@ -1316,7 +1322,7 @@ int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
cmd->lba = -1;
len = scsi_cdb_length(buf);
if (len < 0) {
if (len < 0 || len > buf_len) {
return -1;
}