Re: [Qemu-devel] [PATCH V2 2/3] virtio: destroy region cache during reset

[Jason Wang <jasowang@redhat.com>]

On 2017年03月13日 18:20, Paolo Bonzini wrote:
> On 13/03/2017 11:05, Cornelia Huck wrote:
>> On Mon, 13 Mar 2017 14:29:42 +0800
>> Jason Wang<jasowang@redhat.com>  wrote:
>>
>>> We don't destroy region cache during reset which can make the maps
>>> of previous driver leaked to a buggy or malicious driver that don't
>>> set vring address before starting to use the device. Fix this by
>>> destroy the region cache during reset and validate it before trying to
>>> see them.
>>>
>>> Cc: Cornelia Huck<cornelia.huck@de.ibm.com>
>>> Cc: Paolo Bonzini<pbonzini@redhat.com>
>>> Signed-off-by: Jason Wang<jasowang@redhat.com>
>>> ---
>>> Changes from v1:
>>> - switch to use rcu in virtio_virtqueue_region_cache()
>>> - use unlikely() when needed
>>> ---
>>>   hw/virtio/virtio.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-------
>>>   1 file changed, 53 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>>> index 76cc81b..f086452 100644
>>> --- a/hw/virtio/virtio.c
>>> +++ b/hw/virtio/virtio.c
>>> @@ -190,6 +190,10 @@ static inline uint16_t vring_avail_flags(VirtQueue *vq)
>>>   {
>>>       VRingMemoryRegionCaches *caches = atomic_rcu_read(&vq->vring.caches);
>>>       hwaddr pa = offsetof(VRingAvail, flags);
>>> +    if (unlikely(!caches)) {
>>> +        virtio_error(vq->vdev, "Cannot map avail flags");
>>> +        return 0;
>> I'm still not 100% convinced of those checks; but they don't do any
>> harm.
> Same here... We would be hiding a bug.
>

Yes, I will use assert here.

Thanks