[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio: move sanity checks to ifdef DEBUG
From: |
Cornelia Huck |
Subject: |
Re: [Qemu-devel] [PATCH] virtio: move sanity checks to ifdef DEBUG |
Date: |
Fri, 20 Mar 2015 09:12:09 +0100 |
On Wed, 18 Mar 2015 12:42:46 +0100
"Michael S. Tsirkin" <address@hidden> wrote:
> All that happens when virtqueue_fill is invoked incorrectly is that we
> corrupt guest memory, so this check is not a security measure.
> Move the check to ifdef DEBUG to make sure we don't introduce new
> crashes close to release.
I don't really disagree with this patch, but is anybody actually making
regular runs with VIRTIO_DEBUG activated?
> Array scans aren't free either, so it's a good idea from performance
> point of view, too.
>
> Cc: Rusty Russell <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
> hw/virtio/virtio.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 27429c2..37fb2ee 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -243,15 +243,22 @@ int virtio_queue_empty(VirtQueue *vq)
> void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
> unsigned int len_written, unsigned int idx)
> {
> - unsigned int offset, tot_wlen;
> + unsigned int offset;
> int i;
>
> trace_virtqueue_fill(vq, elem, len_written, idx);
>
> - for (tot_wlen = i = 0; i < elem->in_num; i++) {
> - tot_wlen += elem->in_sg[i].iov_len;
> +#ifdef DEBUG_VIRTIO
> + {
> + /* Check that len_written is <= the writable length. */
> + unsigned int tot_wlen;
> +
> + for (tot_wlen = i = 0; i < elem->in_num; i++) {
> + tot_wlen += elem->in_sg[i].iov_len;
> + }
> + assert(len_written <= tot_wlen);
> }
> - assert(len_written <= tot_wlen);
> +#endif
>
> offset = 0;
> for (i = 0; i < elem->in_num; i++) {