virtio-balloon: fix add/get API use

Since ee7cd8981e15bcb365fc762afe3fc47b8242f630 'virtio: expose added
descriptors immediately.', in virtio balloon virtqueue_get_buf might
now run concurrently with virtqueue_kick.  I audited both and this
seems safe in practice but this is not guaranteed by the API.
Additionally, a spurious interrupt might in theory make
virtqueue_get_buf run in parallel with virtqueue_add_buf, which is
racy.

While we might try to protect against spurious callbacks it's
easier to fix the driver: balloon seems to be the only one
(mis)using the API like this, so let's just fix balloon.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (removed unused var)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index bfbc15ca38ddd3a0ac57cf0860624558a35aee47..0908e604433303d605b9a7cd6ab4ffe36087f96f 100644 (file)
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -47,7 +47,7 @@ struct virtio_balloon
        struct task_struct *thread;
 
        /* Waiting for host to ack the pages we released. */
-       struct completion acked;
+       wait_queue_head_t acked;
 
        /* Number of balloon pages we've told the Host we're not using. */
        unsigned int num_pages;
@@ -89,29 +89,25 @@ static struct page *balloon_pfn_to_page(u32 pfn)
 
 static void balloon_ack(struct virtqueue *vq)
 {
-       struct virtio_balloon *vb;
-       unsigned int len;
+       struct virtio_balloon *vb = vq->vdev->priv;
 
-       vb = virtqueue_get_buf(vq, &len);
-       if (vb)
-               complete(&vb->acked);
+       wake_up(&vb->acked);
 }
 
 static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
 {
        struct scatterlist sg;
+       unsigned int len;
 
        sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
 
-       init_completion(&vb->acked);
-
        /* We should always be able to add one buffer to an empty queue. */
        if (virtqueue_add_buf(vq, &sg, 1, 0, vb, GFP_KERNEL) < 0)
                BUG();
        virtqueue_kick(vq);
 
        /* When host has read buffer, this completes via balloon_ack */
-       wait_for_completion(&vb->acked);
+       wait_event(vb->acked, virtqueue_get_buf(vq, &len));
 }
 
 static void set_page_pfns(u32 pfns[], struct page *page)
@@ -231,12 +227,8 @@ static void update_balloon_stats(struct virtio_balloon *vb)
  */
 static void stats_request(struct virtqueue *vq)
 {
-       struct virtio_balloon *vb;
-       unsigned int len;
+       struct virtio_balloon *vb = vq->vdev->priv;
 
-       vb = virtqueue_get_buf(vq, &len);
-       if (!vb)
-               return;
        vb->need_stats_update = 1;
        wake_up(&vb->config_change);
 }
@@ -245,11 +237,14 @@ static void stats_handle_request(struct virtio_balloon *vb)
 {
        struct virtqueue *vq;
        struct scatterlist sg;
+       unsigned int len;
 
        vb->need_stats_update = 0;
        update_balloon_stats(vb);
 
        vq = vb->stats_vq;
+       if (!virtqueue_get_buf(vq, &len))
+               return;
        sg_init_one(&sg, vb->stats, sizeof(vb->stats));
        if (virtqueue_add_buf(vq, &sg, 1, 0, vb, GFP_KERNEL) < 0)
                BUG();
@@ -358,6 +353,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
        INIT_LIST_HEAD(&vb->pages);
        vb->num_pages = 0;
        init_waitqueue_head(&vb->config_change);
+       init_waitqueue_head(&vb->acked);
        vb->vdev = vdev;
        vb->need_stats_update = 0;