Virtualization - Mar 2011 - [PATCH] virtio: Decrement avail idx on buffer detach

When detaching a buffer from a vq, the avail.idx value should be
decremented as well.

This was noticed by hot-unplugging a virtio console port and then
plugging in a new one on the same number (re-using the vqs which were
just 'disowned').  qemu reported

   'Guest moved used index from 0 to 256'

when any IO was attempted on the new port.

CC: stable at kernel.org
Reported-by: juzhang <juzhang at redhat.com>
Signed-off-by: Amit Shah <amit.shah at redhat.com>
---
 drivers/virtio/virtio_ring.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cc2f73e..b0043fb 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -371,6 +371,7 @@ void *virtqueue_detach_unused_buf(struct virtqueue *_vq)
 		/* detach_buf clears data, so grab it now. */
 		buf = vq->data[i];
 		detach_buf(vq, i);
+		vq->vring.avail->idx--;
 		END_USE(vq);
 		return buf;
 	}
-- 
1.7.4

On Wed, 16 Mar 2011 19:12:10 +0530, Amit Shah <amit.shah at redhat.com>
wrote:
> When detaching a buffer from a vq, the avail.idx value should be
> decremented as well.
> 
> This was noticed by hot-unplugging a virtio console port and then
> plugging in a new one on the same number (re-using the vqs which were
> just 'disowned').  qemu reported
> 
>    'Guest moved used index from 0 to 256'
> 
> when any IO was attempted on the new port.
Yech... detach_unused_buf cannot be used on a live virtqueue; it assumes
we will reset the vq (usually by resetting the entire device).

You've partially violated that assumption by reusing the vq after
calling detach_unused_buf.  So I'm not entirely sure this is the only
bug lurking; safer would be to re-initialize the vq somehow when you
plug back in...

(Though this patch is minimal, and may be better -stable material).

Thanks,
Rusty.