cryptospore / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone

Blame SOURCES/kvm-crypto.c-cleanup-created-file-when-block_crypto_co_c.patch

902636
From 043decff5812c1f46ed44dd0f82099e3b8bb6a28 Mon Sep 17 00:00:00 2001
902636
From: Maxim Levitsky <mlevitsk@redhat.com>
902636
Date: Sun, 31 May 2020 16:40:35 +0100
902636
Subject: [PATCH 7/7] crypto.c: cleanup created file when
902636
 block_crypto_co_create_opts_luks fails
902636
902636
RH-Author: Maxim Levitsky <mlevitsk@redhat.com>
902636
Message-id: <20200531164035.34188-4-mlevitsk@redhat.com>
902636
Patchwork-id: 97060
902636
O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH 3/3] crypto.c: cleanup created file when block_crypto_co_create_opts_luks fails
902636
Bugzilla: 1827630
902636
RH-Acked-by: Sergio Lopez Pascual <slp@redhat.com>
902636
RH-Acked-by: John Snow <jsnow@redhat.com>
902636
RH-Acked-by: Eric Blake <eblake@redhat.com>
902636
902636
From: Daniel Henrique Barboza <danielhb413@gmail.com>
902636
902636
When using a non-UTF8 secret to create a volume using qemu-img, the
902636
following error happens:
902636
902636
$ qemu-img create -f luks --object secret,id=vol_1_encrypt0,file=vol_resize_pool.vol_1.secret.qzVQrI -o key-secret=vol_1_encrypt0 /var/tmp/pool_target/vol_1 10240K
902636
902636
Formatting '/var/tmp/pool_target/vol_1', fmt=luks size=10485760 key-secret=vol_1_encrypt0
902636
qemu-img: /var/tmp/pool_target/vol_1: Data from secret vol_1_encrypt0 is not valid UTF-8
902636
902636
However, the created file '/var/tmp/pool_target/vol_1' is left behind in the
902636
file system after the failure. This behavior can be observed when creating
902636
the volume using Libvirt, via 'virsh vol-create', and then getting "volume
902636
target path already exist" errors when trying to re-create the volume.
902636
902636
The volume file is created inside block_crypto_co_create_opts_luks(), in
902636
block/crypto.c. If the bdrv_create_file() call is successful but any
902636
succeeding step fails*, the existing 'fail' label does not take into
902636
account the created file, leaving it behind.
902636
902636
This patch changes block_crypto_co_create_opts_luks() to delete
902636
'filename' in case of failure. A failure in this point means that
902636
the volume is now truncated/corrupted, so even if 'filename' was an
902636
existing volume before calling qemu-img, it is now unusable. Deleting
902636
the file it is not much worse than leaving it in the filesystem in
902636
this scenario, and we don't have to deal with checking the file
902636
pre-existence in the code.
902636
902636
* in our case, block_crypto_co_create_generic calls qcrypto_block_create,
902636
which calls qcrypto_block_luks_create, and this function fails when
902636
calling qcrypto_secret_lookup_as_utf8.
902636
902636
Reported-by: Srikanth Aithal <bssrikanth@in.ibm.com>
902636
Suggested-by: Kevin Wolf <kwolf@redhat.com>
902636
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
902636
Message-Id: <20200130213907.2830642-4-danielhb413@gmail.com>
902636
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
902636
(cherry picked from commit 1bba30da24e1124ceeb0693c81382a0d77e20ca5)
902636
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
902636
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
902636
---
902636
 block/crypto.c | 18 ++++++++++++++++++
902636
 1 file changed, 18 insertions(+)
902636
902636
diff --git a/block/crypto.c b/block/crypto.c
902636
index 970d463..5e3b15c 100644
902636
--- a/block/crypto.c
902636
+++ b/block/crypto.c
902636
@@ -30,6 +30,7 @@
902636
 #include "qapi/error.h"
902636
 #include "qemu/module.h"
902636
 #include "qemu/option.h"
902636
+#include "qemu/cutils.h"
902636
 #include "crypto.h"
902636
 
902636
 typedef struct BlockCrypto BlockCrypto;
902636
@@ -597,6 +598,23 @@ static int coroutine_fn block_crypto_co_create_opts_luks(BlockDriver *drv,
902636
 
902636
     ret = 0;
902636
 fail:
902636
+    /*
902636
+     * If an error occurred, delete 'filename'. Even if the file existed
902636
+     * beforehand, it has been truncated and corrupted in the process.
902636
+     */
902636
+    if (ret && bs) {
902636
+        Error *local_delete_err = NULL;
902636
+        int r_del = bdrv_co_delete_file(bs, &local_delete_err);
902636
+        /*
902636
+         * ENOTSUP will happen if the block driver doesn't support
902636
+         * the 'bdrv_co_delete_file' interface. This is a predictable
902636
+         * scenario and shouldn't be reported back to the user.
902636
+         */
902636
+        if ((r_del < 0) && (r_del != -ENOTSUP)) {
902636
+            error_report_err(local_delete_err);
902636
+        }
902636
+    }
902636
+
902636
     bdrv_unref(bs);
902636
     qapi_free_QCryptoBlockCreateOptions(create_opts);
902636
     qobject_unref(cryptoopts);
902636
-- 
902636
1.8.3.1
902636