From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48274) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVcnC-0003kU-Nj for qemu-devel@nongnu.org; Wed, 11 Mar 2015 05:19:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVcn8-0001qF-0s for qemu-devel@nongnu.org; Wed, 11 Mar 2015 05:19:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39332) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVcn7-0001py-Od for qemu-devel@nongnu.org; Wed, 11 Mar 2015 05:19:01 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t2B9J1Bp013553 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 11 Mar 2015 05:19:01 -0400 Date: Wed, 11 Mar 2015 10:18:58 +0100 From: "Michael S. Tsirkin" Message-ID: <20150311101858-mutt-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Subject: [Qemu-devel] [PATCH] virtio-scsi: fix cdb/sense List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Paolo Bonzini , Fam Zheng 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 Signed-off-by: Michael S. Tsirkin --- 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(-) diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index de2c739..25d96f3 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -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), \ diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index cfb52e8..52bc00c 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -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 || -- MST