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

[Jason Wang]

On 05/02/2013 10:35 PM, Andreas Färber wrote:
> 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
>

Hi, I was just back from vacation, will draft V2 soon according to
Michael's comments.

Thanks