* [PATCH 0/2] fix two small memleaks
@ 2020-02-26 3:30 Pan Nengyuan
2020-02-26 3:30 ` [PATCH 1/2] block/qcow2: do free crypto_opts in qcow2_close() Pan Nengyuan
2020-02-26 3:30 ` [PATCH 2/2] qemu-img: free memory before re-assign Pan Nengyuan
0 siblings, 2 replies; 6+ messages in thread
From: Pan Nengyuan @ 2020-02-26 3:30 UTC (permalink / raw)
To: kwolf, mreitz
Cc: euler.robot, Pan Nengyuan, qemu-devel, qemu-block,
zhang.zhanghailiang
This series fix two small memleaks.
1. 'crypto_opts' forgot to free in qcow2_close(), do this cleanup in qcow2_close();
2. Do free filename/format in collect_image_check() when we re-allocate it.
Pan Nengyuan (2):
block/qcow2: do free crypto_opts in qcow2_close()
qemu-img: free memory before re-assign
block/qcow2.c | 1 +
qemu-img.c | 2 ++
2 files changed, 3 insertions(+)
--
2.18.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] block/qcow2: do free crypto_opts in qcow2_close()
2020-02-26 3:30 [PATCH 0/2] fix two small memleaks Pan Nengyuan
@ 2020-02-26 3:30 ` Pan Nengyuan
2020-02-26 10:14 ` Max Reitz
2020-02-26 3:30 ` [PATCH 2/2] qemu-img: free memory before re-assign Pan Nengyuan
1 sibling, 1 reply; 6+ messages in thread
From: Pan Nengyuan @ 2020-02-26 3:30 UTC (permalink / raw)
To: kwolf, mreitz
Cc: euler.robot, Pan Nengyuan, qemu-devel, qemu-block,
zhang.zhanghailiang
'crypto_opts' forgot to free in qcow2_close(), this patch fix the bellow leak stack:
Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7f0edd81f970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7f0edc6d149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x55d7eaede63d in qobject_input_start_struct /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qobject-input-visitor.c:295
#3 0x55d7eaed78b8 in visit_start_struct /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qapi-visit-core.c:49
#4 0x55d7eaf5140b in visit_type_QCryptoBlockOpenOptions qapi/qapi-visit-crypto.c:290
#5 0x55d7eae43af3 in block_crypto_open_opts_init /mnt/sdb/qemu-new/qemu_test/qemu/block/crypto.c:163
#6 0x55d7eacd2924 in qcow2_update_options_prepare /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1148
#7 0x55d7eacd33f7 in qcow2_update_options /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1232
#8 0x55d7eacd9680 in qcow2_do_open /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1512
#9 0x55d7eacdc55e in qcow2_open_entry /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1792
#10 0x55d7eacdc8fe in qcow2_open /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1819
#11 0x55d7eac3742d in bdrv_open_driver /mnt/sdb/qemu-new/qemu_test/qemu/block.c:1317
#12 0x55d7eac3e990 in bdrv_open_common /mnt/sdb/qemu-new/qemu_test/qemu/block.c:1575
#13 0x55d7eac4442c in bdrv_open_inherit /mnt/sdb/qemu-new/qemu_test/qemu/block.c:3126
#14 0x55d7eac45c3f in bdrv_open /mnt/sdb/qemu-new/qemu_test/qemu/block.c:3219
#15 0x55d7ead8e8a4 in blk_new_open /mnt/sdb/qemu-new/qemu_test/qemu/block/block-backend.c:397
#16 0x55d7eacde74c in qcow2_co_create /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:3534
#17 0x55d7eacdfa6d in qcow2_co_create_opts /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:3668
#18 0x55d7eac1c678 in bdrv_create_co_entry /mnt/sdb/qemu-new/qemu_test/qemu/block.c:485
#19 0x55d7eb0024d2 in coroutine_trampoline /mnt/sdb/qemu-new/qemu_test/qemu/util/coroutine-ucontext.c:115
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
block/qcow2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/qcow2.c b/block/qcow2.c
index 8dcee5efec..ac231b688e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2603,6 +2603,7 @@ static void qcow2_close(BlockDriverState *bs)
qcrypto_block_free(s->crypto);
s->crypto = NULL;
+ qapi_free_QCryptoBlockOpenOptions(s->crypto_opts);
g_free(s->unknown_header_fields);
cleanup_unknown_header_ext(bs);
--
2.18.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] block/qcow2: do free crypto_opts in qcow2_close()
2020-02-26 3:30 ` [PATCH 1/2] block/qcow2: do free crypto_opts in qcow2_close() Pan Nengyuan
@ 2020-02-26 10:14 ` Max Reitz
0 siblings, 0 replies; 6+ messages in thread
From: Max Reitz @ 2020-02-26 10:14 UTC (permalink / raw)
To: Pan Nengyuan, kwolf
Cc: euler.robot, qemu-devel, qemu-block, zhang.zhanghailiang
[-- Attachment #1.1: Type: text/plain, Size: 2360 bytes --]
On 26.02.20 04:30, Pan Nengyuan wrote:
> 'crypto_opts' forgot to free in qcow2_close(), this patch fix the bellow leak stack:
>
> Direct leak of 24 byte(s) in 1 object(s) allocated from:
> #0 0x7f0edd81f970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
> #1 0x7f0edc6d149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
> #2 0x55d7eaede63d in qobject_input_start_struct /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qobject-input-visitor.c:295
> #3 0x55d7eaed78b8 in visit_start_struct /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qapi-visit-core.c:49
> #4 0x55d7eaf5140b in visit_type_QCryptoBlockOpenOptions qapi/qapi-visit-crypto.c:290
> #5 0x55d7eae43af3 in block_crypto_open_opts_init /mnt/sdb/qemu-new/qemu_test/qemu/block/crypto.c:163
> #6 0x55d7eacd2924 in qcow2_update_options_prepare /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1148
> #7 0x55d7eacd33f7 in qcow2_update_options /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1232
> #8 0x55d7eacd9680 in qcow2_do_open /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1512
> #9 0x55d7eacdc55e in qcow2_open_entry /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1792
> #10 0x55d7eacdc8fe in qcow2_open /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1819
> #11 0x55d7eac3742d in bdrv_open_driver /mnt/sdb/qemu-new/qemu_test/qemu/block.c:1317
> #12 0x55d7eac3e990 in bdrv_open_common /mnt/sdb/qemu-new/qemu_test/qemu/block.c:1575
> #13 0x55d7eac4442c in bdrv_open_inherit /mnt/sdb/qemu-new/qemu_test/qemu/block.c:3126
> #14 0x55d7eac45c3f in bdrv_open /mnt/sdb/qemu-new/qemu_test/qemu/block.c:3219
> #15 0x55d7ead8e8a4 in blk_new_open /mnt/sdb/qemu-new/qemu_test/qemu/block/block-backend.c:397
> #16 0x55d7eacde74c in qcow2_co_create /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:3534
> #17 0x55d7eacdfa6d in qcow2_co_create_opts /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:3668
> #18 0x55d7eac1c678 in bdrv_create_co_entry /mnt/sdb/qemu-new/qemu_test/qemu/block.c:485
> #19 0x55d7eb0024d2 in coroutine_trampoline /mnt/sdb/qemu-new/qemu_test/qemu/util/coroutine-ucontext.c:115
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> block/qcow2.c | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] qemu-img: free memory before re-assign
2020-02-26 3:30 [PATCH 0/2] fix two small memleaks Pan Nengyuan
2020-02-26 3:30 ` [PATCH 1/2] block/qcow2: do free crypto_opts in qcow2_close() Pan Nengyuan
@ 2020-02-26 3:30 ` Pan Nengyuan
2020-02-26 10:13 ` Max Reitz
1 sibling, 1 reply; 6+ messages in thread
From: Pan Nengyuan @ 2020-02-26 3:30 UTC (permalink / raw)
To: kwolf, mreitz
Cc: euler.robot, Pan Nengyuan, qemu-devel, qemu-block,
zhang.zhanghailiang
collect_image_check() is called twice in img_check(), the filename/format will be alloced without free the original memory.
It is not a big deal since the process will exit anyway, but seems like a clean code and it will remove the warning spotted by asan.
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
qemu-img.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/qemu-img.c b/qemu-img.c
index 2b4562b9d9..bcbca6c9a2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -638,6 +638,8 @@ static int collect_image_check(BlockDriverState *bs,
return ret;
}
+ g_free(check->filename);
+ g_free(check->format);
check->filename = g_strdup(filename);
check->format = g_strdup(bdrv_get_format_name(bs));
check->check_errors = result.check_errors;
--
2.18.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] qemu-img: free memory before re-assign
2020-02-26 3:30 ` [PATCH 2/2] qemu-img: free memory before re-assign Pan Nengyuan
@ 2020-02-26 10:13 ` Max Reitz
2020-02-26 10:32 ` Pan Nengyuan
0 siblings, 1 reply; 6+ messages in thread
From: Max Reitz @ 2020-02-26 10:13 UTC (permalink / raw)
To: Pan Nengyuan, kwolf
Cc: euler.robot, qemu-devel, qemu-block, zhang.zhanghailiang
[-- Attachment #1.1: Type: text/plain, Size: 813 bytes --]
On 26.02.20 04:30, Pan Nengyuan wrote:
> collect_image_check() is called twice in img_check(), the filename/format will be alloced without free the original memory.
> It is not a big deal since the process will exit anyway, but seems like a clean code and it will remove the warning spotted by asan.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> qemu-img.c | 2 ++
> 1 file changed, 2 insertions(+)
I think this should happen in the caller. And there I think it would
make more sense to completely discard the old object and allocate a new one:
qapi_free_ImageCheck(check);
check = g_new0(ImageCheck, 1);
This way, we can’t forget to free any fields if new pointers were to be
added to the ImageCheck object.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] qemu-img: free memory before re-assign
2020-02-26 10:13 ` Max Reitz
@ 2020-02-26 10:32 ` Pan Nengyuan
0 siblings, 0 replies; 6+ messages in thread
From: Pan Nengyuan @ 2020-02-26 10:32 UTC (permalink / raw)
To: Max Reitz, kwolf; +Cc: euler.robot, qemu-devel, qemu-block, zhang.zhanghailiang
On 2/26/2020 6:13 PM, Max Reitz wrote:
> On 26.02.20 04:30, Pan Nengyuan wrote:
>> collect_image_check() is called twice in img_check(), the filename/format will be alloced without free the original memory.
>> It is not a big deal since the process will exit anyway, but seems like a clean code and it will remove the warning spotted by asan.
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
>> ---
>> qemu-img.c | 2 ++
>> 1 file changed, 2 insertions(+)
>
> I think this should happen in the caller. And there I think it would
> make more sense to completely discard the old object and allocate a new one:
>
> qapi_free_ImageCheck(check);
> check = g_new0(ImageCheck, 1);
>
> This way, we can’t forget to free any fields if new pointers were to be
> added to the ImageCheck object.
Good idea, thanks.
>
> Max
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-26 10:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-26 3:30 [PATCH 0/2] fix two small memleaks Pan Nengyuan
2020-02-26 3:30 ` [PATCH 1/2] block/qcow2: do free crypto_opts in qcow2_close() Pan Nengyuan
2020-02-26 10:14 ` Max Reitz
2020-02-26 3:30 ` [PATCH 2/2] qemu-img: free memory before re-assign Pan Nengyuan
2020-02-26 10:13 ` Max Reitz
2020-02-26 10:32 ` Pan Nengyuan
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).