Re: [Qemu-devel] [PATCH 1/3] virtio-pci: properly validate address before accessing config

[Andreas Färber]

Am 28.04.2013 10:35, schrieb Michael S. Tsirkin:
> On Sun, Apr 28, 2013 at 03:54:20PM +0800, Jason Wang wrote:
>> On 04/28/2013 03:26 AM, Michael S. Tsirkin wrote:
>>> On Fri, Apr 26, 2013 at 04:34:02PM +0800, Jason Wang wrote:
>>>> There are several several issues in the current checking:
>>>>
>>>> - The check was based on the minus of unsigned values which can overflow
>>>> - It was done after .{set|get}_config() which can lead crash when 
>>>> config_len is
>>>>   zero since vdev->config is NULL
>>>>
>>>> Fix this by:
>>>>
>>>> - Validate the address in virtio_pci_config_{read|write}() before
>>>>   .{set|get}_config
>>>> - Use addition instead minus to do the validation
>>>>
>>>> Cc: Michael S. Tsirkin <address@hidden>
>>>> Cc: Petr Matousek <address@hidden>
>>>> Signed-off-by: Jason Wang <address@hidden>
>>> Why do this in virtio-pci and not in virtio.c?
>>> If instead we correct the checks in virtio.c we
>>> get less code, and all transports will benefit
>>> automatically.
>>
>> I wish I could but looks like vitio_config_read{b|w|l} were only used by
>> virtio-pci. Other transport such as ccw and s390-virtio-bus have their
>> own implementation.
> 
> Okay but still, the bug is in checks in virtio.c, why not fix it there
> instead of making it assume caller does the checks?

Ping? This issue has been assigned a CVE but the solution does not seem
to be agreed on yet - are you working on a different proposal, Jason?

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg