From 043decff5812c1f46ed44dd0f82099e3b8bb6a28 Mon Sep 17 00:00:00 2001

From: Maxim Levitsky <mlevitsk@redhat.com>

Date: Sun, 31 May 2020 16:40:35 +0100

Subject: [PATCH 7/7] crypto.c: cleanup created file when

 block_crypto_co_create_opts_luks fails

RH-Author: Maxim Levitsky <mlevitsk@redhat.com>

Message-id: <20200531164035.34188-4-mlevitsk@redhat.com>

Patchwork-id: 97060

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

Bugzilla: 1827630

RH-Acked-by: Sergio Lopez Pascual <slp@redhat.com>

RH-Acked-by: John Snow <jsnow@redhat.com>

RH-Acked-by: Eric Blake <eblake@redhat.com>

From: Daniel Henrique Barboza <danielhb413@gmail.com>

When using a non-UTF8 secret to create a volume using qemu-img, the

following error happens:

$ 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

Formatting '/var/tmp/pool_target/vol_1', fmt=luks size=10485760 key-secret=vol_1_encrypt0

qemu-img: /var/tmp/pool_target/vol_1: Data from secret vol_1_encrypt0 is not valid UTF-8

However, the created file '/var/tmp/pool_target/vol_1' is left behind in the

file system after the failure. This behavior can be observed when creating

the volume using Libvirt, via 'virsh vol-create', and then getting "volume

target path already exist" errors when trying to re-create the volume.

The volume file is created inside block_crypto_co_create_opts_luks(), in

block/crypto.c. If the bdrv_create_file() call is successful but any

succeeding step fails*, the existing 'fail' label does not take into

account the created file, leaving it behind.

This patch changes block_crypto_co_create_opts_luks() to delete

'filename' in case of failure. A failure in this point means that

the volume is now truncated/corrupted, so even if 'filename' was an

existing volume before calling qemu-img, it is now unusable. Deleting

the file it is not much worse than leaving it in the filesystem in

this scenario, and we don't have to deal with checking the file

pre-existence in the code.

* in our case, block_crypto_co_create_generic calls qcrypto_block_create,

which calls qcrypto_block_luks_create, and this function fails when

calling qcrypto_secret_lookup_as_utf8.

Reported-by: Srikanth Aithal <bssrikanth@in.ibm.com>

Suggested-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Message-Id: <20200130213907.2830642-4-danielhb413@gmail.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

(cherry picked from commit 1bba30da24e1124ceeb0693c81382a0d77e20ca5)

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>

Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>

---

 block/crypto.c | 18 ++++++++++++++++++

 1 file changed, 18 insertions(+)

diff --git a/block/crypto.c b/block/crypto.c

index 970d463..5e3b15c 100644

--- a/block/crypto.c

+++ b/block/crypto.c

@@ -30,6 +30,7 @@

 #include "qapi/error.h"

 #include "qemu/module.h"

 #include "qemu/option.h"

+#include "qemu/cutils.h"

 #include "crypto.h"

 typedef struct BlockCrypto BlockCrypto;

@@ -597,6 +598,23 @@ static int coroutine_fn block_crypto_co_create_opts_luks(BlockDriver *drv,

     ret = 0;

 fail:

+    /*

+     * If an error occurred, delete 'filename'. Even if the file existed

+     * beforehand, it has been truncated and corrupted in the process.

+     */

+    if (ret && bs) {

+        Error *local_delete_err = NULL;

+        int r_del = bdrv_co_delete_file(bs, &local_delete_err);

+        /*

+         * ENOTSUP will happen if the block driver doesn't support

+         * the 'bdrv_co_delete_file' interface. This is a predictable

+         * scenario and shouldn't be reported back to the user.

+         */

+        if ((r_del < 0) && (r_del != -ENOTSUP)) {

+            error_report_err(local_delete_err);

+        }

+    }

+

     bdrv_unref(bs);

     qapi_free_QCryptoBlockCreateOptions(create_opts);

     qobject_unref(cryptoopts);

-- 

1.8.3.1