scsi: Add buf_len parameter to scsi_req_new()
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. Signed-off-by: John Millikin <john@john-millikin.com> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1127 Message-Id: <20220817053458.698416-1-john@john-millikin.com> [Fill in correct length for adapters other than ESP. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
		
				
					committed by
					
						
						Paolo Bonzini
					
				
			
			
				
	
			
			
			
						parent
						
							c6e51f1bb2
						
					
				
				
					commit
					fe9d8927e2
				
			@@ -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);
 | 
			
		||||
 
 | 
			
		||||
@@ -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);
 | 
			
		||||
                                   s->dbc, s->current);
 | 
			
		||||
 | 
			
		||||
    n = scsi_req_enqueue(s->current->req);
 | 
			
		||||
    if (n) {
 | 
			
		||||
 
 | 
			
		||||
@@ -1062,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, 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");
 | 
			
		||||
@@ -1080,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, 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");
 | 
			
		||||
@@ -1268,7 +1268,7 @@ 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);
 | 
			
		||||
        cmd->req = scsi_req_new(sdev, cmd->index, lun, cdb, sizeof(cdb), cmd);
 | 
			
		||||
        if (!cmd->req) {
 | 
			
		||||
            trace_megasas_dcmd_req_alloc_failed(cmd->index,
 | 
			
		||||
                                                "LD get info vpd inquiry");
 | 
			
		||||
@@ -1748,7 +1748,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 +1823,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);
 | 
			
		||||
 
 | 
			
		||||
@@ -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_io->CDBLength, req);
 | 
			
		||||
 | 
			
		||||
    if (req->sreq->cmd.xfer > scsi_io->DataLength) {
 | 
			
		||||
        goto overrun;
 | 
			
		||||
 
 | 
			
		||||
@@ -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;
 | 
			
		||||
@@ -1713,7 +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);
 | 
			
		||||
        req = scsi_req_new(s, tag, lun, buf, 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);
 | 
			
		||||
 
 | 
			
		||||
@@ -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);
 | 
			
		||||
    }
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -791,9 +791,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)
 | 
			
		||||
 
 | 
			
		||||
@@ -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 = 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) {
 | 
			
		||||
@@ -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],
 | 
			
		||||
 
 | 
			
		||||
@@ -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;
 | 
			
		||||
 | 
			
		||||
@@ -696,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, req);
 | 
			
		||||
                             req->req.cmd.cdb, vs->cdb_size, req);
 | 
			
		||||
 | 
			
		||||
    if (req->sreq->cmd.mode != SCSI_XFER_NONE
 | 
			
		||||
        && (req->sreq->cmd.mode != req->mode ||
 | 
			
		||||
 
 | 
			
		||||
@@ -730,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, 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;
 | 
			
		||||
 
 | 
			
		||||
@@ -415,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, 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);
 | 
			
		||||
            }
 | 
			
		||||
 
 | 
			
		||||
@@ -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,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 = 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");
 | 
			
		||||
@@ -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);
 | 
			
		||||
    }
 | 
			
		||||
 
 | 
			
		||||
@@ -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);
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user