[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 0/3] virtio: restore elem->in/out_sg after iov_discard_fro
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v2 0/3] virtio: restore elem->in/out_sg after iov_discard_front/back() |
Date: |
Tue, 22 Sep 2020 08:59:22 +0100 |
On Thu, Sep 17, 2020 at 10:44:52AM +0100, Stefan Hajnoczi wrote:
> v2:
> * Add missing undo in virtio-blk write zeroes error path [Li Qiang]
>
> Both virtio-blk and virtio-crypto use destructive iov_discard_front/back()
> operations on elem->in/out_sg. virtqueue_push() calls dma_memory_unmap() on
> the
> modified iovec arrays. The memory addresses may not match those originally
> mapped with dma_memory_map().
>
> This raises several issues:
> 1. MemoryRegion references can be leaked.
> 2. Dirty memory may not be tracked.
> 3. The non-RAM bounce buffer can be leaked.
>
> This patch series solves the issue in two ways:
> 1. virtio-blk uses a new iov_discard_undo() API to restore iovec arrays.
> 2. virtio-crypto uses g_memdup() to avoid modifying the original iovec arrays.
>
> The g_memdup() approach is slower than iov_discard_undo() but less
> complex/fragile. I am less familiar with the virtio-crypto code and it uses
> more complex sequences of iov_discard_front/back() calls than virtio-blk. If
> anyone feels like optimizing virtio-crypto, please go ahead.
>
> The virtio-blk bug was found by Alexander Bulekov's fuzzing effort. I found
> the
> virtio-crypto bug through code inspection.
>
> Stefan Hajnoczi (3):
> util/iov: add iov_discard_undo()
> virtio-blk: undo destructive iov_discard_*() operations
> virtio-crypto: don't modify elem->in/out_sg
>
> include/hw/virtio/virtio-blk.h | 2 +
> include/qemu/iov.h | 23 +++++
> hw/block/virtio-blk.c | 11 ++-
> hw/virtio/virtio-crypto.c | 17 +++-
> tests/test-iov.c | 165 +++++++++++++++++++++++++++++++++
> util/iov.c | 50 +++++++++-
> 6 files changed, 259 insertions(+), 9 deletions(-)
>
> --
> 2.26.2
>
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
signature.asc
Description: PGP signature