throttle-groups: fix memory leak in throttle_group_set_limits

Commit Message

Pan Nengyuan Nov. 26, 2019, 8:17 a.m. UTC

From: PanNengyuan <[e-mail]>

This avoid a memory leak when qom-set is called to set throttle_group
limits, here is an easy way to reproduce:

1. run qemu-iotests as follow and check the result with asan:
       ./check -qcow2 184

Following is the asan output backtrack:
Direct leak of 912 byte(s) in 3 object(s) allocated from:
    #0 0xffff8d7ab3c3 in __interceptor_calloc    (/lib64/libasan.so.4+0xd33c3)
    #1 0xffff8d4c31cb in g_malloc0 (/lib64/libglib-2.0.so.0+0x571cb)
    #2 0x190c857 in qobject_input_start_struct  /mnt/sdc/qemu-master/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:295
    #3 0x19070df in visit_start_struct /mnt/sdc/qemu-master/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:49
    #4 0x1948b87 in visit_type_ThrottleLimits  qapi/qapi-visit-block-core.c:3759
    #5 0x17e4aa3 in throttle_group_set_limits  /mnt/sdc/qemu-master/qemu-4.2.0-rc0/block/throttle-groups.c:900
    #6 0x1650eff in object_property_set  /mnt/sdc/qemu-master/qemu-4.2.0-rc0/qom/object.c:1272
    #7 0x1658517 in object_property_set_qobject  /mnt/sdc/qemu-master/qemu-4.2.0-rc0/qom/qom-qobject.c:26
    #8 0x15880bb in qmp_qom_set /mnt/sdc/qemu-master/qemu-4.2.0-rc0/qom/qom-qmp-cmds.c:74
    #9 0x157e3e3 in qmp_marshal_qom_set  qapi/qapi-commands-qom.c:154

Reported-by: Euler Robot <[e-mail]>
Signed-off-by: PanNengyuan <[e-mail]>
---
 block/throttle-groups.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Alberto Garcia Nov. 26, 2019, 9:59 a.m. UTC | #1

On Tue 26 Nov 2019 09:17:02 AM CET, [e-mail] wrote:
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -912,6 +912,7 @@ static void throttle_group_set_limits(Object *obj, Visitor *v,
>  unlock:
>      qemu_mutex_unlock(&tg->lock);
>  ret:
> +    qapi_free_ThrottleLimits(argp);
>      error_propagate(errp, local_err);
>      return;

Thanks, but I also think that 'arg' is not used so it can be removed?

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 77014c741b..37695b0cd7 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -893,8 +893,7 @@ static void throttle_group_set_limits(Object *obj, Visitor *v,
 {
     ThrottleGroup *tg = THROTTLE_GROUP(obj);
     ThrottleConfig cfg;
-    ThrottleLimits arg = { 0 };
-    ThrottleLimits *argp = &arg;
+    ThrottleLimits *argp;
     Error *local_err = NULL;
 
     visit_type_ThrottleLimits(v, name, &argp, &local_err);
@@ -912,6 +911,7 @@ static void throttle_group_set_limits(Object *obj, Visitor *v,
 unlock:
     qemu_mutex_unlock(&tg->lock);
 ret:
+    qapi_free_ThrottleLimits(argp);
     error_propagate(errp, local_err);
     return;
 }

Berto

Pan Nengyuan Nov. 27, 2019, 2:47 a.m. UTC | #2

Thanks, I think it can be removed, I will send a new version later.

On 2019/11/26 17:59, Alberto Garcia wrote:
> On Tue 26 Nov 2019 09:17:02 AM CET, [e-mail] wrote:
>> --- a/block/throttle-groups.c
>> +++ b/block/throttle-groups.c
>> @@ -912,6 +912,7 @@ static void throttle_group_set_limits(Object *obj, Visitor *v,
>>  unlock:
>>      qemu_mutex_unlock(&tg->lock);
>>  ret:
>> +    qapi_free_ThrottleLimits(argp);
>>      error_propagate(errp, local_err);
>>      return;
> 
> Thanks, but I also think that 'arg' is not used so it can be removed?
> 
> diff --git a/block/throttle-groups.c b/block/throttle-groups.c
> index 77014c741b..37695b0cd7 100644
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -893,8 +893,7 @@ static void throttle_group_set_limits(Object *obj, Visitor *v,
>  {
>      ThrottleGroup *tg = THROTTLE_GROUP(obj);
>      ThrottleConfig cfg;
> -    ThrottleLimits arg = { 0 };
> -    ThrottleLimits *argp = &arg;
> +    ThrottleLimits *argp;
>      Error *local_err = NULL;
>  
>      visit_type_ThrottleLimits(v, name, &argp, &local_err);
> @@ -912,6 +911,7 @@ static void throttle_group_set_limits(Object *obj, Visitor *v,
>  unlock:
>      qemu_mutex_unlock(&tg->lock);
>  ret:
> +    qapi_free_ThrottleLimits(argp);
>      error_propagate(errp, local_err);
>      return;
>  }
> 
> Berto
> 
> .
>

Patch

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 77014c7..88418e6 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -912,6 +912,7 @@  static void throttle_group_set_limits(Object *obj, Visitor *v,
 unlock:
     qemu_mutex_unlock(&tg->lock);
 ret:
+    qapi_free_ThrottleLimits(argp);
     error_propagate(errp, local_err);
     return;
 }