[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] block: always fill entire LUKS header space with zeros

From: Max Reitz
Subject: Re: [PATCH v2] block: always fill entire LUKS header space with zeros
Date: Fri, 7 Feb 2020 16:24:57 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1

On 07.02.20 14:55, Daniel P. Berrangé wrote:
> When initializing the LUKS header the size with default encryption
> parameters will currently be 2068480 bytes. This is rounded up to
> a multiple of the cluster size, 2081792, with 64k sectors. If the
> end of the header is not the same as the end of the cluster we fill
> the extra space with zeros. This was forgetting that not even the
> space allocated for the header will be fully initialized, as we
> only write key material for the first key slot. The space left
> for the other 7 slots is never written to.
> An optimization to the ref count checking code:
>   commit a5fff8d4b4d928311a5005efa12d0991fe3b66f9 (refs/bisect/bad)
>   Author: Vladimir Sementsov-Ogievskiy <address@hidden>
>   Date:   Wed Feb 27 16:14:30 2019 +0300
>     qcow2-refcount: avoid eating RAM
> made the assumption that every cluster which was allocated would
> have at least some data written to it. This was violated by way
> the LUKS header is only partially written, with much space simply
> reserved for future use.
> Depending on the cluster size this problem was masked by the
> logic which wrote zeros between the end of the LUKS header and
> the end of the cluster.
> $ qemu-img create --object secret,id=cluster_encrypt0,data=123456 \
>    -f qcow2 -o cluster_size=2k,encrypt.iter-time=1,\
>                encrypt.format=luks,encrypt.key-secret=cluster_encrypt0 \
>                cluster_size_check.qcow2 100M
>   Formatting 'cluster_size_check.qcow2', fmt=qcow2 size=104857600
>     encrypt.format=luks encrypt.key-secret=cluster_encrypt0
>     encrypt.iter-time=1 cluster_size=2048 lazy_refcounts=off refcount_bits=16
> $ qemu-img check --object secret,id=cluster_encrypt0,data=redhat \
>     'json:{"driver": "qcow2", "encrypt.format": "luks", \
>            "encrypt.key-secret": "cluster_encrypt0", \
>            "file.driver": "file", "file.filename": 
> "cluster_size_check.qcow2"}'
> ERROR: counting reference for region exceeding the end of the file by one 
> cluster or more: offset 0x2000 size 0x1f9000
> Leaked cluster 4 refcount=1 reference=0
> ...snip...
> Leaked cluster 130 refcount=1 reference=0
> 1 errors were found on the image.
> Data may be corrupted, or further writes to the image may corrupt it.
> 127 leaked clusters were found on the image.
> This means waste of disk space, but no harm to data.
> Image end offset: 268288
> The problem only exists when the disk image is entirely empty. Writing
> data to the disk image payload will solve the problem by causing the
> end of the file to be extended further.
> The change fixes it by ensuring that the entire allocated LUKS header
> region is fully initialized with zeros. The qemu-img check will still
> fail for any pre-existing disk images created prior to this change,
> unless at least 1 byte of the payload is written to.
> Fully writing zeros to the entire LUKS header is a good idea regardless
> as it ensures that space has been allocated on the host filesystem (or
> whatever block storage backend is used).
> Signed-off-by: Daniel P. Berrangé <address@hidden>
> ---
> In v2:
>  - Cut down test scenarios to speed up
>  - Use _check_test_img helper
>  - Fix outdated comments
>  - Use space not tabs

Using eight spaces for indentation is...  Interesting, but at least
consistent. :-)

>  block/qcow2.c              | 11 +++--
>  tests/qemu-iotests/284     | 97 ++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/284.out | 62 ++++++++++++++++++++++++
>  tests/qemu-iotests/group   |  1 +
>  4 files changed, 167 insertions(+), 4 deletions(-)
>  create mode 100755 tests/qemu-iotests/284
>  create mode 100644 tests/qemu-iotests/284.out
Thanks, applied to my block branch:



Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]