Am 03.12.2013 20:24, schrieb Paolo Bonzini: > Il 03/12/2013 19:19, Peter Maydell ha scritto: >> On 3 December 2013 16:29, Michael S. Tsirkin <m...@redhat.com> wrote: >>> 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. >>> suggested patch: >>> >>> Signed-off-by: Michael S. Tsirkin <m...@redhat.com> >>> --- >>> hw/scsi/virtio-scsi.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c >>> index 26d95a1..51cc929 100644 >>> --- a/hw/scsi/virtio-scsi.c >>> +++ b/hw/scsi/virtio-scsi.c >>> @@ -147,6 +147,8 @@ static void *virtio_scsi_load_request(QEMUFile *f, >>> SCSIRequest *sreq) >>> qemu_get_be32s(f, &n); >>> assert(n < vs->conf.num_queues); >>> qemu_get_buffer(f, (unsigned char *)&req->elem, sizeof(req->elem)); >>> + assert(req->elem.in_num <= ARRAY_SIZE(req->elem.in_sg)); >>> + assert(req->elem.out_num <= ARRAY_SIZE(req->elem.out_sg)); >> >> Wouldn't it be better to fail migration, as other patches in >> this series do? "Silent security hole if you compile with >> -DNDEBUG" is a little bit unfriendly... > > The problem is that SCSIBusInfo's load_request cannot fail. I can look > at fixing it on top of this series.
Paolo, with this series not yet in, am I seeing correctly that no changes to allow failing this have been committed? Can you consider coordinating this with mst? Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg