Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set

[P J P]

+-- On Mon, 27 Nov 2017, Cornelia Huck wrote --+
|The check for align is not really needed, as virtio-1 disallows setting align 
|anyway.

 disallows...?

| Checking for !desc is wrong (why shouldn't a driver be able to unset a
| descriptor table?)

+-- On Mon, 27 Nov 2017, Stefan Hajnoczi wrote --+
| > +    if (!vdev->vq[n].vring.num || !desc || !vdev->vq[n].vring.align) {
| ...
|   vdev->vq[n].vring.desc = desc;
|
| Why !desc?

virtio_queue_set_rings
 virtio_init_region_cache
   VirtQueue *vq = &vdev->vq[n];
   ...
   addr = vq->vring.desc;
   if (!addr) {
       return;
   }

These checks seem to be repeating all over. As mentioned earlier, could these 
be collated in one place, maybe virtio_queue_get_num()?

  int virtio_queue_get_num(VirtIODevice *vdev, int n)
  {
      VirtQueue *vq = &vdev->vq[n];

      if (!vq->.vring.num
           || !vq->vring.desc
           || !vq->vring.align) {
          return 0;  /* vq not set */
      }

      return vdev->vq[n].vring.num;
  }


Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F