* [Qemu-devel] [PATCH] virtio: check vring descriptor buffer length
@ 2016-07-27 15:37 P J P
2016-07-27 16:13 ` Eric Blake
2016-07-28 8:34 ` Stefan Hajnoczi
0 siblings, 2 replies; 4+ messages in thread
From: P J P @ 2016-07-27 15:37 UTC (permalink / raw)
To: Qemu Developers
Cc: Michael S. Tsirkin, Li Qiang, Stefan Hajnoczi, Prasad J Pandit
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);
+ }
+
while (sz) {
hwaddr len = sz;
--
2.5.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio: check vring descriptor buffer length
2016-07-27 15:37 [Qemu-devel] [PATCH] virtio: check vring descriptor buffer length P J P
@ 2016-07-27 16:13 ` Eric Blake
2016-07-27 18:11 ` Michael S. Tsirkin
2016-07-28 8:34 ` Stefan Hajnoczi
1 sibling, 1 reply; 4+ messages in thread
From: Eric Blake @ 2016-07-27 16:13 UTC (permalink / raw)
To: P J P, Qemu Developers
Cc: Li Qiang, Prasad J Pandit, Stefan Hajnoczi, Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio: check vring descriptor buffer length
2016-07-27 16:13 ` Eric Blake
@ 2016-07-27 18:11 ` Michael S. Tsirkin
0 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2016-07-27 18:11 UTC (permalink / raw)
To: Eric Blake
Cc: P J P, Qemu Developers, Li Qiang, Prasad J Pandit, Stefan Hajnoczi
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.
--
MST
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio: check vring descriptor buffer length
2016-07-27 15:37 [Qemu-devel] [PATCH] virtio: check vring descriptor buffer length P J P
2016-07-27 16:13 ` Eric Blake
@ 2016-07-28 8:34 ` Stefan Hajnoczi
1 sibling, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2016-07-28 8:34 UTC (permalink / raw)
To: P J P; +Cc: Qemu Developers, Michael S. Tsirkin, Li Qiang, Prasad J Pandit
[-- Attachment #1: Type: text/plain, Size: 518 bytes --]
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>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-07-28 8:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-27 15:37 [Qemu-devel] [PATCH] virtio: check vring descriptor buffer length P J P
2016-07-27 16:13 ` Eric Blake
2016-07-27 18:11 ` Michael S. Tsirkin
2016-07-28 8:34 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.