* [PATCH 1/2] scsi: Add buf_len parameter to scsi_req_new()
@ 2022-08-17 5:34 John Millikin
2022-08-17 5:35 ` [PATCH 2/2] scsi: Reject commands if the CDB length exceeds buf_len John Millikin
2022-08-19 16:06 ` [PATCH 1/2] scsi: Add buf_len parameter to scsi_req_new() Paolo Bonzini
0 siblings, 2 replies; 4+ messages in thread
From: John Millikin @ 2022-08-17 5:34 UTC (permalink / raw)
To: qemu-devel
Cc: John Millikin, Mark Cave-Ayland, Paolo Bonzini, Fam Zheng,
Bill Paul
When a SCSI command is received from the guest, the CDB length implied
by the first byte might exceed the number of bytes the guest sent. In
this case scsi_req_new() will read uninitialized data, causing
unpredictable behavior.
Adds the buf_len parameter to scsi_req_new() and plumbs it through the
call stack.
The sigil SCSI_CMD_BUF_LEN_TODO() is used to indicate that the buffer
length calculation is TODO it should be replaced by a better value,
such as the length of a successful DMA read.
Signed-off-by: John Millikin <john@john-millikin.com>
Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1127
---
hw/scsi/esp.c | 2 +-
hw/scsi/lsi53c895a.c | 2 +-
hw/scsi/megasas.c | 13 ++++++++-----
hw/scsi/mptsas.c | 3 ++-
hw/scsi/scsi-bus.c | 19 +++++++++++--------
hw/scsi/scsi-disk.c | 7 ++++---
hw/scsi/scsi-generic.c | 5 +++--
hw/scsi/spapr_vscsi.c | 3 ++-
hw/scsi/virtio-scsi.c | 6 ++++--
hw/scsi/vmw_pvscsi.c | 3 ++-
hw/usb/dev-storage.c | 3 ++-
hw/usb/dev-uas.c | 3 ++-
include/hw/scsi/scsi.h | 11 ++++++-----
include/scsi/utils.h | 6 ++++++
14 files changed, 54 insertions(+), 32 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 2d3c649567..19fafad2a3 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -292,7 +292,7 @@ static void do_command_phase(ESPState *s)
esp_fifo_pop_buf(&s->cmdfifo, buf, cmdlen);
current_lun = scsi_device_find(&s->bus, 0, s->current_dev->id, s->lun);
- s->current_req = scsi_req_new(current_lun, 0, s->lun, buf, s);
+ s->current_req = scsi_req_new(current_lun, 0, s->lun, buf, cmdlen, s);
datalen = scsi_req_enqueue(s->current_req);
s->ti_size = datalen;
fifo8_reset(&s->cmdfifo);
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index ad5f5e5f39..e8a4a705e7 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -864,7 +864,7 @@ static void lsi_do_command(LSIState *s)
s->current = g_new0(lsi_request, 1);
s->current->tag = s->select_tag;
s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun, buf,
- s->current);
+ SCSI_CMD_BUF_LEN_TODO(s->dbc), s->current);
n = scsi_req_enqueue(s->current->req);
if (n) {
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index d5dfb412ba..e887ae8adb 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1053,6 +1053,7 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
uint64_t pd_size;
uint16_t pd_id = ((sdev->id & 0xFF) << 8) | (lun & 0xFF);
uint8_t cmdbuf[6];
+ size_t cmdbuf_len = SCSI_CMD_BUF_LEN_TODO(sizeof(cmdbuf));
size_t len;
dma_addr_t residual;
@@ -1062,7 +1063,7 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
info->inquiry_data[0] = 0x7f; /* Force PQual 0x3, PType 0x1f */
info->vpd_page83[0] = 0x7f;
megasas_setup_inquiry(cmdbuf, 0, sizeof(info->inquiry_data));
- cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmd);
+ cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmdbuf_len, cmd);
if (!cmd->req) {
trace_megasas_dcmd_req_alloc_failed(cmd->index,
"PD get info std inquiry");
@@ -1080,7 +1081,7 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
return MFI_STAT_INVALID_STATUS;
} else if (info->inquiry_data[0] != 0x7f && info->vpd_page83[0] == 0x7f) {
megasas_setup_inquiry(cmdbuf, 0x83, sizeof(info->vpd_page83));
- cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmd);
+ cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmdbuf_len, cmd);
if (!cmd->req) {
trace_megasas_dcmd_req_alloc_failed(cmd->index,
"PD get info vpd inquiry");
@@ -1259,6 +1260,7 @@ static int megasas_ld_get_info_submit(SCSIDevice *sdev, int lun,
struct mfi_ld_info *info = cmd->iov_buf;
size_t dcmd_size = sizeof(struct mfi_ld_info);
uint8_t cdb[6];
+ size_t cdb_len;
ssize_t len;
dma_addr_t residual;
uint16_t sdev_id = ((sdev->id & 0xFF) << 8) | (lun & 0xFF);
@@ -1268,7 +1270,8 @@ static int megasas_ld_get_info_submit(SCSIDevice *sdev, int lun,
cmd->iov_buf = g_malloc0(dcmd_size);
info = cmd->iov_buf;
megasas_setup_inquiry(cdb, 0x83, sizeof(info->vpd_page83));
- cmd->req = scsi_req_new(sdev, cmd->index, lun, cdb, cmd);
+ cdb_len = SCSI_CMD_BUF_LEN_TODO(sizeof(cdb));
+ cmd->req = scsi_req_new(sdev, cmd->index, lun, cdb, cdb_len, cmd);
if (!cmd->req) {
trace_megasas_dcmd_req_alloc_failed(cmd->index,
"LD get info vpd inquiry");
@@ -1748,7 +1751,7 @@ static int megasas_handle_scsi(MegasasState *s, MegasasCmd *cmd,
return MFI_STAT_SCSI_DONE_WITH_ERROR;
}
- cmd->req = scsi_req_new(sdev, cmd->index, lun_id, cdb, cmd);
+ cmd->req = scsi_req_new(sdev, cmd->index, lun_id, cdb, cdb_len, cmd);
if (!cmd->req) {
trace_megasas_scsi_req_alloc_failed(
mfi_frame_desc(frame_cmd), target_id, lun_id);
@@ -1823,7 +1826,7 @@ static int megasas_handle_io(MegasasState *s, MegasasCmd *cmd, int frame_cmd)
megasas_encode_lba(cdb, lba_start, lba_count, is_write);
cmd->req = scsi_req_new(sdev, cmd->index,
- lun_id, cdb, cmd);
+ lun_id, cdb, cdb_len, cmd);
if (!cmd->req) {
trace_megasas_scsi_req_alloc_failed(
mfi_frame_desc(frame_cmd), target_id, lun_id);
diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
index 706cf0df3a..711613b5b1 100644
--- a/hw/scsi/mptsas.c
+++ b/hw/scsi/mptsas.c
@@ -324,7 +324,8 @@ static int mptsas_process_scsi_io_request(MPTSASState *s,
}
req->sreq = scsi_req_new(sdev, scsi_io->MsgContext,
- scsi_io->LUN[1], scsi_io->CDB, req);
+ scsi_io->LUN[1], scsi_io->CDB,
+ SCSI_CMD_BUF_LEN_TODO(SIZE_MAX), req);
if (req->sreq->cmd.xfer > scsi_io->DataLength) {
goto overrun;
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index b2e2bc3c96..288ea12969 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -102,15 +102,15 @@ static void scsi_device_unrealize(SCSIDevice *s)
}
int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
- void *hba_private)
+ size_t buf_len, void *hba_private)
{
SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
int rc;
assert(cmd->len == 0);
- rc = scsi_req_parse_cdb(dev, cmd, buf);
+ rc = scsi_req_parse_cdb(dev, cmd, buf, buf_len);
if (bus->info->parse_cdb) {
- rc = bus->info->parse_cdb(dev, cmd, buf, hba_private);
+ rc = bus->info->parse_cdb(dev, cmd, buf, buf_len, hba_private);
}
return rc;
}
@@ -703,7 +703,7 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d,
}
SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
- uint8_t *buf, void *hba_private)
+ uint8_t *buf, size_t buf_len, void *hba_private)
{
SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus);
const SCSIReqOps *ops;
@@ -734,9 +734,9 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
}
if (ops != NULL || !sc->parse_cdb) {
- ret = scsi_req_parse_cdb(d, &cmd, buf);
+ ret = scsi_req_parse_cdb(d, &cmd, buf, buf_len);
} else {
- ret = sc->parse_cdb(d, &cmd, buf, hba_private);
+ ret = sc->parse_cdb(d, &cmd, buf, buf_len, hba_private);
}
if (ret != 0) {
@@ -1308,7 +1308,8 @@ static void scsi_cmd_xfer_mode(SCSICommand *cmd)
}
}
-int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf)
+int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+ size_t buf_len)
{
int rc;
int len;
@@ -1706,6 +1707,7 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size,
while ((sbyte = qemu_get_sbyte(f)) > 0) {
uint8_t buf[SCSI_CMD_BUF_SIZE];
+ size_t buf_len;
uint32_t tag;
uint32_t lun;
SCSIRequest *req;
@@ -1713,7 +1715,8 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size,
qemu_get_buffer(f, buf, sizeof(buf));
qemu_get_be32s(f, &tag);
qemu_get_be32s(f, &lun);
- req = scsi_req_new(s, tag, lun, buf, NULL);
+ buf_len = SCSI_CMD_BUF_LEN_TODO(sizeof(buf));
+ req = scsi_req_new(s, tag, lun, buf, buf_len, NULL);
req->retry = (sbyte == 1);
if (bus->info->load_request) {
req->hba_private = bus->info->load_request(f, req);
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index efee6739f9..399e1787ea 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -3030,14 +3030,15 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
}
static int scsi_block_parse_cdb(SCSIDevice *d, SCSICommand *cmd,
- uint8_t *buf, void *hba_private)
+ uint8_t *buf, size_t buf_len,
+ void *hba_private)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
if (scsi_block_is_passthrough(s, buf)) {
- return scsi_bus_parse_cdb(&s->qdev, cmd, buf, hba_private);
+ return scsi_bus_parse_cdb(&s->qdev, cmd, buf, buf_len, hba_private);
} else {
- return scsi_req_parse_cdb(&s->qdev, cmd, buf);
+ return scsi_req_parse_cdb(&s->qdev, cmd, buf, buf_len);
}
}
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index ada24d7486..55db1684de 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -784,9 +784,10 @@ static Property scsi_generic_properties[] = {
};
static int scsi_generic_parse_cdb(SCSIDevice *dev, SCSICommand *cmd,
- uint8_t *buf, void *hba_private)
+ uint8_t *buf, size_t buf_len,
+ void *hba_private)
{
- return scsi_bus_parse_cdb(dev, cmd, buf, hba_private);
+ return scsi_bus_parse_cdb(dev, cmd, buf, buf_len, hba_private);
}
static void scsi_generic_class_initfn(ObjectClass *klass, void *data)
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index e320ccaa23..c17bb47f7b 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -783,6 +783,7 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
union srp_iu *srp = &req_iu(req)->srp;
SCSIDevice *sdev;
int n, lun;
+ size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
if ((srp->cmd.lun == 0 || be64_to_cpu(srp->cmd.lun) == SRP_REPORT_LUNS_WLUN)
&& srp->cmd.cdb[0] == REPORT_LUNS) {
@@ -801,7 +802,7 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
} return 1;
}
- req->sreq = scsi_req_new(sdev, req->qtag, lun, srp->cmd.cdb, req);
+ req->sreq = scsi_req_new(sdev, req->qtag, lun, srp->cmd.cdb, cdb_len, req);
n = scsi_req_enqueue(req->sreq);
trace_spapr_vscsi_queue_cmd(req->qtag, srp->cmd.cdb[0],
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 4141dddd51..632e3aa58f 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -622,7 +622,8 @@ static void virtio_scsi_command_complete(SCSIRequest *r, size_t resid)
}
static int virtio_scsi_parse_cdb(SCSIDevice *dev, SCSICommand *cmd,
- uint8_t *buf, void *hba_private)
+ uint8_t *buf, size_t buf_len,
+ void *hba_private)
{
VirtIOSCSIReq *req = hba_private;
@@ -672,6 +673,7 @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
VirtIOSCSICommon *vs = &s->parent_obj;
SCSIDevice *d;
int rc;
+ size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
sizeof(VirtIOSCSICmdResp) + vs->sense_size);
@@ -696,7 +698,7 @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
virtio_scsi_ctx_check(s, d);
req->sreq = scsi_req_new(d, req->req.cmd.tag,
virtio_scsi_get_lun(req->req.cmd.lun),
- req->req.cmd.cdb, req);
+ req->req.cmd.cdb, cdb_len, req);
if (req->sreq->cmd.mode != SCSI_XFER_NONE
&& (req->sreq->cmd.mode != req->mode ||
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 4d9969f3b1..f845c88378 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -716,6 +716,7 @@ pvscsi_process_request_descriptor(PVSCSIState *s,
SCSIDevice *d;
PVSCSIRequest *r = pvscsi_queue_pending_descriptor(s, &d, descr);
int64_t n;
+ size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
trace_pvscsi_process_req_descr(descr->cdb[0], descr->context);
@@ -730,7 +731,7 @@ pvscsi_process_request_descriptor(PVSCSIState *s,
r->sg.elemAddr = descr->dataAddr;
}
- r->sreq = scsi_req_new(d, descr->context, r->lun, descr->cdb, r);
+ r->sreq = scsi_req_new(d, descr->context, r->lun, descr->cdb, cdb_len, r);
if (r->sreq->cmd.mode == SCSI_XFER_FROM_DEV &&
(descr->flags & PVSCSI_FLAG_CMD_DIR_TODEVICE)) {
r->cmp.hostStatus = BTSTAT_BADMSG;
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index dca62d544f..353e129fac 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -379,6 +379,7 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket *p)
uint8_t devep = p->ep->nr;
SCSIDevice *scsi_dev;
uint32_t len;
+ size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
switch (p->pid) {
case USB_TOKEN_OUT:
@@ -415,7 +416,7 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket *p)
cbw.cmd_len, s->data_len);
assert(le32_to_cpu(s->csw.residue) == 0);
s->scsi_len = 0;
- s->req = scsi_req_new(scsi_dev, tag, cbw.lun, cbw.cmd, NULL);
+ s->req = scsi_req_new(scsi_dev, tag, cbw.lun, cbw.cmd, cdb_len, NULL);
if (s->commandlog) {
scsi_req_print(s->req);
}
diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index c9f295e7e4..768df8958c 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -699,6 +699,7 @@ static void usb_uas_command(UASDevice *uas, uas_iu *iu)
UASRequest *req;
uint32_t len;
uint16_t tag = be16_to_cpu(iu->hdr.tag);
+ size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
if (iu->command.add_cdb_length > 0) {
qemu_log_mask(LOG_UNIMP, "additional adb length not yet supported\n");
@@ -729,7 +730,7 @@ static void usb_uas_command(UASDevice *uas, uas_iu *iu)
req->req = scsi_req_new(req->dev, req->tag,
usb_uas_get_lun(req->lun),
- iu->command.cdb, req);
+ iu->command.cdb, cdb_len, req);
if (uas->requestlog) {
scsi_req_print(req->req);
}
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index e284e3a4ec..001103488c 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -59,7 +59,7 @@ struct SCSIDeviceClass {
void (*realize)(SCSIDevice *dev, Error **errp);
void (*unrealize)(SCSIDevice *dev);
int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
- void *hba_private);
+ size_t buf_len, void *hba_private);
SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
uint8_t *buf, void *hba_private);
void (*unit_attention_reported)(SCSIDevice *s);
@@ -122,7 +122,7 @@ struct SCSIBusInfo {
int tcq;
int max_channel, max_target, max_lun;
int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
- void *hba_private);
+ size_t buf_len, void *hba_private);
void (*transfer_data)(SCSIRequest *req, uint32_t arg);
void (*fail)(SCSIRequest *req);
void (*complete)(SCSIRequest *req, size_t residual);
@@ -192,14 +192,15 @@ void scsi_legacy_handle_cmdline(void);
SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d,
uint32_t tag, uint32_t lun, void *hba_private);
SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
- uint8_t *buf, void *hba_private);
+ uint8_t *buf, size_t buf_len, void *hba_private);
int32_t scsi_req_enqueue(SCSIRequest *req);
SCSIRequest *scsi_req_ref(SCSIRequest *req);
void scsi_req_unref(SCSIRequest *req);
int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
- void *hba_private);
-int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf);
+ size_t buf_len, void *hba_private);
+int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+ size_t buf_len);
void scsi_req_build_sense(SCSIRequest *req, SCSISense sense);
void scsi_req_print(SCSIRequest *req);
void scsi_req_continue(SCSIRequest *req);
diff --git a/include/scsi/utils.h b/include/scsi/utils.h
index d5c8efa16e..1a36d25ee4 100644
--- a/include/scsi/utils.h
+++ b/include/scsi/utils.h
@@ -144,4 +144,10 @@ int scsi_cdb_length(uint8_t *buf);
int scsi_sense_from_errno(int errno_value, SCSISense *sense);
int scsi_sense_from_host_status(uint8_t host_status, SCSISense *sense);
+/**
+ * Annotates places where a SCSI command buffer is passed to scsi_req_new()
+ * but haven't yet been updated to pass the buffer's true (populated) length.
+ */
+#define SCSI_CMD_BUF_LEN_TODO(guess) guess
+
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] scsi: Reject commands if the CDB length exceeds buf_len
2022-08-17 5:34 [PATCH 1/2] scsi: Add buf_len parameter to scsi_req_new() John Millikin
@ 2022-08-17 5:35 ` John Millikin
2022-08-19 16:06 ` [PATCH 1/2] scsi: Add buf_len parameter to scsi_req_new() Paolo Bonzini
1 sibling, 0 replies; 4+ messages in thread
From: John Millikin @ 2022-08-17 5:35 UTC (permalink / raw)
To: qemu-devel
Cc: John Millikin, Mark Cave-Ayland, Paolo Bonzini, Fam Zheng,
Bill Paul
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>
Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1127
---
hw/scsi/scsi-bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 288ea12969..1beb1b0cfc 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -712,6 +712,8 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
SCSICommand cmd = { .len = 0 };
int ret;
+ assert(buf_len > 0);
+
if ((d->unit_attention.key == UNIT_ATTENTION ||
bus->unit_attention.key == UNIT_ATTENTION) &&
(buf[0] != INQUIRY &&
@@ -1316,7 +1318,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;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/2] scsi: Add buf_len parameter to scsi_req_new()
2022-08-17 5:34 [PATCH 1/2] scsi: Add buf_len parameter to scsi_req_new() John Millikin
2022-08-17 5:35 ` [PATCH 2/2] scsi: Reject commands if the CDB length exceeds buf_len John Millikin
@ 2022-08-19 16:06 ` Paolo Bonzini
2022-08-21 10:46 ` John Millikin
1 sibling, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2022-08-19 16:06 UTC (permalink / raw)
To: John Millikin, qemu-devel; +Cc: Mark Cave-Ayland, Fam Zheng, Bill Paul
On 8/17/22 07:34, John Millikin wrote:
> The sigil SCSI_CMD_BUF_LEN_TODO() is used to indicate that the buffer
> length calculation is TODO it should be replaced by a better value,
> such as the length of a successful DMA read.
Let's just do it right:
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index e8a4a705e7..05a43ec807 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -864,7 +864,7 @@ static void lsi_do_command(LSIState *s)
s->current = g_new0(lsi_request, 1);
s->current->tag = s->select_tag;
s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun, buf,
- SCSI_CMD_BUF_LEN_TODO(s->dbc), s->current);
+ s->dbc, s->current);
n = scsi_req_enqueue(s->current->req);
if (n) {
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index e887ae8adb..842edc3f9d 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1053,7 +1053,6 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
uint64_t pd_size;
uint16_t pd_id = ((sdev->id & 0xFF) << 8) | (lun & 0xFF);
uint8_t cmdbuf[6];
- size_t cmdbuf_len = SCSI_CMD_BUF_LEN_TODO(sizeof(cmdbuf));
size_t len;
dma_addr_t residual;
@@ -1063,7 +1062,7 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
info->inquiry_data[0] = 0x7f; /* Force PQual 0x3, PType 0x1f */
info->vpd_page83[0] = 0x7f;
megasas_setup_inquiry(cmdbuf, 0, sizeof(info->inquiry_data));
- cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmdbuf_len, cmd);
+ cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, sizeof(cmdbuf), cmd);
if (!cmd->req) {
trace_megasas_dcmd_req_alloc_failed(cmd->index,
"PD get info std inquiry");
@@ -1081,7 +1080,7 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
return MFI_STAT_INVALID_STATUS;
} else if (info->inquiry_data[0] != 0x7f && info->vpd_page83[0] == 0x7f) {
megasas_setup_inquiry(cmdbuf, 0x83, sizeof(info->vpd_page83));
- cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmdbuf_len, cmd);
+ cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, sizeof(cmdbuf), cmd);
if (!cmd->req) {
trace_megasas_dcmd_req_alloc_failed(cmd->index,
"PD get info vpd inquiry");
diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
index 711613b5b1..a90c2546f1 100644
--- a/hw/scsi/mptsas.c
+++ b/hw/scsi/mptsas.c
@@ -324,8 +324,8 @@ static int mptsas_process_scsi_io_request(MPTSASState *s,
}
req->sreq = scsi_req_new(sdev, scsi_io->MsgContext,
- scsi_io->LUN[1], scsi_io->CDB,
- SCSI_CMD_BUF_LEN_TODO(SIZE_MAX), req);
+ scsi_io->LUN[1], scsi_io->CDB,
+ scsi_io->CDBLength, req);
if (req->sreq->cmd.xfer > scsi_io->DataLength) {
goto overrun;
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 288ea12969..cc71524024 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1707,7 +1707,6 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size,
while ((sbyte = qemu_get_sbyte(f)) > 0) {
uint8_t buf[SCSI_CMD_BUF_SIZE];
- size_t buf_len;
uint32_t tag;
uint32_t lun;
SCSIRequest *req;
@@ -1715,8 +1714,11 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size,
qemu_get_buffer(f, buf, sizeof(buf));
qemu_get_be32s(f, &tag);
qemu_get_be32s(f, &lun);
- buf_len = SCSI_CMD_BUF_LEN_TODO(sizeof(buf));
- req = scsi_req_new(s, tag, lun, buf, buf_len, NULL);
+ /*
+ * A too-short CDB would have been rejected by scsi_req_new, so just use
+ * SCSI_CMD_BUF_SIZE as the CDB length.
+ */
+ req = scsi_req_new(s, tag, lun, buf, sizeof(buf), NULL);
req->retry = (sbyte == 1);
if (bus->info->load_request) {
req->hba_private = bus->info->load_request(f, req);
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index c17bb47f7b..0a8cbf5a4b 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -783,7 +783,7 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
union srp_iu *srp = &req_iu(req)->srp;
SCSIDevice *sdev;
int n, lun;
- size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
+ size_t cdb_len = sizeof (srp->cmd.cdb) + (srp->cmd.add_cdb_len & ~3);
if ((srp->cmd.lun == 0 || be64_to_cpu(srp->cmd.lun) == SRP_REPORT_LUNS_WLUN)
&& srp->cmd.cdb[0] == REPORT_LUNS) {
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 632e3aa58f..41f2a56301 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -673,7 +673,6 @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
VirtIOSCSICommon *vs = &s->parent_obj;
SCSIDevice *d;
int rc;
- size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
sizeof(VirtIOSCSICmdResp) + vs->sense_size);
@@ -698,7 +697,7 @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
virtio_scsi_ctx_check(s, d);
req->sreq = scsi_req_new(d, req->req.cmd.tag,
virtio_scsi_get_lun(req->req.cmd.lun),
- req->req.cmd.cdb, cdb_len, req);
+ req->req.cmd.cdb, vs->cdb_size, req);
if (req->sreq->cmd.mode != SCSI_XFER_NONE
&& (req->sreq->cmd.mode != req->mode ||
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index f845c88378..91e2f858ab 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -716,7 +716,6 @@ pvscsi_process_request_descriptor(PVSCSIState *s,
SCSIDevice *d;
PVSCSIRequest *r = pvscsi_queue_pending_descriptor(s, &d, descr);
int64_t n;
- size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
trace_pvscsi_process_req_descr(descr->cdb[0], descr->context);
@@ -731,7 +730,7 @@ pvscsi_process_request_descriptor(PVSCSIState *s,
r->sg.elemAddr = descr->dataAddr;
}
- r->sreq = scsi_req_new(d, descr->context, r->lun, descr->cdb, cdb_len, r);
+ r->sreq = scsi_req_new(d, descr->context, r->lun, descr->cdb, descr->cdbLen, r);
if (r->sreq->cmd.mode == SCSI_XFER_FROM_DEV &&
(descr->flags & PVSCSI_FLAG_CMD_DIR_TODEVICE)) {
r->cmp.hostStatus = BTSTAT_BADMSG;
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 353e129fac..98639696e6 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -379,7 +379,6 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket *p)
uint8_t devep = p->ep->nr;
SCSIDevice *scsi_dev;
uint32_t len;
- size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
switch (p->pid) {
case USB_TOKEN_OUT:
@@ -416,7 +415,7 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket *p)
cbw.cmd_len, s->data_len);
assert(le32_to_cpu(s->csw.residue) == 0);
s->scsi_len = 0;
- s->req = scsi_req_new(scsi_dev, tag, cbw.lun, cbw.cmd, cdb_len, NULL);
+ s->req = scsi_req_new(scsi_dev, tag, cbw.lun, cbw.cmd, cbw.cmd_len, NULL);
if (s->commandlog) {
scsi_req_print(s->req);
}
diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 768df8958c..5192b062d6 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -71,7 +71,7 @@ typedef struct {
uint8_t reserved_2;
uint64_t lun;
uint8_t cdb[16];
- uint8_t add_cdb[1]; /* not supported by QEMU */
+ uint8_t add_cdb[1];
} QEMU_PACKED uas_iu_command;
typedef struct {
@@ -699,7 +699,7 @@ static void usb_uas_command(UASDevice *uas, uas_iu *iu)
UASRequest *req;
uint32_t len;
uint16_t tag = be16_to_cpu(iu->hdr.tag);
- size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
+ size_t cdb_len = sizeof(iu->command.cdb) + iu->command.add_cdb_length;
if (iu->command.add_cdb_length > 0) {
qemu_log_mask(LOG_UNIMP, "additional adb length not yet supported\n");
diff --git a/include/scsi/utils.h b/include/scsi/utils.h
index 1a36d25ee4..d5c8efa16e 100644
--- a/include/scsi/utils.h
+++ b/include/scsi/utils.h
@@ -144,10 +144,4 @@ int scsi_cdb_length(uint8_t *buf);
int scsi_sense_from_errno(int errno_value, SCSISense *sense);
int scsi_sense_from_host_status(uint8_t host_status, SCSISense *sense);
-/**
- * Annotates places where a SCSI command buffer is passed to scsi_req_new()
- * but haven't yet been updated to pass the buffer's true (populated) length.
- */
-#define SCSI_CMD_BUF_LEN_TODO(guess) guess
-
#endif
(or something like that).
paolo
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/2] scsi: Add buf_len parameter to scsi_req_new()
2022-08-19 16:06 ` [PATCH 1/2] scsi: Add buf_len parameter to scsi_req_new() Paolo Bonzini
@ 2022-08-21 10:46 ` John Millikin
0 siblings, 0 replies; 4+ messages in thread
From: John Millikin @ 2022-08-21 10:46 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: Mark Cave-Ayland, Fam Zheng, Bill Paul
Thank you for the suggestions for CDB sizes! Especially the tricky ones
in spapr_vscsi.c and dev-uas.c.
v2: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02997.html
On Fri, Aug 19, 2022 at 06:06:13PM +0200, Paolo Bonzini wrote:
> On 8/17/22 07:34, John Millikin wrote:
> > The sigil SCSI_CMD_BUF_LEN_TODO() is used to indicate that the buffer
> > length calculation is TODO it should be replaced by a better value,
> > such as the length of a successful DMA read.
>
> Let's just do it right:
>
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index e8a4a705e7..05a43ec807 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -864,7 +864,7 @@ static void lsi_do_command(LSIState *s)
> s->current = g_new0(lsi_request, 1);
> s->current->tag = s->select_tag;
> s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun, buf,
> - SCSI_CMD_BUF_LEN_TODO(s->dbc), s->current);
> + s->dbc, s->current);
> n = scsi_req_enqueue(s->current->req);
> if (n) {
> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> index e887ae8adb..842edc3f9d 100644
> --- a/hw/scsi/megasas.c
> +++ b/hw/scsi/megasas.c
> @@ -1053,7 +1053,6 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
> uint64_t pd_size;
> uint16_t pd_id = ((sdev->id & 0xFF) << 8) | (lun & 0xFF);
> uint8_t cmdbuf[6];
> - size_t cmdbuf_len = SCSI_CMD_BUF_LEN_TODO(sizeof(cmdbuf));
> size_t len;
> dma_addr_t residual;
> @@ -1063,7 +1062,7 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
> info->inquiry_data[0] = 0x7f; /* Force PQual 0x3, PType 0x1f */
> info->vpd_page83[0] = 0x7f;
> megasas_setup_inquiry(cmdbuf, 0, sizeof(info->inquiry_data));
> - cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmdbuf_len, cmd);
> + cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, sizeof(cmdbuf), cmd);
> if (!cmd->req) {
> trace_megasas_dcmd_req_alloc_failed(cmd->index,
> "PD get info std inquiry");
> @@ -1081,7 +1080,7 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
> return MFI_STAT_INVALID_STATUS;
> } else if (info->inquiry_data[0] != 0x7f && info->vpd_page83[0] == 0x7f) {
> megasas_setup_inquiry(cmdbuf, 0x83, sizeof(info->vpd_page83));
> - cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmdbuf_len, cmd);
> + cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, sizeof(cmdbuf), cmd);
> if (!cmd->req) {
> trace_megasas_dcmd_req_alloc_failed(cmd->index,
> "PD get info vpd inquiry");
> diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
> index 711613b5b1..a90c2546f1 100644
> --- a/hw/scsi/mptsas.c
> +++ b/hw/scsi/mptsas.c
> @@ -324,8 +324,8 @@ static int mptsas_process_scsi_io_request(MPTSASState *s,
> }
> req->sreq = scsi_req_new(sdev, scsi_io->MsgContext,
> - scsi_io->LUN[1], scsi_io->CDB,
> - SCSI_CMD_BUF_LEN_TODO(SIZE_MAX), req);
> + scsi_io->LUN[1], scsi_io->CDB,
> + scsi_io->CDBLength, req);
> if (req->sreq->cmd.xfer > scsi_io->DataLength) {
> goto overrun;
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index 288ea12969..cc71524024 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -1707,7 +1707,6 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size,
> while ((sbyte = qemu_get_sbyte(f)) > 0) {
> uint8_t buf[SCSI_CMD_BUF_SIZE];
> - size_t buf_len;
> uint32_t tag;
> uint32_t lun;
> SCSIRequest *req;
> @@ -1715,8 +1714,11 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size,
> qemu_get_buffer(f, buf, sizeof(buf));
> qemu_get_be32s(f, &tag);
> qemu_get_be32s(f, &lun);
> - buf_len = SCSI_CMD_BUF_LEN_TODO(sizeof(buf));
> - req = scsi_req_new(s, tag, lun, buf, buf_len, NULL);
> + /*
> + * A too-short CDB would have been rejected by scsi_req_new, so just use
> + * SCSI_CMD_BUF_SIZE as the CDB length.
> + */
> + req = scsi_req_new(s, tag, lun, buf, sizeof(buf), NULL);
> req->retry = (sbyte == 1);
> if (bus->info->load_request) {
> req->hba_private = bus->info->load_request(f, req);
> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
> index c17bb47f7b..0a8cbf5a4b 100644
> --- a/hw/scsi/spapr_vscsi.c
> +++ b/hw/scsi/spapr_vscsi.c
> @@ -783,7 +783,7 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
> union srp_iu *srp = &req_iu(req)->srp;
> SCSIDevice *sdev;
> int n, lun;
> - size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
> + size_t cdb_len = sizeof (srp->cmd.cdb) + (srp->cmd.add_cdb_len & ~3);
> if ((srp->cmd.lun == 0 || be64_to_cpu(srp->cmd.lun) == SRP_REPORT_LUNS_WLUN)
> && srp->cmd.cdb[0] == REPORT_LUNS) {
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 632e3aa58f..41f2a56301 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -673,7 +673,6 @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
> VirtIOSCSICommon *vs = &s->parent_obj;
> SCSIDevice *d;
> int rc;
> - size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
> rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
> sizeof(VirtIOSCSICmdResp) + vs->sense_size);
> @@ -698,7 +697,7 @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
> virtio_scsi_ctx_check(s, d);
> req->sreq = scsi_req_new(d, req->req.cmd.tag,
> virtio_scsi_get_lun(req->req.cmd.lun),
> - req->req.cmd.cdb, cdb_len, req);
> + req->req.cmd.cdb, vs->cdb_size, req);
> if (req->sreq->cmd.mode != SCSI_XFER_NONE
> && (req->sreq->cmd.mode != req->mode ||
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index f845c88378..91e2f858ab 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -716,7 +716,6 @@ pvscsi_process_request_descriptor(PVSCSIState *s,
> SCSIDevice *d;
> PVSCSIRequest *r = pvscsi_queue_pending_descriptor(s, &d, descr);
> int64_t n;
> - size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
> trace_pvscsi_process_req_descr(descr->cdb[0], descr->context);
> @@ -731,7 +730,7 @@ pvscsi_process_request_descriptor(PVSCSIState *s,
> r->sg.elemAddr = descr->dataAddr;
> }
> - r->sreq = scsi_req_new(d, descr->context, r->lun, descr->cdb, cdb_len, r);
> + r->sreq = scsi_req_new(d, descr->context, r->lun, descr->cdb, descr->cdbLen, r);
> if (r->sreq->cmd.mode == SCSI_XFER_FROM_DEV &&
> (descr->flags & PVSCSI_FLAG_CMD_DIR_TODEVICE)) {
> r->cmp.hostStatus = BTSTAT_BADMSG;
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index 353e129fac..98639696e6 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -379,7 +379,6 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket *p)
> uint8_t devep = p->ep->nr;
> SCSIDevice *scsi_dev;
> uint32_t len;
> - size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
> switch (p->pid) {
> case USB_TOKEN_OUT:
> @@ -416,7 +415,7 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket *p)
> cbw.cmd_len, s->data_len);
> assert(le32_to_cpu(s->csw.residue) == 0);
> s->scsi_len = 0;
> - s->req = scsi_req_new(scsi_dev, tag, cbw.lun, cbw.cmd, cdb_len, NULL);
> + s->req = scsi_req_new(scsi_dev, tag, cbw.lun, cbw.cmd, cbw.cmd_len, NULL);
> if (s->commandlog) {
> scsi_req_print(s->req);
> }
> diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
> index 768df8958c..5192b062d6 100644
> --- a/hw/usb/dev-uas.c
> +++ b/hw/usb/dev-uas.c
> @@ -71,7 +71,7 @@ typedef struct {
> uint8_t reserved_2;
> uint64_t lun;
> uint8_t cdb[16];
> - uint8_t add_cdb[1]; /* not supported by QEMU */
> + uint8_t add_cdb[1];
> } QEMU_PACKED uas_iu_command;
> typedef struct {
> @@ -699,7 +699,7 @@ static void usb_uas_command(UASDevice *uas, uas_iu *iu)
> UASRequest *req;
> uint32_t len;
> uint16_t tag = be16_to_cpu(iu->hdr.tag);
> - size_t cdb_len = SCSI_CMD_BUF_LEN_TODO(SIZE_MAX);
> + size_t cdb_len = sizeof(iu->command.cdb) + iu->command.add_cdb_length;
> if (iu->command.add_cdb_length > 0) {
> qemu_log_mask(LOG_UNIMP, "additional adb length not yet supported\n");
> diff --git a/include/scsi/utils.h b/include/scsi/utils.h
> index 1a36d25ee4..d5c8efa16e 100644
> --- a/include/scsi/utils.h
> +++ b/include/scsi/utils.h
> @@ -144,10 +144,4 @@ int scsi_cdb_length(uint8_t *buf);
> int scsi_sense_from_errno(int errno_value, SCSISense *sense);
> int scsi_sense_from_host_status(uint8_t host_status, SCSISense *sense);
> -/**
> - * Annotates places where a SCSI command buffer is passed to scsi_req_new()
> - * but haven't yet been updated to pass the buffer's true (populated) length.
> - */
> -#define SCSI_CMD_BUF_LEN_TODO(guess) guess
> -
> #endif
>
> (or something like that).
>
> paolo
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-21 10:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-17 5:34 [PATCH 1/2] scsi: Add buf_len parameter to scsi_req_new() John Millikin
2022-08-17 5:35 ` [PATCH 2/2] scsi: Reject commands if the CDB length exceeds buf_len John Millikin
2022-08-19 16:06 ` [PATCH 1/2] scsi: Add buf_len parameter to scsi_req_new() Paolo Bonzini
2022-08-21 10:46 ` John Millikin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.