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
