[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio-crypto: fix virtio_queue_set_notificatio
From: |
Gonglei (Arei) |
Subject: |
Re: [Qemu-devel] [PATCH] virtio-crypto: fix virtio_queue_set_notification() race |
Date: |
Thu, 17 Nov 2016 01:41:33 +0000 |
> -----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