[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:
https://git.xanclic.moe/XanClic/qemu/commits/branch/block
Max
signature.asc
Description: OpenPGP digital signature