yeahuh / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone

Blame SOURCES/kvm-block-always-fill-entire-LUKS-header-space-with-zero.patch

902636
From 67f36d057aa71ca56ebc17ef28a7cb70bac6c6b6 Mon Sep 17 00:00:00 2001
902636
From: "Daniel P. Berrange" <berrange@redhat.com>
902636
Date: Tue, 5 May 2020 16:46:01 +0100
902636
Subject: [PATCH 01/12] block: always fill entire LUKS header space with zeros
902636
MIME-Version: 1.0
902636
Content-Type: text/plain; charset=UTF-8
902636
Content-Transfer-Encoding: 8bit
902636
902636
RH-Author: Daniel P. Berrange <berrange@redhat.com>
902636
Message-id: <20200505164601.1059974-2-berrange@redhat.com>
902636
Patchwork-id: 96277
902636
O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH 1/1] block: always fill entire LUKS header space with zeros
902636
Bugzilla: 1775462
902636
RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
902636
RH-Acked-by: John Snow <jsnow@redhat.com>
902636
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
902636
902636
When initializing the LUKS header the size with default encryption
902636
parameters will currently be 2068480 bytes. This is rounded up to
902636
a multiple of the cluster size, 2081792, with 64k sectors. If the
902636
end of the header is not the same as the end of the cluster we fill
902636
the extra space with zeros. This was forgetting that not even the
902636
space allocated for the header will be fully initialized, as we
902636
only write key material for the first key slot. The space left
902636
for the other 7 slots is never written to.
902636
902636
An optimization to the ref count checking code:
902636
902636
  commit a5fff8d4b4d928311a5005efa12d0991fe3b66f9 (refs/bisect/bad)
902636
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
902636
  Date:   Wed Feb 27 16:14:30 2019 +0300
902636
902636
    qcow2-refcount: avoid eating RAM
902636
902636
made the assumption that every cluster which was allocated would
902636
have at least some data written to it. This was violated by way
902636
the LUKS header is only partially written, with much space simply
902636
reserved for future use.
902636
902636
Depending on the cluster size this problem was masked by the
902636
logic which wrote zeros between the end of the LUKS header and
902636
the end of the cluster.
902636
902636
$ qemu-img create --object secret,id=cluster_encrypt0,data=123456 \
902636
   -f qcow2 -o cluster_size=2k,encrypt.iter-time=1,\
902636
               encrypt.format=luks,encrypt.key-secret=cluster_encrypt0 \
902636
               cluster_size_check.qcow2 100M
902636
  Formatting 'cluster_size_check.qcow2', fmt=qcow2 size=104857600
902636
    encrypt.format=luks encrypt.key-secret=cluster_encrypt0
902636
    encrypt.iter-time=1 cluster_size=2048 lazy_refcounts=off refcount_bits=16
902636
902636
$ qemu-img check --object secret,id=cluster_encrypt0,data=redhat \
902636
    'json:{"driver": "qcow2", "encrypt.format": "luks", \
902636
           "encrypt.key-secret": "cluster_encrypt0", \
902636
           "file.driver": "file", "file.filename": "cluster_size_check.qcow2"}'
902636
ERROR: counting reference for region exceeding the end of the file by one cluster or more: offset 0x2000 size 0x1f9000
902636
Leaked cluster 4 refcount=1 reference=0
902636
...snip...
902636
Leaked cluster 130 refcount=1 reference=0
902636
902636
1 errors were found on the image.
902636
Data may be corrupted, or further writes to the image may corrupt it.
902636
902636
127 leaked clusters were found on the image.
902636
This means waste of disk space, but no harm to data.
902636
Image end offset: 268288
902636
902636
The problem only exists when the disk image is entirely empty. Writing
902636
data to the disk image payload will solve the problem by causing the
902636
end of the file to be extended further.
902636
902636
The change fixes it by ensuring that the entire allocated LUKS header
902636
region is fully initialized with zeros. The qemu-img check will still
902636
fail for any pre-existing disk images created prior to this change,
902636
unless at least 1 byte of the payload is written to.
902636
902636
Fully writing zeros to the entire LUKS header is a good idea regardless
902636
as it ensures that space has been allocated on the host filesystem (or
902636
whatever block storage backend is used).
902636
902636
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
902636
Message-Id: <20200207135520.2669430-1-berrange@redhat.com>
902636
Reviewed-by: Eric Blake <eblake@redhat.com>
902636
Signed-off-by: Max Reitz <mreitz@redhat.com>
902636
(cherry picked from commit 087ab8e775f48766068e65de1bc99d03b40d1670)
902636
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
902636
902636
Conflicts:
902636
	tests/qemu-iotests/group: no test 283 in downstream
