Re: [Qemu-devel] [PATCH] virtio-crypto: fix virtio_queue_set_notification() race

[Gonglei (Arei)]

> -----Original Message-----
> From: Stefan Hajnoczi [mailto:address@hidden
> Sent: Thursday, November 17, 2016 4:18 AM
> To: address@hidden
> Cc: Gonglei (Arei); Michael S. Tsirkin; Stefan Hajnoczi
> Subject: [PATCH] virtio-crypto: fix virtio_queue_set_notification() race
> 
> We must check for new virtqueue buffers after re-enabling notifications.
> This prevents the race condition where the guest added buffers just
> after we stopped popping the virtqueue but before we re-enabled
> notifications.
> 
> I think the virtio-crypto code was based on virtio-net but this crucial
> detail was missed.  virtio-net does not have the race condition because
> it processes the virtqueue one more time after re-enabling
> notifications.
> 
> Cc: Gonglei <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  hw/virtio/virtio-crypto.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
Reviewed-by: Gonglei <address@hidden>

Thanks,
-Gonglei

> diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
> index 3293843..847dc9d 100644
> --- a/hw/virtio/virtio-crypto.c
> +++ b/hw/virtio/virtio-crypto.c
> @@ -692,8 +692,17 @@ static void virtio_crypto_dataq_bh(void *opaque)
>          return;
>      }
> 
> -    virtio_crypto_handle_dataq(vdev, q->dataq);
> -    virtio_queue_set_notification(q->dataq, 1);
> +    for (;;) {
> +        virtio_crypto_handle_dataq(vdev, q->dataq);
> +        virtio_queue_set_notification(q->dataq, 1);
> +
> +        /* Are we done or did the guest add more buffers? */
> +        if (virtio_queue_empty(q->dataq)) {
> +            break;
> +        }
> +
> +        virtio_queue_set_notification(q->dataq, 0);
> +    }
>  }
> 
>  static void
> --
> 2.7.4