Re: [Qemu-devel] [PATCH] virtio: destroy region cache during reset

[Jason Wang <jasowang@redhat.com>]

On 2017年03月08日 17:10, Paolo Bonzini wrote:
>
> ----- Original Message -----
>> From: "Jason Wang" <jasowang@redhat.com>
>> To: "Paolo Bonzini" <pbonzini@redhat.com>, mst@redhat.com, qemu-devel@nongnu.org
>> Cc: peterx@redhat.com
>> Sent: Wednesday, March 8, 2017 7:22:06 AM
>> Subject: Re: [Qemu-devel] [PATCH] virtio: destroy region cache during reset
>>
>>
>>
>> On 2017年03月08日 11:21, Jason Wang wrote:
>>> On 2017年03月07日 18:55, Paolo Bonzini wrote:
>>>> On 07/03/2017 09:47, Jason Wang 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.
>>>> I'm still not sure as to how this can happen.  Reset does clear
>>>> desc/used/avail, which should then be checked before accessing the
>>>> caches.
>>> But the code does not check them in fact? (E.g the attached qtest
>>> patch can still pass check-qtest).
>>>
>>> Thanks
>> Ok, the reproducer seems wrong. And I think what you mean is something
>> like the check done in virtio_queue_ready(). But looks like not all
>> virtqueue check for this. One example is virtio_net_handle_ctrl(), and
>> there may be even more. So you want to fix them all?
> Why would virtio_net_handle_ctrl be called when desc == 0?  The checks
> are all in common virtio code.
>
> static void virtio_queue_notify_vq(VirtQueue *vq)
> {
>      if (vq->vring.desc && vq->handle_output) {
>          VirtIODevice *vdev = vq->vdev;
>
>          if (unlikely(vdev->broken)) {
>              return;
>          }
>
>          trace_virtio_queue_notify(vdev, vq - vdev->vq, vq);
>          vq->handle_output(vdev, vq);
>      }
> }
>                                                                                                                                                                                 1440,29       55%
> Paolo

Right, I miss this.

But I find two possible leaks by auditing the callers of virtqueue_pop():

virtio_input_send() and virtio_balloon_set_status() which will call 
virtio_balloon_receive_stats().

Thanks