| Message ID | 20171124075542.16979-2-ppandit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 24 Nov 2017 13:25:41 +0530 P J P <ppandit@redhat.com> wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > An user could attempt to use an uninitialised VirtQueue object > or unset Vring.align leading to a arithmetic exception. Add check > to avoid it. > > Reported-by: Zhangboxian <zhangboxian@huawei.com> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/virtio/virtio.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > Updated: add vring.num check to virtio_queue_update_rings > -> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg04499.html > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 5884ce3480..0940f1daf3 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -182,7 +182,7 @@ void virtio_queue_update_rings(VirtIODevice *vdev, int n) > { > VRing *vring = &vdev->vq[n].vring; > > - if (!vring->desc) { > + if (!vdev->vq[n].vring.num || !vring->desc || !vring->align) { > /* not yet setup -> nothing to do */ The check now implies "not yet setup or non-existing queue", no? (Not that the check is wrong.) > return; > } > @@ -1414,6 +1414,9 @@ void virtio_config_modern_writel(VirtIODevice *vdev, > > void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr) > { > + if (!vdev->vq[n].vring.num) { > + return; > + } > vdev->vq[n].vring.desc = addr; > virtio_queue_update_rings(vdev, n); > } > @@ -1494,8 +1497,10 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align) > */ > assert(k->has_variable_vring_alignment); > > - vdev->vq[n].vring.align = align; > - virtio_queue_update_rings(vdev, n); > + if (align) { > + vdev->vq[n].vring.align = align; > + virtio_queue_update_rings(vdev, n); > + } > } > > static bool virtio_queue_notify_aio_vq(VirtQueue *vq) I think you also need to check for num in virtio_queue_set_rings().
On Fri, Nov 24, 2017 at 01:25:41PM +0530, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > An user could attempt to use an uninitialised VirtQueue object > or unset Vring.align leading to a arithmetic exception. Add check > to avoid it. > > Reported-by: Zhangboxian <zhangboxian@huawei.com> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/virtio/virtio.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > Updated: add vring.num check to virtio_queue_update_rings > -> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg04499.html Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 5884ce3480..0940f1daf3 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -182,7 +182,7 @@ void virtio_queue_update_rings(VirtIODevice *vdev, int n) { VRing *vring = &vdev->vq[n].vring; - if (!vring->desc) { + if (!vdev->vq[n].vring.num || !vring->desc || !vring->align) { /* not yet setup -> nothing to do */ return; } @@ -1414,6 +1414,9 @@ void virtio_config_modern_writel(VirtIODevice *vdev, void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr) { + if (!vdev->vq[n].vring.num) { + return; + } vdev->vq[n].vring.desc = addr; virtio_queue_update_rings(vdev, n); } @@ -1494,8 +1497,10 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align) */ assert(k->has_variable_vring_alignment); - vdev->vq[n].vring.align = align; - virtio_queue_update_rings(vdev, n); + if (align) { + vdev->vq[n].vring.align = align; + virtio_queue_update_rings(vdev, n); + } } static bool virtio_queue_notify_aio_vq(VirtQueue *vq)