From 0145b4c38110d36d4ce9f1659a93a6f0859a0ff7 Mon Sep 17 00:00:00 2001 From: Michael S. Tsirkin Date: Sun, 11 May 2014 16:21:54 -0500 Subject: [PATCH 13/20] virtio-scsi: fix buffer overrun on invalid state load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RH-Author: Michael S. Tsirkin Message-id: <1399825266-9366-1-git-send-email-mst@redhat.com> Patchwork-id: 58791 O-Subject: [PATCH qemu-kvm RHEL6.6] virtio-scsi: fix buffer overrun on invalid state load Bugzilla: 1096097 RH-Acked-by: Paolo Bonzini RH-Acked-by: Juan Quintela RH-Acked-by: Fam Zheng CVE-2013-4542 hw/scsi/scsi-bus.c invokes load_request. virtio_scsi_load_request does: qemu_get_buffer(f, (unsigned char *)&req->elem, sizeof(req->elem)); this probably can make elem invalid, for example, make in_num or out_num huge, then: virtio_scsi_parse_req(s, vs->cmd_vqs[n], req); will do: if (req->elem.out_num > 1) { qemu_sgl_init_external(req, &req->elem.out_sg[1], &req->elem.out_addr[1], req->elem.out_num - 1); } else { qemu_sgl_init_external(req, &req->elem.in_sg[1], &req->elem.in_addr[1], req->elem.in_num - 1); } and this will access out of array bounds. Note: this adds security checks within assert calls since SCSIBusInfo's load_request cannot fail. For now simply disable builds with NDEBUG - there seems to be little value in supporting these. Cc: Andreas Färber Signed-off-by: Michael S. Tsirkin Signed-off-by: Juan Quintela (cherry picked from commit 3c3ce981423e0d6c18af82ee62f1850c2cda5976) Bugzilla: 1096097 Tested: lightly on developer's box Brew build: http://brewweb.devel.redhat.com/brew/taskinfo?taskID=7440378 --- Note: same as 6.5.z backport hw/virtio-scsi.c | 9 +++++++++ 1 file changed, 9 insertions(+) Signed-off-by: Jeff E. Nelson --- hw/virtio-scsi.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c index cfb9764..b55c355 100644 --- a/hw/virtio-scsi.c +++ b/hw/virtio-scsi.c @@ -267,6 +267,15 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq) qemu_get_be32s(f, &n); assert(n == 0); qemu_get_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem)); + /* TODO: add a way for SCSIBusInfo's load_request to fail, + * and fail migration instead of asserting here. + * When we do, we might be able to re-enable NDEBUG below. + */ +#ifdef NDEBUG +#error building with NDEBUG is not supported +#endif + assert(req->elem.in_num <= ARRAY_SIZE(req->elem.in_sg)); + assert(req->elem.out_num <= ARRAY_SIZE(req->elem.out_sg)); virtio_scsi_parse_req(s, s->cmd_vq, req); scsi_req_ref(sreq); -- 1.7.1