virtio-scsi: fix cdb/sense
PATCH 1/1 virtio-scsi: fix cdb/sense
Commit "virtio-scsi: use standard-headers" added cdb and sense into req/rep structures, which breaks uses of sizeof for these structures, since qemu adds its own arrays on top.
To fix, replace sizeof with offsetof everywhere.
Reported-by Fam Zheng <[email protected]>
Signed-off-by Michael S. Tsirkin <[email protected]>
---
Note: this is very lightly tested.
Help with testing will be very much appreciated!
include/hw/virtio/virtio-scsi.h | 6 +++--- hw/scsi/virtio-scsi.c | 28 ++++++++++++++++++---------- 2 files changed, 21 insertions(+), 13 deletions(-)
include/hw/virtio/virtio-scsi.h +3 -3
@@ -136,15 +136,15 @@ typedef struct VirtIOSCSIReq {union {struct {VirtIOSCSICmdReq cmd; -uint8_t cdb[]; } QEMU_PACKED;VirtIOSCSICtrlTMFReq tmf;VirtIOSCSICtrlANReq an; } req; } VirtIOSCSIReq; -QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cdb) != - offsetof(VirtIOSCSIReq, req.cmd) + sizeof(VirtIOSCSICmdReq)); +QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cmd.cdb) != + offsetof(VirtIOSCSIReq, req.cmd) + + offsetof(VirtIOSCSICmdReq, cdb));#define DEFINE_VIRTIO_SCSI_PROPERTIES (_state, _conf_field) \ DEFINE_PROP_UINT32("num_queues", _state, _conf_field.num_queues, 1), \
hw/scsi/virtio-scsi.c +18 -10
@@ -47,7 +47,8 @@ VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)const size_t zero_skip = offsetof(VirtIOSCSIReq , elem) +sizeof (VirtQueueElement); - req =g_slice_alloc (sizeof (*req) + vs->cdb_size); + req =g_slice_alloc (sizeof (*req) +MAX (vs->cdb_size,VIRTIO_SCSI_CDB_SIZE ) - +VIRTIO_SCSI_CDB_SIZE ); req->vq = vq; req->dev = s;qemu_sglist_init (&req->qsgl,DEVICE (s),8 , &address_space_memory);
@@ -62,7 +63,8 @@ void virtio_scsi_free_req(VirtIOSCSIReq *req)qemu_iovec_destroy (&req->resp_iov);qemu_sglist_destroy (&req->qsgl); -g_slice_free1 (sizeof (*req) + vs->cdb_size, req); +g_slice_free1 (sizeof (*req) +MAX (vs->cdb_size,VIRTIO_SCSI_CDB_SIZE ) - +VIRTIO_SCSI_CDB_SIZE , req); }static void virtio_scsi_complete_req (VirtIOSCSIReq *req)
@@ -213,8 +215,10 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)assert (req->elem.in_num <=ARRAY_SIZE (req->elem.in_sg));assert (req->elem.out_num <=ARRAY_SIZE (req->elem.out_sg)); -if (virtio_scsi_parse_req (req,sizeof (VirtIOSCSICmdReq) + vs->cdb_size, -sizeof (VirtIOSCSICmdResp) + vs->sense_size) <0 ) { +if (virtio_scsi_parse_req (req, offsetof(VirtIOSCSICmdReq , cdb) + + vs->cdb_size, + offsetof(VirtIOSCSICmdResp , sense) + + vs->sense_size) <0 ) {error_report ("invalid SCSI request migration data" );exit (1 ); }
@@ -439,7 +443,7 @@ static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req)/* Sense data is not in req->resp and is copied separately * in virtio_scsi_command_complete. */ - req->resp_size =sizeof (VirtIOSCSICmdResp); + req->resp_size = offsetof(VirtIOSCSICmdResp , sense);virtio_scsi_complete_req (req); }
@@ -462,8 +466,10 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status, }else { req->resp.cmd.resid =0 ; sense_len =scsi_req_get_sense (r, sense,sizeof (sense)); - sense_len =MIN (sense_len, req->resp_iov.size -sizeof (req->resp.cmd)); -qemu_iovec_from_buf (&req->resp_iov,sizeof (req->resp.cmd), + sense_len = MIN(sense_len, req->resp_iov.size - + offsetof(typeof(req->resp.cmd), sense)); + qemu_iovec_from_buf(&req->resp_iov, + offsetof(typeof(req->resp.cmd), sense), sense, sense_len); req->resp.cmd.sense_len =virtio_tswap32 (vdev, sense_len); }
@@ -522,8 +528,10 @@ bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)SCSIDevice *d;int rc; - rc =virtio_scsi_parse_req (req,sizeof (VirtIOSCSICmdReq) + vs->cdb_size, -sizeof (VirtIOSCSICmdResp) + vs->sense_size); + rc =virtio_scsi_parse_req (req, offsetof(VirtIOSCSICmdReq , cdb) + + vs->cdb_size, + offsetof(VirtIOSCSICmdResp , sense) + + vs->sense_size);if (rc <0 ) {if (rc == -ENOTSUP ) {virtio_scsi_fail_cmd_req (req);
@@ -544,7 +552,7 @@ bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req) } req->sreq =scsi_req_new (d, req->req.cmd.tag,virtio_scsi_get_lun (req->req.cmd.lun), - req->req.cdb, req); + req->req.cmd.cdb, req); if (req->sreq->cmd.mode != SCSI_XFER_NONE && (req->sreq->cmd.mode != req->mode ||
Signature · 1 line
MST
Quoted 6 lines · Michael S. Tsirkin
Commit "virtio-scsi: use standard-headers" added cdb and sense into req/rep structures, which breaks uses of sizeof for these structures, since qemu adds its own arrays on top.
To fix, replace sizeof with offsetof everywhere.
This fixes breakage in SLOF on PPC64.
Quoted 117 lines
Reported-by Fam Zheng <[email protected]>Signed-off-by Michael S. Tsirkin <[email protected]>---
Note: this is very lightly tested.
Help with testing will be very much appreciated!include/hw/virtio/virtio-scsi.h | 6 +++--- hw/scsi/virtio-scsi.c | 28 ++++++++++++++++++---------- 2 files changed, 21 insertions(+), 13 deletions(-)include/hw/virtio/virtio-scsi.h +3
-3@@ -136,15 +136,15 @@ typedef struct VirtIOSCSIReq {union {struct {VirtIOSCSICmdReq cmd; -uint8_t cdb[]; } QEMU_PACKED;VirtIOSCSICtrlTMFReq tmf;VirtIOSCSICtrlANReq an; } req; } VirtIOSCSIReq; -QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cdb) != - offsetof(VirtIOSCSIReq, req.cmd) + sizeof(VirtIOSCSICmdReq)); +QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cmd.cdb) != + offsetof(VirtIOSCSIReq, req.cmd) + + offsetof(VirtIOSCSICmdReq, cdb));#define DEFINE_VIRTIO_SCSI_PROPERTIES (_state, _conf_field) \ DEFINE_PROP_UINT32("num_queues", _state, _conf_field.num_queues, 1), \hw/scsi/virtio-scsi.c +18
-10@@ -47,7 +47,8 @@ VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)const size_t zero_skip = offsetof(VirtIOSCSIReq , elem) +sizeof (VirtQueueElement); - req =g_slice_alloc (sizeof (*req) + vs->cdb_size); + req =g_slice_alloc (sizeof (*req) +MAX (vs->cdb_size,VIRTIO_SCSI_CDB_SIZE ) - +VIRTIO_SCSI_CDB_SIZE ); req->vq = vq; req->dev = s;qemu_sglist_init (&req->qsgl,DEVICE (s),8 , &address_space_memory);@@ -62,7 +63,8 @@ void virtio_scsi_free_req(VirtIOSCSIReq *req)qemu_iovec_destroy (&req->resp_iov);qemu_sglist_destroy (&req->qsgl); -g_slice_free1 (sizeof (*req) + vs->cdb_size, req); +g_slice_free1 (sizeof (*req) +MAX (vs->cdb_size,VIRTIO_SCSI_CDB_SIZE ) - +VIRTIO_SCSI_CDB_SIZE , req); }static void virtio_scsi_complete_req (VirtIOSCSIReq *req)@@ -213,8 +215,10 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)assert (req->elem.in_num <=ARRAY_SIZE (req->elem.in_sg));assert (req->elem.out_num <=ARRAY_SIZE (req->elem.out_sg)); -if (virtio_scsi_parse_req (req,sizeof (VirtIOSCSICmdReq) + vs->cdb_size, -sizeof (VirtIOSCSICmdResp) + vs->sense_size) <0 ) { +if (virtio_scsi_parse_req (req, offsetof(VirtIOSCSICmdReq , cdb) + + vs->cdb_size, + offsetof(VirtIOSCSICmdResp , sense) + + vs->sense_size) <0 ) {error_report ("invalid SCSI request migration data" );exit (1 ); }@@ -439,7 +443,7 @@ static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req)/* Sense data is not in req->resp and is copied separately * in virtio_scsi_command_complete. */ - req->resp_size =sizeof (VirtIOSCSICmdResp); + req->resp_size = offsetof(VirtIOSCSICmdResp , sense);virtio_scsi_complete_req (req); }@@ -462,8 +466,10 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status, }else { req->resp.cmd.resid =0 ; sense_len =scsi_req_get_sense (r, sense,sizeof (sense)); - sense_len =MIN (sense_len, req->resp_iov.size -sizeof (req->resp.cmd)); -qemu_iovec_from_buf (&req->resp_iov,sizeof (req->resp.cmd), + sense_len = MIN(sense_len, req->resp_iov.size - + offsetof(typeof(req->resp.cmd), sense)); + qemu_iovec_from_buf(&req->resp_iov, + offsetof(typeof(req->resp.cmd), sense), sense, sense_len); req->resp.cmd.sense_len =virtio_tswap32 (vdev, sense_len); }@@ -522,8 +528,10 @@ bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)SCSIDevice *d;int rc; - rc =virtio_scsi_parse_req (req,sizeof (VirtIOSCSICmdReq) + vs->cdb_size, -sizeof (VirtIOSCSICmdResp) + vs->sense_size); + rc =virtio_scsi_parse_req (req, offsetof(VirtIOSCSICmdReq , cdb) + + vs->cdb_size, + offsetof(VirtIOSCSICmdResp , sense) + + vs->sense_size);if (rc <0 ) {if (rc == -ENOTSUP ) {virtio_scsi_fail_cmd_req (req);@@ -544,7 +552,7 @@ bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req) } req->sreq =scsi_req_new (d, req->req.cmd.tag,virtio_scsi_get_lun (req->req.cmd.lun), - req->req.cdb, req); + req->req.cmd.cdb, req); if (req->sreq->cmd.mode != SCSI_XFER_NONE && (req->sreq->cmd.mode != req->mode ||
Signature · 1 line
Alexey