[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 4/5] qcow2: Don't allow overflow during clust
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v5 4/5] qcow2: Don't allow overflow during cluster allocation |
Date: |
Wed, 25 Apr 2018 13:26:45 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
On 04/25/2018 09:44 AM, Max Reitz wrote:
> On 2018-04-24 00:33, Eric Blake wrote:
>> Our code was already checking that we did not attempt to
>> allocate more clusters than what would fit in an INT64 (the
>> physical maximimum if we can access a full off_t's worth of
s/maximimum/maximum/
>> data). But this does not catch smaller limits enforced by
>> various spots in the qcow2 image description: L1 and normal
>> clusters of L2 are documented as having bits 63-56 reserved
>> for other purposes, capping our maximum offset at 64PB (bit
>> 55 is the maximum bit set). And for compressed images with
>> 2M clusters, the cap drops the maximum offset to bit 48, or
>> a maximum offset of 512TB. If we overflow that offset, we
>> would write compressed data into one place, but try to
>> decompress from another, which won't work.
>>
>> I don't have 512TB handy to prove whether things break if we
>> compress so much data that we overflow that limit, and don't
>> think that iotests can (quickly) test it either. Test 138
>> comes close (it corrupts an image into thinking something lives
>> at 32PB, which is half the maximum for L1 sizing - although
>> it relies on 512-byte clusters). But that test points out
>> that we will generally hit other limits first (such as running
>> out of memory for the refcount table, or exceeding file system
>> limits like 16TB on ext4, etc), so this is more a theoretical
>> safety valve than something likely to be hit.
>
> You don't need 512 TB, though, 36 MB is sufficient.
Cool. I'll have to attempt that as a followup patch.
>
> Here's what you do:
> (1) Create a 513 TB image with cluster_size=2M,refcount_bits=1
> (2) Take a hex editor and enter 16 refblocks into the reftable
> (3) Fill all of those refblocks with 1s
That's a lot of leaked clusters ;)
>
> (Funny side note: qemu-img check thinks that image is clean because it
> doesn't check refcounts beyond the image end...)
Eww - yet another bug to fix...
>
> I've attached a compressed test image (unsurprisingly, it compresses
> really well).
>
> Before this series:
> $ ./qemu-io -c 'write -c 0 2M' test.qcow2
> qcow2: Marking image as corrupt: Preventing invalid write on metadata
> (overlaps with refcount block); further corruption events will be suppressed
> write failed: Input/output error
>
> Aw.
>
> After this series:
> $ ./qemu-io -c 'write -c 0 2M' test.qcow2
> write failed: Input/output error
>
> (Normal writes just work fine.)
>
>
> Maybe you want to add a test still -- creating the image is rather quick
> (well, you have to write 64 MB of 1s, but other than that). The only
> thing that takes a bit of time is qemu figuring out where the first free
> cluster is... That takes like 15 seconds here.
Then the test doesn't belong in '-g quick'.
>
> And another issue of course is...
>
> $ ls -lhs test.qcow2
> 42M -rw-r--r--. 1 maxx maxx 513T 25. Apr 16:42 test.qcow2
>
> Yeah, that. Depends on the host file system, of course, whether that is
> a real issue. O:-)
As long as iotests can gracefully skip if qemu-img fails to create the
image, then the test should still run on all remaining filesystems that
support sparse files that large.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v5 0/5] minor qcow2 compression improvements, Eric Blake, 2018/04/23
- [Qemu-devel] [PATCH v5 2/5] qcow2: Document some maximum size constraints, Eric Blake, 2018/04/23
- [Qemu-devel] [PATCH v5 4/5] qcow2: Don't allow overflow during cluster allocation, Eric Blake, 2018/04/23
- [Qemu-devel] [PATCH v5 1/5] qcow2: Prefer byte-based calls into bs->file, Eric Blake, 2018/04/23
- [Qemu-devel] [PATCH v5 5/5] qcow2: Avoid memory over-allocation on compressed images, Eric Blake, 2018/04/23
- [Qemu-devel] [PATCH v5 3/5] qcow2: Reduce REFT_OFFSET_MASK, Eric Blake, 2018/04/23