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

[Daniel P . Berrangé]

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>
---
 block/qcow2.c              |   4 +-
 tests/qemu-iotests/278     |  88 +++++++++++++++++
 tests/qemu-iotests/278.out | 197 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 4 files changed, 288 insertions(+), 2 deletions(-)
 create mode 100755 tests/qemu-iotests/278
 create mode 100644 tests/qemu-iotests/278.out

diff --git a/block/qcow2.c b/block/qcow2.c
index 7c18721741..dcfdd200fc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -140,8 +140,8 @@ static ssize_t qcow2_crypto_hdr_init_func(QCryptoBlock 
*block, size_t headerlen,
     clusterlen = size_to_clusters(s, headerlen) * s->cluster_size;
     assert(qcow2_pre_write_overlap_check(bs, 0, ret, clusterlen, false) == 0);
     ret = bdrv_pwrite_zeroes(bs->file,
-                             ret + headerlen,
-                             clusterlen - headerlen, 0);
+                             ret,
+                             clusterlen, 0);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not zero fill encryption header");
         return -1;
diff --git a/tests/qemu-iotests/278 b/tests/qemu-iotests/278
new file mode 100755
index 0000000000..c52f03dd52
--- /dev/null
+++ b/tests/qemu-iotests/278
@@ -0,0 +1,88 @@
+#!/usr/bin/env bash
+#
+# Test ref count checks on encrypted images
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=address@hidden
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1       # failure is the default!
+
+_cleanup()
+{
+       _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+
+size=1M
+
+SECRET="secret,id=sec0,data=astrochicken"
+
+IMGSPEC="driver=$IMGFMT,file.filename=$TEST_IMG,encrypt.key-secret=sec0"
+QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
+
+_run_test()
+{
+        echo
+       echo "== cluster size $csize"
+       echo "== checking image refcounts =="
+       $QEMU_IMG check --object $SECRET --image-opts $IMGSPEC | _filter_testdir
+
+       echo
+       echo "== writing some data =="
+       $QEMU_IO --object $SECRET -c "write -P 0x9 0 1" --image-opts $IMGSPEC | 
_filter_qemu_io | _filter_testdir
+       echo
+       echo "== rechecking image refcounts =="
+       $QEMU_IMG check --object $SECRET --image-opts $IMGSPEC | _filter_testdir
+
+       echo
+       echo "== writing some more data =="
+       $QEMU_IO --object $SECRET -c "write -P 0x9 $csize 1" --image-opts 
$IMGSPEC | _filter_qemu_io | _filter_testdir
+       echo
+       echo "== rechecking image refcounts =="
+       $QEMU_IMG check --object $SECRET --image-opts $IMGSPEC | _filter_testdir
+}
+
+
+echo
+echo "testing LUKS qcow2 encryption"
+echo
+
+for csize in 512 1024 2048 4096 8192 16384 32768 65536
+do
+  _make_test_img --object $SECRET -o 
"encrypt.format=luks,encrypt.key-secret=sec0,encrypt.iter-time=10,cluster_size=$csize"
 $size
+  _run_test "driver=$IMGFMT,encrypt.key-secret=sec0,file.filename=$TEST_IMG"
+  _cleanup_test_img
+done
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/278.out b/tests/qemu-iotests/278.out
new file mode 100644
index 0000000000..687260fe90
--- /dev/null
+++ b/tests/qemu-iotests/278.out
@@ -0,0 +1,197 @@
+QA output created by 278
+
+testing LUKS qcow2 encryption
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks 
encrypt.key-secret=sec0 encrypt.iter-time=10
+
+== cluster size 512
+== checking image refcounts ==
+No errors were found on the image.
+Image end offset: 2078208
+
+== writing some data ==
+wrote 1/1 bytes at offset 0
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+1/2048 = 0.05% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2079232
+
+== writing some more data ==
+wrote 1/1 bytes at offset 512
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+2/2048 = 0.10% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2079744
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks 
encrypt.key-secret=sec0 encrypt.iter-time=10
+
+== cluster size 1024
+== checking image refcounts ==
+No errors were found on the image.
+Image end offset: 2075648
+
+== writing some data ==
+wrote 1/1 bytes at offset 0
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+1/1024 = 0.10% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2077696
+
+== writing some more data ==
+wrote 1/1 bytes at offset 1024
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+2/1024 = 0.20% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2078720
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks 
encrypt.key-secret=sec0 encrypt.iter-time=10
+
+== cluster size 2048
+== checking image refcounts ==
+No errors were found on the image.
+Image end offset: 2076672
+
+== writing some data ==
+wrote 1/1 bytes at offset 0
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+1/512 = 0.20% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2080768
+
+== writing some more data ==
+wrote 1/1 bytes at offset 2048
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+2/512 = 0.39% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2082816
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks 
encrypt.key-secret=sec0 encrypt.iter-time=10
+
+== cluster size 4096
+== checking image refcounts ==
+No errors were found on the image.
+Image end offset: 2084864
+
+== writing some data ==
+wrote 1/1 bytes at offset 0
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+1/256 = 0.39% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2093056
+
+== writing some more data ==
+wrote 1/1 bytes at offset 4096
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+2/256 = 0.78% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2097152
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks 
encrypt.key-secret=sec0 encrypt.iter-time=10
+
+== cluster size 8192
+== checking image refcounts ==
+No errors were found on the image.
+Image end offset: 2105344
+
+== writing some data ==
+wrote 1/1 bytes at offset 0
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+1/128 = 0.78% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2121728
+
+== writing some more data ==
+wrote 1/1 bytes at offset 8192
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+2/128 = 1.56% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2129920
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks 
encrypt.key-secret=sec0 encrypt.iter-time=10
+
+== cluster size 16384
+== checking image refcounts ==
+No errors were found on the image.
+Image end offset: 2146304
+
+== writing some data ==
+wrote 1/1 bytes at offset 0
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+1/64 = 1.56% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2179072
+
+== writing some more data ==
+wrote 1/1 bytes at offset 16384
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+2/64 = 3.12% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2195456
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks 
encrypt.key-secret=sec0 encrypt.iter-time=10
+
+== cluster size 32768
+== checking image refcounts ==
+No errors were found on the image.
+Image end offset: 2228224
+
+== writing some data ==
+wrote 1/1 bytes at offset 0
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+1/32 = 3.12% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2293760
+
+== writing some more data ==
+wrote 1/1 bytes at offset 32768
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+2/32 = 6.25% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2326528
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks 
encrypt.key-secret=sec0 encrypt.iter-time=10
+
+== cluster size 65536
+== checking image refcounts ==
+No errors were found on the image.
+Image end offset: 2359296
+
+== writing some data ==
+wrote 1/1 bytes at offset 0
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+1/16 = 6.25% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2490368
+
+== writing some more data ==
+wrote 1/1 bytes at offset 65536
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rechecking image refcounts ==
+No errors were found on the image.
+2/16 = 12.50% allocated, 0.00% fragmented, 0.00% compressed clusters
+Image end offset: 2555904
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 6b10a6a762..3f3da6dd36 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -286,3 +286,4 @@
 272 rw
 273 backing quick
 277 rw quick
+278 rw
-- 
2.23.0