qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Subject: [Qemu-devel] [PULL 01/33] qcow2: fix encryption during cow of sectors
Date: Thu, 22 Sep 2016 18:29:02 +0200	[thread overview]
Message-ID: <1474561774-19256-2-git-send-email-kwolf@redhat.com> (raw)
In-Reply-To: <1474561774-19256-1-git-send-email-kwolf@redhat.com>

From: "Daniel P. Berrange" <berrange@redhat.com>

Broken in previous commit:

  commit aaa4d20b4972bb1a811ce929502e6741835d584e
  Author: Kevin Wolf <kwolf@redhat.com>
  Date:   Wed Jun 1 15:21:05 2016 +0200

      qcow2: Make copy_sectors() byte based

The copy_sectors() code was originally using the 'sector'
parameter for encryption, which was passed in by the caller
from the QCowL2Meta.offset field (aka the guest logical
offset).

After the change, the code is using 'cluster_offset' which
was passed in from QCow2L2Meta.alloc_offset field (aka the
host physical offset).

This would cause the data to be encrypted using an incorrect
initialization vector which will in turn cause later reads
to return garbage.

Although current qcow2 built-in encryption is blocked from
usage in the emulator, one could still hit this if writing
to the file via qemu-{img,io,nbd} commands.

Cc: qemu-stable@nongnu.org
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cluster.c      |  2 +-
 tests/qemu-iotests/158     | 80 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/158.out | 36 +++++++++++++++++++++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100755 tests/qemu-iotests/158
 create mode 100644 tests/qemu-iotests/158.out

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 9ab445d..61d1ffd 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -429,7 +429,7 @@ static int coroutine_fn do_perform_cow(BlockDriverState *bs,
 
     if (bs->encrypted) {
         Error *err = NULL;
-        int64_t sector = (cluster_offset + offset_in_cluster)
+        int64_t sector = (src_cluster_offset + offset_in_cluster)
                          >> BDRV_SECTOR_BITS;
         assert(s->cipher);
         assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
diff --git a/tests/qemu-iotests/158 b/tests/qemu-iotests/158
new file mode 100755
index 0000000..a6cdd6d
--- /dev/null
+++ b/tests/qemu-iotests/158
@@ -0,0 +1,80 @@
+#!/bin/bash
+#
+# Test encrypted read/write using backing files
+#
+# Copyright (C) 2015 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=berrange@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+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=128M
+TEST_IMG_BASE=$TEST_IMG.base
+
+TEST_IMG_SAVE=$TEST_IMG
+TEST_IMG=$TEST_IMG_BASE
+echo "== create base =="
+IMGOPTS="encryption=on" _make_test_img $size
+TEST_IMG=$TEST_IMG_SAVE
+
+echo
+echo "== writing whole image =="
+echo "astrochicken" | $QEMU_IO -c "write -P 0xa 0 $size" "$TEST_IMG_BASE" | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== verify pattern =="
+echo "astrochicken" | $QEMU_IO -c "read -P 0xa 0 $size" "$TEST_IMG_BASE" | _filter_qemu_io | _filter_testdir
+
+echo "== create overlay =="
+IMGOPTS="encryption=on" _make_test_img -b "$TEST_IMG_BASE" $size
+
+echo
+echo "== writing part of a cluster =="
+echo "astrochicken" | $QEMU_IO -c "write -P 0xe 0 1024" "$TEST_IMG" | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== verify pattern =="
+echo "astrochicken" | $QEMU_IO -c "read -P 0xe 0 1024" "$TEST_IMG" | _filter_qemu_io | _filter_testdir
+echo
+echo "== verify pattern =="
+echo "astrochicken" | $QEMU_IO -c "read -P 0xa 1024 64512" "$TEST_IMG" | _filter_qemu_io | _filter_testdir
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/158.out b/tests/qemu-iotests/158.out
new file mode 100644
index 0000000..b3f37e2
--- /dev/null
+++ b/tests/qemu-iotests/158.out
@@ -0,0 +1,36 @@
+QA output created by 158
+== create base ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728 encryption=on
+
+== writing whole image ==
+Disk image 'TEST_DIR/t.qcow2.base' is encrypted.
+password:
+wrote 134217728/134217728 bytes at offset 0
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+Disk image 'TEST_DIR/t.qcow2.base' is encrypted.
+password:
+read 134217728/134217728 bytes at offset 0
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+== create overlay ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base encryption=on
+
+== writing part of a cluster ==
+Disk image 'TEST_DIR/t.qcow2' is encrypted.
+password:
+wrote 1024/1024 bytes at offset 0
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+Disk image 'TEST_DIR/t.qcow2' is encrypted.
+password:
+read 1024/1024 bytes at offset 0
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+Disk image 'TEST_DIR/t.qcow2' is encrypted.
+password:
+read 64512/64512 bytes at offset 1024
+63 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index a57fc92..7eb1770 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -157,6 +157,7 @@
 155 rw auto
 156 rw auto quick
 157 auto
+158 rw auto quick
 159 rw auto quick
 160 rw auto quick
 162 auto quick
-- 
1.8.3.1

  reply	other threads:[~2016-09-22 16:29 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-22 16:29 [Qemu-devel] [PULL 00/33] Block layer patches Kevin Wolf
2016-09-22 16:29 ` Kevin Wolf [this message]
2016-09-22 16:29 ` [Qemu-devel] [PULL 02/33] hmp: Remove dead code in hmp_qemu_io() Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 03/33] tests: allow to specify list of formats to test for check-block.sh Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 04/33] block: Remove bdrv_is_snapshot Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 05/33] block: Set BDRV_O_ALLOW_RDWR and snapshot_options before storing the flags Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 06/33] block: Update bs->open_flags earlier in bdrv_open_common() Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 07/33] block: Add "read-only" to the options QDict Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 08/33] block: Don't queue the same BDS twice in bdrv_reopen_queue_child() Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 09/33] commit: Add 'base' to the reopen queue before 'overlay_bs' Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 10/33] block: rename "read-only" to BDRV_OPT_READ_ONLY Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 11/33] block: Fix 'since' for compressed Drive/BlockdevBackup Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 12/33] block: Add blk_by_dev() Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 13/33] qdev-monitor: Factor out find_device_state() Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 14/33] qdev-monitor: Add blk_by_qdev_id() Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 15/33] block: Accept device model name for blockdev-open/close-tray Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 16/33] block: Accept device model name for x-blockdev-insert-medium Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 17/33] block: Accept device model name for x-blockdev-remove-medium Kevin Wolf
2016-09-26 10:59   ` Paolo Bonzini
2016-09-26 13:08     ` Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 18/33] block: Accept device model name for eject Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 19/33] block: Accept device model name for blockdev-change-medium Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 20/33] block: Accept device model name for block_set_io_throttle Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 21/33] qemu-iotests/118: Test media change with qdev name Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 22/33] qemu-iotests/041: Avoid blockdev-add with id Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 23/33] qemu-iotests/067: " Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 24/33] qemu-iotests/071: " Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 25/33] qemu-iotests/081: " Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 26/33] qemu-iotests/087: " Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 27/33] qemu-iotests/117: " Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 28/33] qemu-iotests/118: " Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 29/33] qemu-iotests/124: " Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 30/33] qemu-iotests/139: " Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 31/33] block: Avoid printing NULL string in error messages Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 32/33] qemu-iotests/141: Avoid blockdev-add with id Kevin Wolf
2016-09-22 16:29 ` [Qemu-devel] [PULL 33/33] block: Remove BB interface from blockdev-add/del Kevin Wolf
2016-09-23 11:12 ` [Qemu-devel] [PULL 00/33] Block layer patches Peter Maydell
2016-09-23 11:41   ` Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1474561774-19256-2-git-send-email-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).