Re: [Qemu-devel] [PATCH v2 0/5] qcow2: Check L1/L2/reftable entries for alignment

[Stefan Hajnoczi]

On Fri, Sep 05, 2014 at 04:07:14PM +0200, Max Reitz wrote:
> The image fuzzer from Maria exposed a lot of assertions which might fail
> in qemu when fed with a broken qcow2 image. Some of them are related to
> qemu trusting the offsets given in the L1, L2 and refcount tables to
> always be properly aligned on cluster boundaries (e.g.
> https://bugs.launchpad.net/qemu/+bug/1354529).
> 
> This series fixes this by verifying (hopefully) all data read from L1,
> L2 and refcount tables accordingly; if the offsets are not aligned on
> cluster boundaries, an error message is emitted and the image is marked
> corrupt unless it has been opened read-only.
> 
> v2:
> - Added patch 1 which adds a new field to the BLOCK_IMAGE_CORRUPTED. I
>   know this might be incompatible to existing users of that field, so I
>   did an investigative search (I googled) but did not find any. So I'm
>   confident this won't break anything.
> - Patch 2:
>   - Suppress corruption events and messages after the first one;
>     however, a fatal corruption should still be signaled even after a
>     non-fatal one has already occured [Kevin]
>   - Also, use the new "fatal" field
> - Patch 3:
>   - Fix test 060 right in this patch (squashed the old patch 3 into
>     patch 2, the result is now this patch 3) [Kevin]
>   - Set the "fatal" field to true
> - Patch 4:
>   - Emit more debugging information (like table indices etc.) [Kevin]
>   - Generally set the "fatal" field to true
>   - In case a cluster with an unaligned offset should be freed, call the
>     corruption signalling function just like everywhere else; but set
>     "fatal" to false [Kevin+Eric]
> - Patch 5: Added [Kevin]
> 
> 
> git-backport-diff against v1 (although not very useful):
> 
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, 
> respectively
> 
> 001/5:[down] 'qapi/block: Add "fatal" to BLOCK_IMAGE_CORRUPTED'
> 002/5:[0045] [FC] 'qcow2: Add qcow2_signal_corruption()'
> 003/5:[0015] [FC] 'qcow2: Use qcow2_signal_corruption() for overlaps'
> 004/5:[0068] [FC] 'qcow2: Check L1/L2/reftable entries for alignment'
> 005/5:[down] 'iotests: Add more tests for qcow2 corruption'
> 
> 
> Max Reitz (5):
>   qapi/block: Add "fatal" to BLOCK_IMAGE_CORRUPTED
>   qcow2: Add qcow2_signal_corruption()
>   qcow2: Use qcow2_signal_corruption() for overlaps
>   qcow2: Check L1/L2/reftable entries for alignment
>   iotests: Add more tests for qcow2 corruption
> 
>  block/qcow2-cluster.c      | 43 ++++++++++++++++++++++++++---
>  block/qcow2-refcount.c     | 67 
> +++++++++++++++++++++++++++++++---------------
>  block/qcow2.c              | 48 +++++++++++++++++++++++++++++++++
>  block/qcow2.h              |  5 ++++
>  qapi/block-core.json       |  9 +++++--
>  tests/qemu-iotests/060     | 56 ++++++++++++++++++++++++++++++++++++--
>  tests/qemu-iotests/060.out | 61 +++++++++++++++++++++++++++++++++++++----
>  7 files changed, 255 insertions(+), 34 deletions(-)
> 
> -- 
> 2.1.0
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

pgp4KRyAf07qg.pgp
Description: PGP signature