* [PULL 1/3] block/vvfat.c fix leak when failure occurs
2021-11-23 15:59 [PULL 0/3] Block patches Hanna Reitz
@ 2021-11-23 15:59 ` Hanna Reitz
2021-11-23 15:59 ` [PULL 2/3] iotests: Use aes-128-cbc Hanna Reitz
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Hanna Reitz @ 2021-11-23 15:59 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Peter Maydell, Hanna Reitz, Richard Henderson,
qemu-devel
From: Daniella Lee <daniellalee111@gmail.com>
Function vvfat_open called function enable_write_target and init_directories,
and these functions malloc new memory for BDRVVVFATState::qcow_filename,
BDRVVVFATState::used_clusters, and BDRVVVFATState::cluster_buff.
When the specified folder does not exist ,it may contains memory leak.
After init_directories function is executed, the vvfat_open return -EIO,
and bdrv_open_driver goto label open_failed,
the program use g_free(bs->opaque) to release BDRVVVFATState struct
without members mentioned.
command line:
qemu-system-x86_64 -hdb <vdisk qcow file> -usb -device usb-storage,drive=fat16
-drive file=fat:rw:fat-type=16:"<path of a host folder does not exist>",
id=fat16,format=raw,if=none
enable_write_target called:
(gdb) bt
at ../block/vvfat.c:3114
flags=155650, errp=0x7fffffffd780) at ../block/vvfat.c:1236
node_name=0x0, options=0x555556fa45d0, open_flags=155650,
errp=0x7fffffffd890) at ../block.c:1558
errp=0x7fffffffd890) at ../block.c:1852
reference=0x0, options=0x555556fa45d0, flags=40962, parent=0x555556f98cd0,
child_class=0x555556b1d6a0 <child_of_bds>, child_role=19,
errp=0x7fffffffda90) at ../block.c:3779
options=0x555556f9cfc0, bdref_key=0x555556239bb8 "file",
parent=0x555556f98cd0, child_class=0x555556b1d6a0 <child_of_bds>,
child_role=19, allow_none=true, errp=0x7fffffffda90) at ../block.c:3419
reference=0x0, options=0x555556f9cfc0, flags=8194, parent=0x0,
child_class=0x0, child_role=0, errp=0x555556c98c40 <error_fatal>)
at ../block.c:3726
options=0x555556f757b0, flags=0, errp=0x555556c98c40 <error_fatal>)
at ../block.c:3872
options=0x555556f757b0, flags=0, errp=0x555556c98c40 <error_fatal>)
at ../block/block-backend.c:436
bs_opts=0x555556f757b0, errp=0x555556c98c40 <error_fatal>)
at ../blockdev.c:608
errp=0x555556c98c40 <error_fatal>) at ../blockdev.c:992
......
Signed-off-by: Daniella Lee <daniellalee111@gmail.com>
Message-Id: <20211119112553.352222-1-daniellalee111@gmail.com>
[hreitz: Took commit message from v1]
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
block/vvfat.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 05e78e3c27..5dacc6cfac 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1279,8 +1279,18 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
qemu_co_mutex_init(&s->lock);
- ret = 0;
+ qemu_opts_del(opts);
+
+ return 0;
+
fail:
+ g_free(s->qcow_filename);
+ s->qcow_filename = NULL;
+ g_free(s->cluster_buffer);
+ s->cluster_buffer = NULL;
+ g_free(s->used_clusters);
+ s->used_clusters = NULL;
+
qemu_opts_del(opts);
return ret;
}
@@ -3118,7 +3128,7 @@ static int enable_write_target(BlockDriverState *bs, Error **errp)
int size = sector2cluster(s, s->sector_count);
QDict *options;
- s->used_clusters = calloc(size, 1);
+ s->used_clusters = g_malloc0(size);
array_init(&(s->commits), sizeof(commit_t));
@@ -3166,8 +3176,6 @@ static int enable_write_target(BlockDriverState *bs, Error **errp)
return 0;
err:
- g_free(s->qcow_filename);
- s->qcow_filename = NULL;
return ret;
}
--
2.33.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PULL 2/3] iotests: Use aes-128-cbc
2021-11-23 15:59 [PULL 0/3] Block patches Hanna Reitz
2021-11-23 15:59 ` [PULL 1/3] block/vvfat.c fix leak when failure occurs Hanna Reitz
@ 2021-11-23 15:59 ` Hanna Reitz
2021-11-23 15:59 ` [PULL 3/3] iotests/149: Skip on unsupported ciphers Hanna Reitz
2021-11-23 17:58 ` [PULL 0/3] Block patches Richard Henderson
3 siblings, 0 replies; 5+ messages in thread
From: Hanna Reitz @ 2021-11-23 15:59 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Peter Maydell, Hanna Reitz, Richard Henderson,
qemu-devel
Our gnutls crypto backend (which is the default as of 8bd0931f6)
supports neither twofish-128 nor the CTR mode. CBC and aes-128 are
supported by all of our backends (as far as I can tell), so use
aes-128-cbc in our iotests.
(We could also use e.g. aes-256-cbc, but the different key sizes would
lead to different key slot offsets and so change the reference output
more, which is why I went with aes-128.)
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20211117151707.52549-2-hreitz@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
---
tests/qemu-iotests/206 | 4 ++--
tests/qemu-iotests/206.out | 6 +++---
tests/qemu-iotests/210 | 4 ++--
tests/qemu-iotests/210.out | 6 +++---
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/tests/qemu-iotests/206 b/tests/qemu-iotests/206
index c3cdad4ce4..10eff343f7 100755
--- a/tests/qemu-iotests/206
+++ b/tests/qemu-iotests/206
@@ -162,8 +162,8 @@ with iotests.FilePath('t.qcow2') as disk_path, \
'encrypt': {
'format': 'luks',
'key-secret': 'keysec0',
- 'cipher-alg': 'twofish-128',
- 'cipher-mode': 'ctr',
+ 'cipher-alg': 'aes-128',
+ 'cipher-mode': 'cbc',
'ivgen-alg': 'plain64',
'ivgen-hash-alg': 'md5',
'hash-alg': 'sha1',
diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out
index 3593e8e9c2..80cd274223 100644
--- a/tests/qemu-iotests/206.out
+++ b/tests/qemu-iotests/206.out
@@ -97,7 +97,7 @@ Format specific information:
=== Successful image creation (encrypted) ===
-{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "qcow2", "encrypt": {"cipher-alg": "twofish-128", "cipher-mode": "ctr", "format": "luks", "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0"}, "file": {"driver": "file", "filename": "TEST_DIR/PID-t.qcow2"}, "size": 33554432}}}
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "qcow2", "encrypt": {"cipher-alg": "aes-128", "cipher-mode": "cbc", "format": "luks", "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0"}, "file": {"driver": "file", "filename": "TEST_DIR/PID-t.qcow2"}, "size": 33554432}}}
{"return": {}}
{"execute": "job-dismiss", "arguments": {"id": "job0"}}
{"return": {}}
@@ -115,10 +115,10 @@ Format specific information:
encrypt:
ivgen alg: plain64
hash alg: sha1
- cipher alg: twofish-128
+ cipher alg: aes-128
uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
format: luks
- cipher mode: ctr
+ cipher mode: cbc
slots:
[0]:
active: true
diff --git a/tests/qemu-iotests/210 b/tests/qemu-iotests/210
index 5a62ed4dd1..a4dcc5fe59 100755
--- a/tests/qemu-iotests/210
+++ b/tests/qemu-iotests/210
@@ -83,8 +83,8 @@ with iotests.FilePath('t.luks') as disk_path, \
},
'size': size,
'key-secret': 'keysec0',
- 'cipher-alg': 'twofish-128',
- 'cipher-mode': 'ctr',
+ 'cipher-alg': 'aes-128',
+ 'cipher-mode': 'cbc',
'ivgen-alg': 'plain64',
'ivgen-hash-alg': 'md5',
'hash-alg': 'sha1',
diff --git a/tests/qemu-iotests/210.out b/tests/qemu-iotests/210.out
index 55c0844370..96d9f749dd 100644
--- a/tests/qemu-iotests/210.out
+++ b/tests/qemu-iotests/210.out
@@ -59,7 +59,7 @@ Format specific information:
{"execute": "job-dismiss", "arguments": {"id": "job0"}}
{"return": {}}
-{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"cipher-alg": "twofish-128", "cipher-mode": "ctr", "driver": "luks", "file": {"driver": "file", "filename": "TEST_DIR/PID-t.luks"}, "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0", "size": 67108864}}}
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"cipher-alg": "aes-128", "cipher-mode": "cbc", "driver": "luks", "file": {"driver": "file", "filename": "TEST_DIR/PID-t.luks"}, "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0", "size": 67108864}}}
{"return": {}}
{"execute": "job-dismiss", "arguments": {"id": "job0"}}
{"return": {}}
@@ -71,9 +71,9 @@ encrypted: yes
Format specific information:
ivgen alg: plain64
hash alg: sha1
- cipher alg: twofish-128
+ cipher alg: aes-128
uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
- cipher mode: ctr
+ cipher mode: cbc
slots:
[0]:
active: true
--
2.33.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PULL 3/3] iotests/149: Skip on unsupported ciphers
2021-11-23 15:59 [PULL 0/3] Block patches Hanna Reitz
2021-11-23 15:59 ` [PULL 1/3] block/vvfat.c fix leak when failure occurs Hanna Reitz
2021-11-23 15:59 ` [PULL 2/3] iotests: Use aes-128-cbc Hanna Reitz
@ 2021-11-23 15:59 ` Hanna Reitz
2021-11-23 17:58 ` [PULL 0/3] Block patches Richard Henderson
3 siblings, 0 replies; 5+ messages in thread
From: Hanna Reitz @ 2021-11-23 15:59 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Peter Maydell, Hanna Reitz, Richard Henderson,
qemu-devel
Whenever qemu-img or qemu-io report that some cipher is unsupported,
skip the whole test, because that is probably because qemu has been
configured with the gnutls crypto backend.
We could taylor the algorithm list to what gnutls supports, but this is
a test that is run rather rarely anyway (because it requires
password-less sudo), and so it seems better and easier to skip it. When
this test is intentionally run to check LUKS compatibility, it seems
better not to limit the algorithms but keep the list extensive.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20211117151707.52549-3-hreitz@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qemu-iotests/149 | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index 328fd05a4c..d49646ca60 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -230,6 +230,18 @@ def create_image(config, size_mb):
fn.truncate(size_mb * 1024 * 1024)
+def check_cipher_support(config, output):
+ """Check the output of qemu-img or qemu-io for mention of the respective
+ cipher algorithm being unsupported, and if so, skip this test.
+ (Returns `output` for convenience.)"""
+
+ if 'Unsupported cipher algorithm' in output:
+ iotests.notrun('Unsupported cipher algorithm '
+ f'{config.cipher}-{config.keylen}-{config.mode}; '
+ 'consider configuring qemu with a different crypto '
+ 'backend')
+ return output
+
def qemu_img_create(config, size_mb):
"""Create and format a disk image with LUKS using qemu-img"""
@@ -253,7 +265,8 @@ def qemu_img_create(config, size_mb):
"%dM" % size_mb]
iotests.log("qemu-img " + " ".join(args), filters=[iotests.filter_test_dir])
- iotests.log(iotests.qemu_img_pipe(*args), filters=[iotests.filter_test_dir])
+ iotests.log(check_cipher_support(config, iotests.qemu_img_pipe(*args)),
+ filters=[iotests.filter_test_dir])
def qemu_io_image_args(config, dev=False):
"""Get the args for access an image or device with qemu-io"""
@@ -279,8 +292,8 @@ def qemu_io_write_pattern(config, pattern, offset_mb, size_mb, dev=False):
args = ["-c", "write -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
args.extend(qemu_io_image_args(config, dev))
iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
- iotests.log(iotests.qemu_io(*args), filters=[iotests.filter_test_dir,
- iotests.filter_qemu_io])
+ iotests.log(check_cipher_support(config, iotests.qemu_io(*args)),
+ filters=[iotests.filter_test_dir, iotests.filter_qemu_io])
def qemu_io_read_pattern(config, pattern, offset_mb, size_mb, dev=False):
@@ -291,8 +304,8 @@ def qemu_io_read_pattern(config, pattern, offset_mb, size_mb, dev=False):
args = ["-c", "read -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
args.extend(qemu_io_image_args(config, dev))
iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
- iotests.log(iotests.qemu_io(*args), filters=[iotests.filter_test_dir,
- iotests.filter_qemu_io])
+ iotests.log(check_cipher_support(config, iotests.qemu_io(*args)),
+ filters=[iotests.filter_test_dir, iotests.filter_qemu_io])
def test_once(config, qemu_img=False):
--
2.33.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PULL 0/3] Block patches
2021-11-23 15:59 [PULL 0/3] Block patches Hanna Reitz
` (2 preceding siblings ...)
2021-11-23 15:59 ` [PULL 3/3] iotests/149: Skip on unsupported ciphers Hanna Reitz
@ 2021-11-23 17:58 ` Richard Henderson
3 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2021-11-23 17:58 UTC (permalink / raw)
To: Hanna Reitz, qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel
On 11/23/21 4:59 PM, Hanna Reitz wrote:
> The following changes since commit 73e0f70e097b7c92a5ce16ee35b53afe119b20d7:
>
> Merge tag 'pull-lu-20211123' of https://gitlab.com/rth7680/qemu into staging (2021-11-23 11:33:14 +0100)
>
> are available in the Git repository at:
>
> https://gitlab.com/hreitz/qemu.git tags/pull-block-2021-11-23
>
> for you to fetch changes up to 4dd218fd0717ed3cddb69c01eeb9da630107d89d:
>
> iotests/149: Skip on unsupported ciphers (2021-11-23 15:39:12 +0100)
>
> ----------------------------------------------------------------
> Block patches for 6.2-rc2:
> - Fix memory leak in vvfat when vvfat_open() fails
> - iotest fixes for the gnutls crypto backend
>
> ----------------------------------------------------------------
> Daniella Lee (1):
> block/vvfat.c fix leak when failure occurs
>
> Hanna Reitz (2):
> iotests: Use aes-128-cbc
> iotests/149: Skip on unsupported ciphers
>
> block/vvfat.c | 16 ++++++++++++----
> tests/qemu-iotests/149 | 23 ++++++++++++++++++-----
> tests/qemu-iotests/206 | 4 ++--
> tests/qemu-iotests/206.out | 6 +++---
> tests/qemu-iotests/210 | 4 ++--
> tests/qemu-iotests/210.out | 6 +++---
> 6 files changed, 40 insertions(+), 19 deletions(-)
Applied, thanks.
r~
^ permalink raw reply [flat|nested] 5+ messages in thread