[PATCH for-4.2 0/2] qcow2: Fix QCOW2_COMPRESSED_SECTOR_MASK

Max Reitz posted 2 patches 4 years, 5 months ago
Test FreeBSD passed
Test checkpatch passed
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191028161841.1198-1-mreitz@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
block/qcow2.h              |  2 +-
tests/qemu-iotests/272     | 79 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/272.out | 10 +++++
tests/qemu-iotests/group   |  1 +
4 files changed, 91 insertions(+), 1 deletion(-)
create mode 100755 tests/qemu-iotests/272
create mode 100644 tests/qemu-iotests/272.out
[PATCH for-4.2 0/2] qcow2: Fix QCOW2_COMPRESSED_SECTOR_MASK
Posted by Max Reitz 4 years, 5 months ago
This fixes a bug reported on
https://bugs.launchpad.net/qemu/+bug/1850000.  The problem is that
QCOW2_COMPRESSED_SECTOR_MASK is a 32-bit mask when it really needs to be
a 64-bit mask.

The launchpad report mentions only problems with qemu-img check on large
compressed images, but I think it might extend further than that:
- I suppose qcow2_free_any_clusters() would free every compressed offset
  modulo 4G, which isn’t good
- qcow2_update_snapshot_refcount() will probably update compressed
  cluster’s refcounts (in snapshots) modulo 4G, which also isn’t good
- And then we have check_refcount_l2() which updates the wrong clusters
  for qemu-img check (as demonstrated in the bug report)

- (qcow2_co_preadv_compressed() is safe because it uses the inverted
  mask, which of course is again just 32 bit)


But I haven’t tested those other cases.


Max Reitz (2):
  qcow2: Fix QCOW2_COMPRESSED_SECTOR_MASK
  iotests: Add test for 4G+ compressed qcow2 write

 block/qcow2.h              |  2 +-
 tests/qemu-iotests/272     | 79 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/272.out | 10 +++++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100755 tests/qemu-iotests/272
 create mode 100644 tests/qemu-iotests/272.out

-- 
2.21.0


Re: [PATCH for-4.2 0/2] qcow2: Fix QCOW2_COMPRESSED_SECTOR_MASK
Posted by Alberto Garcia 4 years, 5 months ago
On Mon 28 Oct 2019 05:18:39 PM CET, Max Reitz <mreitz@redhat.com> wrote:
> This fixes a bug reported on
> https://bugs.launchpad.net/qemu/+bug/1850000.  The problem is that
> QCOW2_COMPRESSED_SECTOR_MASK is a 32-bit mask when it really needs to be
> a 64-bit mask.

Ouch!

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [PATCH for-4.2 0/2] qcow2: Fix QCOW2_COMPRESSED_SECTOR_MASK
Posted by Max Reitz 4 years, 4 months ago
On 28.10.19 17:18, Max Reitz wrote:
> This fixes a bug reported on
> https://bugs.launchpad.net/qemu/+bug/1850000.  The problem is that
> QCOW2_COMPRESSED_SECTOR_MASK is a 32-bit mask when it really needs to be
> a 64-bit mask.
> 
> The launchpad report mentions only problems with qemu-img check on large
> compressed images, but I think it might extend further than that:
> - I suppose qcow2_free_any_clusters() would free every compressed offset
>   modulo 4G, which isn’t good
> - qcow2_update_snapshot_refcount() will probably update compressed
>   cluster’s refcounts (in snapshots) modulo 4G, which also isn’t good
> - And then we have check_refcount_l2() which updates the wrong clusters
>   for qemu-img check (as demonstrated in the bug report)
> 
> - (qcow2_co_preadv_compressed() is safe because it uses the inverted
>   mask, which of course is again just 32 bit)
> 
> 
> But I haven’t tested those other cases.
> 
> 
> Max Reitz (2):
>   qcow2: Fix QCOW2_COMPRESSED_SECTOR_MASK
>   iotests: Add test for 4G+ compressed qcow2 write
> 
>  block/qcow2.h              |  2 +-
>  tests/qemu-iotests/272     | 79 ++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/272.out | 10 +++++
>  tests/qemu-iotests/group   |  1 +
>  4 files changed, 91 insertions(+), 1 deletion(-)
>  create mode 100755 tests/qemu-iotests/272
>  create mode 100644 tests/qemu-iotests/272.out

Thanks for the review, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max