902636
902636
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
902636
---
902636
 block/qcow2.c              | 11 ++++--
902636
 tests/qemu-iotests/284     | 97 ++++++++++++++++++++++++++++++++++++++++++++++
902636
 tests/qemu-iotests/284.out | 62 +++++++++++++++++++++++++++++
902636
 tests/qemu-iotests/group   |  1 +
902636
 4 files changed, 167 insertions(+), 4 deletions(-)
902636
 create mode 100755 tests/qemu-iotests/284
902636
 create mode 100644 tests/qemu-iotests/284.out
902636
902636
diff --git a/block/qcow2.c b/block/qcow2.c
902636
index 71067c6..af0ad4a 100644
902636
--- a/block/qcow2.c
902636
+++ b/block/qcow2.c
902636
@@ -135,13 +135,16 @@ static ssize_t qcow2_crypto_hdr_init_func(QCryptoBlock *block, size_t headerlen,
902636
     s->crypto_header.length = headerlen;
902636
     s->crypto_header.offset = ret;
902636
 
902636
-    /* Zero fill remaining space in cluster so it has predictable
902636
-     * content in case of future spec changes */
902636
+    /*
902636
+     * Zero fill all space in cluster so it has predictable
902636
+     * content, as we may not initialize some regions of the
902636
+     * header (eg only 1 out of 8 key slots will be initialized)
902636
+     */
902636
     clusterlen = size_to_clusters(s, headerlen) * s->cluster_size;
902636
     assert(qcow2_pre_write_overlap_check(bs, 0, ret, clusterlen, false) == 0);
902636
     ret = bdrv_pwrite_zeroes(bs->file,
902636
-                             ret + headerlen,
902636
-                             clusterlen - headerlen, 0);
902636
+                             ret,
902636
+                             clusterlen, 0);
902636
     if (ret < 0) {
902636
         error_setg_errno(errp, -ret, "Could not zero fill encryption header");
902636
         return -1;
902636
diff --git a/tests/qemu-iotests/284 b/tests/qemu-iotests/284
902636
new file mode 100755
902636
index 0000000..071e89b
902636
--- /dev/null
902636
+++ b/tests/qemu-iotests/284
902636
@@ -0,0 +1,97 @@
902636
+#!/usr/bin/env bash
902636
+#
902636
+# Test ref count checks on encrypted images
902636
+#
902636
+# Copyright (C) 2019 Red Hat, Inc.
902636
+#
902636
+# This program is free software; you can redistribute it and/or modify
902636
+# it under the terms of the GNU General Public License as published by
902636
+# the Free Software Foundation; either version 2 of the License, or
902636
+# (at your option) any later version.
902636
+#
902636
+# This program is distributed in the hope that it will be useful,
902636
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
902636
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
902636
+# GNU General Public License for more details.
902636
+#
902636
+# You should have received a copy of the GNU General Public License
902636
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
902636
+#
902636
+
902636
+# creator
902636
+owner=berrange@redhat.com
902636
+
902636
+seq=`basename $0`
902636
+echo "QA output created by $seq"
902636
+
902636
+status=1        # failure is the default!
902636
+
902636
+_cleanup()
902636
+{
902636
+        _cleanup_test_img
902636
+}
902636
+trap "_cleanup; exit \$status" 0 1 2 3 15
902636
+
902636
+# get standard environment, filters and checks
902636
+. ./common.rc
902636
+. ./common.filter
902636
+
902636
+_supported_fmt qcow2
902636
+_supported_proto generic
902636
+_supported_os Linux
902636
+
902636
+
902636
+size=1M
902636
+
902636
+SECRET="secret,id=sec0,data=astrochicken"
902636
+
902636
+IMGSPEC="driver=$IMGFMT,file.filename=$TEST_IMG,encrypt.key-secret=sec0"
902636
+QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
902636
+
902636
+_run_test()
902636
+{
902636
+        IMGOPTSSYNTAX=true
902636
+        OLD_TEST_IMG="$TEST_IMG"
902636
+        TEST_IMG="driver=$IMGFMT,file.filename=$TEST_IMG,encrypt.key-secret=sec0"
902636
+        QEMU_IMG_EXTRA_ARGS="--image-opts --object $SECRET"
902636
+
902636
+        echo
902636
+        echo "== cluster size $csize"
902636
+        echo "== checking image refcounts =="
902636
+        _check_test_img
902636
+
902636
+        echo
902636
+        echo "== writing some data =="
902636
+        $QEMU_IO -c "write -P 0x9 0 1"  $QEMU_IMG_EXTRA_ARGS $TEST_IMG | _filter_qemu_io | _filter_testdir
902636
+        echo
902636
+        echo "== rechecking image refcounts =="
902636
+        _check_test_img
902636
+
902636
+        echo
902636
+        echo "== writing some more data =="
902636
+        $QEMU_IO -c "write -P 0x9 $csize 1" $QEMU_IMG_EXTRA_ARGS $TEST_IMG | _filter_qemu_io | _filter_testdir
902636
+        echo
902636
+        echo "== rechecking image refcounts =="
902636
+        _check_test_img
902636
+
902636
+        TEST_IMG="$OLD_TEST_IMG"
902636
+        QEMU_IMG_EXTRA_ARGS=
902636
+        IMGOPTSSYNTAX=
902636
+}
902636
+
902636
+
902636
+echo
902636
+echo "testing LUKS qcow2 encryption"
902636
+echo
902636
+
902636
+for csize in 512 2048 32768
902636
+do
902636
+  _make_test_img --object $SECRET -o "encrypt.format=luks,encrypt.key-secret=sec0,encrypt.iter-time=10,cluster_size=$csize" $size
902636
+  _run_test
902636
+  _cleanup_test_img
902636
+done
902636
+
902636
+# success, all done
902636
+echo "*** done"
902636
+rm -f $seq.full
902636
+status=0
902636
diff --git a/tests/qemu-iotests/284.out b/tests/qemu-iotests/284.out
902636
new file mode 100644
902636
index 0000000..48216f5
902636
--- /dev/null
902636
+++ b/tests/qemu-iotests/284.out
902636
@@ -0,0 +1,62 @@
902636
+QA output created by 284
902636
+
902636
+testing LUKS qcow2 encryption
902636
+
902636
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
902636
+
902636
+== cluster size 512
902636
+== checking image refcounts ==
902636
+No errors were found on the image.
902636
+
902636
+== writing some data ==
902636
+wrote 1/1 bytes at offset 0
902636
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
902636
+
902636
+== rechecking image refcounts ==
902636
+No errors were found on the image.
902636
+
902636
+== writing some more data ==
902636
+wrote 1/1 bytes at offset 512
902636
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
902636
+
902636
+== rechecking image refcounts ==
902636
+No errors were found on the image.
902636
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
902636
+
902636
+== cluster size 2048
902636
+== checking image refcounts ==
902636
+No errors were found on the image.
902636
+
902636
+== writing some data ==
902636
+wrote 1/1 bytes at offset 0
902636
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
902636
+
902636
+== rechecking image refcounts ==
902636
+No errors were found on the image.
902636
+
902636
+== writing some more data ==
902636
+wrote 1/1 bytes at offset 2048
902636
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
902636
+
902636
+== rechecking image refcounts ==
902636
+No errors were found on the image.
902636
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
902636
+
902636
+== cluster size 32768
902636
+== checking image refcounts ==
902636
+No errors were found on the image.
902636
+
902636
+== writing some data ==
902636
+wrote 1/1 bytes at offset 0
902636
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
902636
+
902636
+== rechecking image refcounts ==
902636
+No errors were found on the image.
902636
+
902636
+== writing some more data ==
902636
+wrote 1/1 bytes at offset 32768
902636
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
902636
+
902636
+== rechecking image refcounts ==
902636
+No errors were found on the image.
902636
+*** done
902636
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
902636
index e47cbfc..9c565cf 100644
902636
--- a/tests/qemu-iotests/group
902636
+++ b/tests/qemu-iotests/group
902636
@@ -289,3 +289,4 @@
902636
 277 rw quick
902636
 280 rw migration quick
902636
 281 rw quick
902636
+284 rw
902636
-- 
902636
1.8.3.1
902636