[PATCH v2 06/10] target/i386/cpu: Fix memleak in x86_cpu_class_check_missing_features

Pan Nengyuan posted 10 patches 5 years, 5 months ago
Maintainers: Hailiang Zhang <zhang.zhanghailiang@huawei.com>, Markus Armbruster <armbru@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Juan Quintela <quintela@redhat.com>, Michael Roth <mdroth@linux.vnet.ibm.com>, Gerd Hoffmann <kraxel@redhat.com>, Max Reitz <mreitz@redhat.com>, Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Viktor Prutyanov <viktor.prutyanov@phystech.edu>
[PATCH v2 06/10] target/i386/cpu: Fix memleak in x86_cpu_class_check_missing_features
Posted by Pan Nengyuan 5 years, 5 months ago
'err' forgot to free in x86_cpu_class_check_missing_features error path.
Fix that.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
---
- V2: no changes in v2.
---
 target/i386/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 588f32e136..4678aac0b4 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4872,6 +4872,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
         new->value = g_strdup("type");
         *next = new;
         next = &new->next;
+        error_free(err);
     }
 
     x86_cpu_filter_features(xc, false);
-- 
2.18.2


Re: [PATCH v2 06/10] target/i386/cpu: Fix memleak in x86_cpu_class_check_missing_features
Posted by Markus Armbruster 5 years, 5 months ago
Pan Nengyuan <pannengyuan@huawei.com> writes:

> 'err' forgot to free in x86_cpu_class_check_missing_features error path.
> Fix that.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> Reviewed-by: Li Qiang <liq3ea@gmail.com>
> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> ---
> - V2: no changes in v2.
> ---
>  target/i386/cpu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 588f32e136..4678aac0b4 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4872,6 +4872,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
       x86_cpu_expand_features(xc, &err);
       if (err) {
           /* Errors at x86_cpu_expand_features should never happen,
            * but in case it does, just report the model as not
            * runnable at all using the "type" property.
            */
           strList *new = g_new0(strList, 1);
>          new->value = g_strdup("type");
>          *next = new;
>          next = &new->next;
> +        error_free(err);
>      }
>  
>      x86_cpu_filter_features(xc, false);

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Recommended cleanup: change x86_cpu_filter_features() to return true on
success, false on failure, then pass NULL here and check the return
value.  Can be done on top.


Re: [PATCH v2 06/10] target/i386/cpu: Fix memleak in x86_cpu_class_check_missing_features
Posted by Pan Nengyuan 5 years, 5 months ago

On 2020/9/1 20:03, Markus Armbruster wrote:
> Pan Nengyuan <pannengyuan@huawei.com> writes:
> 
>> 'err' forgot to free in x86_cpu_class_check_missing_features error path.
>> Fix that.
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
>> Reviewed-by: Li Qiang <liq3ea@gmail.com>
>> ---
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Richard Henderson <rth@twiddle.net>
>> Cc: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>> - V2: no changes in v2.
>> ---
>>  target/i386/cpu.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 588f32e136..4678aac0b4 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -4872,6 +4872,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
>        x86_cpu_expand_features(xc, &err);
>        if (err) {
>            /* Errors at x86_cpu_expand_features should never happen,
>             * but in case it does, just report the model as not
>             * runnable at all using the "type" property.
>             */
>            strList *new = g_new0(strList, 1);
>>          new->value = g_strdup("type");
>>          *next = new;
>>          next = &new->next;
>> +        error_free(err);
>>      }
>>  
>>      x86_cpu_filter_features(xc, false);
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 
> Recommended cleanup: change x86_cpu_filter_features() to return true on
> success, false on failure, then pass NULL here and check the return
> value.  Can be done on top.
>
Agree with you, 'err' is not used, we can pass NULL here.
BTW, I think the func you mentioned shoule be x86_cpu_expand_features(), not x86_cpu_filter_features()?

Thanks.

> .
> 


Re: [PATCH v2 06/10] target/i386/cpu: Fix memleak in x86_cpu_class_check_missing_features
Posted by Eduardo Habkost 5 years, 5 months ago
On Mon, Aug 31, 2020 at 09:43:11AM -0400, Pan Nengyuan wrote:
> 'err' forgot to free in x86_cpu_class_check_missing_features error path.
> Fix that.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> Reviewed-by: Li Qiang <liq3ea@gmail.com>

Queued, thanks!

-- 
Eduardo