| Message ID | 1469633876-23209-1-git-send-email-ppandit@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 07/27/2016 09:37 AM, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > virtio back end uses set of buffers to facilitate I/O operations. > An infinite loop unfolds in virtqueue_pop() if a buffer was > of zero size. Add check to avoid it. > > Reported-by: Li Qiang <liqiang6-s@360.cn> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/virtio/virtio.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 30ede3d..8de896c 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -457,6 +457,11 @@ static void virtqueue_map_desc(unsigned int *p_num_sg, hwaddr *addr, struct iove > unsigned num_sg = *p_num_sg; > assert(num_sg <= max_num_sg); > > + if (!sz) { > + error_report("virtio: zero sized buffers are not allowed"); > + exit(1); > + } This lets the guest forcefully exit qemu. Isn't it better to just make the guest error degrade the virtio device into a broken state (the guest can no longer use it, but qemu doesn't exit)?
On Wed, Jul 27, 2016 at 10:13:04AM -0600, Eric Blake wrote: > On 07/27/2016 09:37 AM, P J P wrote: > > From: Prasad J Pandit <pjp@fedoraproject.org> > > > > virtio back end uses set of buffers to facilitate I/O operations. > > An infinite loop unfolds in virtqueue_pop() if a buffer was > > of zero size. Add check to avoid it. > > > > Reported-by: Li Qiang <liqiang6-s@360.cn> > > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > > --- > > hw/virtio/virtio.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > > index 30ede3d..8de896c 100644 > > --- a/hw/virtio/virtio.c > > +++ b/hw/virtio/virtio.c > > @@ -457,6 +457,11 @@ static void virtqueue_map_desc(unsigned int *p_num_sg, hwaddr *addr, struct iove > > unsigned num_sg = *p_num_sg; > > assert(num_sg <= max_num_sg); > > > > + if (!sz) { > > + error_report("virtio: zero sized buffers are not allowed"); > > + exit(1); > > + } > > This lets the guest forcefully exit qemu. Isn't it better to just make > the guest error degrade the virtio device into a broken state (the guest > can no longer use it, but qemu doesn't exit)? > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > ATM that's how other guest bugs are handled so I think it's fine. With a kernel driver it's easy for guest to forcefully exit qemu anyway. Hey just call halt(). We do want to improve this over time, and in particular supporting dpdk within guest will need that, but I don't think we should block this fix because of that.
On Wed, Jul 27, 2016 at 09:07:56PM +0530, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > virtio back end uses set of buffers to facilitate I/O operations. > An infinite loop unfolds in virtqueue_pop() if a buffer was > of zero size. Add check to avoid it. > > Reported-by: Li Qiang <liqiang6-s@360.cn> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/virtio/virtio.c | 5 +++++ > 1 file changed, 5 insertions(+) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 30ede3d..8de896c 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -457,6 +457,11 @@ static void virtqueue_map_desc(unsigned int *p_num_sg, hwaddr *addr, struct iove unsigned num_sg = *p_num_sg; assert(num_sg <= max_num_sg); + if (!sz) { + error_report("virtio: zero sized buffers are not allowed"); + exit(1); + } + while (sz) { hwaddr len = sz;