All of lore.kernel.org
 help / color / mirror / Atom feed
* [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
* [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 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
* 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 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.