All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxim Levitsky <mlevitsk@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Vladimir Sementsov-Ogievskiy" <vsementsov@virtuozzo.com>,
	"Daniel P . Berrangé" <berrange@redhat.com>,
	qemu-block@nongnu.org, qemu-stable <qemu-stable@nongnu.org>,
	"Maxim Levitsky" <mlevitsk@redhat.com>,
	"Max Reitz" <mreitz@redhat.com>
Subject: [Qemu-devel] [PATCH 2/3] block/qcow2: fix the corruption when rebasing luks encrypted files
Date: Fri,  6 Sep 2019 20:32:00 +0300	[thread overview]
Message-ID: <20190906173201.7926-3-mlevitsk@redhat.com> (raw)
In-Reply-To: <20190906173201.7926-1-mlevitsk@redhat.com>

This fixes subltle corruption introduced by luks threaded encryption
in commit 8ac0f15f335

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1745922

The corruption happens when we do
   * write to two or more unallocated clusters at once
   * write doesn't fully cover nether first nor last cluster

In this case, when allocating the new clusters we COW both area
prior to the write and after the write, and we encrypt them.

The above mentioned commit accidently made it so, we encrypt the
second COW are using the physical cluster offset of the first area.

Fix this by:
 * remove the offset_in_cluster parameter of do_perform_cow_encrypt
   since it is misleading. That offset can be larger that cluster size.
   instead just add the start and end COW are offsets to both host and guest offsets
   that do_perform_cow_encrypt receives.

*  in do_perform_cow_encrypt, remove the cluster offset from the host_offset
   And thus pass correctly to the qcow2_co_encrypt, the host cluster offset and full guest offset


Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 block/qcow2-cluster.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index b95e64c237..32477f0156 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -463,20 +463,20 @@ static int coroutine_fn do_perform_cow_read(BlockDriverState *bs,
 }
 
 static bool coroutine_fn do_perform_cow_encrypt(BlockDriverState *bs,
-                                                uint64_t guest_cluster_offset,
-                                                uint64_t host_cluster_offset,
-                                                unsigned offset_in_cluster,
+                                                uint64_t guest_offset,
+                                                uint64_t host_offset,
                                                 uint8_t *buffer,
                                                 unsigned bytes)
 {
     if (bytes && bs->encrypted) {
         BDRVQcow2State *s = bs->opaque;
-        assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
+        assert((guest_offset & ~BDRV_SECTOR_MASK) == 0);
+        assert((host_offset & ~BDRV_SECTOR_MASK) == 0);
         assert((bytes & ~BDRV_SECTOR_MASK) == 0);
         assert(s->crypto);
-        if (qcow2_co_encrypt(bs, host_cluster_offset,
-                             guest_cluster_offset + offset_in_cluster,
-                             buffer, bytes) < 0) {
+
+        if (qcow2_co_encrypt(bs, start_of_cluster(s, host_offset),
+                             guest_offset, buffer, bytes) < 0) {
             return false;
         }
     }
@@ -890,11 +890,15 @@ static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
 
     /* Encrypt the data if necessary before writing it */
     if (bs->encrypted) {
-        if (!do_perform_cow_encrypt(bs, m->offset, m->alloc_offset,
-                                    start->offset, start_buffer,
+        if (!do_perform_cow_encrypt(bs,
+                                    m->offset + start->offset,
+                                    m->alloc_offset + start->offset,
+                                    start_buffer,
                                     start->nb_bytes) ||
-            !do_perform_cow_encrypt(bs, m->offset, m->alloc_offset,
-                                    end->offset, end_buffer, end->nb_bytes)) {
+            !do_perform_cow_encrypt(bs,
+                                    m->offset + end->offset,
+                                    m->alloc_offset + end->offset,
+                                    end_buffer, end->nb_bytes)) {
             ret = -EIO;
             goto fail;
         }
-- 
2.17.2



  parent reply	other threads:[~2019-09-06 17:35 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-06 17:31 [Qemu-devel] [PATCH 0/3] Fix qcow2+luks corruption introduced by commit 8ac0f15f335 Maxim Levitsky
2019-09-06 17:31 ` [Qemu-devel] [PATCH 1/3] block/qcow2: refactoring of threaded encryption code Maxim Levitsky
2019-09-06 18:00   ` Eric Blake
2019-09-06 18:55     ` Maxim Levitsky
2019-09-06 19:00       ` Eric Blake
2019-09-06 19:03         ` Maxim Levitsky
2019-09-06 17:32 ` Maxim Levitsky [this message]
2019-09-06 19:17   ` [Qemu-devel] [PATCH 2/3] block/qcow2: fix the corruption when rebasing luks encrypted files Eric Blake
2019-09-06 19:46     ` Maxim Levitsky
2019-09-09 10:56     ` Kevin Wolf
2019-09-10 11:12       ` [Qemu-devel] [Qemu-stable] " Maxim Levitsky
2019-09-06 17:32 ` [Qemu-devel] [PATCH 3/3] qemu-iotests: test for bz #1745922 Maxim Levitsky
2019-09-06 17:37 ` [Qemu-devel] [PATCH 0/3] Fix qcow2+luks corruption introduced by commit 8ac0f15f335 Maxim Levitsky

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=20190906173201.7926-3-mlevitsk@redhat.com \
    --to=mlevitsk@redhat.com \
    --cc=berrange@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.