From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:46047)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1WhJnL-0006AG-Ox
	for qemu-devel@nongnu.org; Mon, 05 May 2014 10:23:10 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1WhJnF-0007eS-Gn
	for qemu-devel@nongnu.org; Mon, 05 May 2014 10:23:03 -0400
Received: from mx1.redhat.com ([209.132.183.28]:22069)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1WhJnF-0007eO-8D
	for qemu-devel@nongnu.org; Mon, 05 May 2014 10:22:57 -0400
From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <87y4ygnwoe.fsf@elfo.mitica> (Juan Quintela's message of "Mon, 05
	May 2014 16:10:25 +0200")
References: <1398690327-7838-1-git-send-email-mst@redhat.com>
	<1398690327-7838-5-git-send-email-mst@redhat.com>
	<87y4ygnwoe.fsf@elfo.mitica>
Date: Mon, 05 May 2014 16:22:53 +0200
Message-ID: <87tx94nw3m.fsf@elfo.mitica>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH v6 4/5] virtio: validate config_len on load
Reply-To: quintela@redhat.com
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: qemu-devel@nongnu.org, Anthony Liguori <aliguori@amazon.com>

Juan Quintela <quintela@redhat.com> wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> Malformed input can have config_len in migration stream
>> exceed the array size allocated on destination, the
>> result will be heap overflow.
>>
>> To fix, that config_len matches on both sides.
>>
>> CVE-2014-0182
>>
>> Reported-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>  hw/virtio/virtio.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 3bad71e..0d5d368 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -898,6 +898,7 @@ int virtio_set_features(VirtIODevice *vdev, uint32_t=
 val)
>>  int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>>  {
>>      int i, ret;
>> +    int32_t config_len;
>
> Has a warning.
>
> /mnt/kvm/qemu/next/hw/virtio/virtio.c: In function =E2=80=98virtio_load=
=E2=80=99:
> /mnt/kvm/qemu/next/hw/virtio/virtio.c:931:22: error: format =E2=80=98%i=
=E2=80=99
> expects argument of type =E2=80=98int=E2=80=99, but argument 2 has type =
=E2=80=98size_t=E2=80=99
> [-Werror=3Dformat=3D]
>                       config_len, vdev->config_len);
>                       ^
>
> changing config_len to size_t.

After discussing with michael, left it as int32_t


>
>>      uint32_t num;
>>      uint32_t features;
>>      uint32_t supported_features;
>> @@ -924,7 +925,12 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>>                       features, supported_features);
>>          return -1;
>>      }
>> -    vdev->config_len =3D qemu_get_be32(f);
>> +    config_len =3D qemu_get_be32(f);
>> +    if (config_len !=3D vdev->config_len) {
>> +        error_report("Unexpected config length 0x%x. Expected 0x%x",
>
> and this to:
>
> s/%x/%zx/

and use here "%ix   .... %zx"

Later, Juan.

> Later, Juan.
>
>
>> +                     config_len, vdev->config_len);
>> +        return -1;
>> +    }
>>      qemu_get_buffer(f, vdev->config, vdev->config_len);
>>=20=20
>>      num =3D qemu_get_be32(f);