Re: [Qemu-devel] [PATCH] hw/i386/acpi-build.c: Fix memory leak in acpi_build_tables_cleanup()

[Christian Borntraeger <borntraeger@de.ibm.com>]

Am 29.10.2014 15:07, schrieb Nikita Belov:
> There are three ACPI tables: 'linker_data', 'rsdp' and 'table_data'. They are
> used differently. Two of them are being copied before using and only the copy
> is used later. But the third is used directly. Because of that we need to free
> two tables completely and delete only wrapper for the third one.
> 
> Valgrind output:
> ==23931== 131,072 bytes in 1 blocks are definitely lost in loss record 7,729 of 7,734
> ==23931==    at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==23931==    by 0x2EA920: realloc_and_trace (vl.c:2811)
> ==23931==    by 0x509E6AE: g_realloc (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
> ==23931==    by 0x506DB32: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
> ==23931==    by 0x506E463: g_array_set_size (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
> ==23931==    by 0x256A4F: acpi_align_size (acpi-build.c:487)
> ==23931==    by 0x259F92: acpi_build (acpi-build.c:1601)
> ==23931==    by 0x25A212: acpi_setup (acpi-build.c:1682)
> ==23931==    by 0x24F346: pc_guest_info_machine_done (pc.c:1110)
> ==23931==    by 0x55FAAB: notifier_list_notify (notify.c:39)
> ==23931==    by 0x2EA704: qemu_run_machine_init_done_notifiers (vl.c:2759)
> ==23931==    by 0x2EEC3C: main (vl.c:4504)
> 
> Signed-off-by: Nikita Belov <zodiac@ispras.ru>

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

I found the same issue during the preparation of my KVM forum presentation and came to the same conclusion. 
CC Paolo (as x86 maintainer.)

> ---
>  hw/i386/acpi-build.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 00be4bb..c1778db 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1498,11 +1498,9 @@ static inline void acpi_build_tables_init(AcpiBuildTables *tables)
>  static inline void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre)
>  {
>      void *linker_data = bios_linker_loader_cleanup(tables->linker);
> -    if (mfre) {
> -        g_free(linker_data);
> -    }
> +    g_free(linker_data);
>      g_array_free(tables->rsdp, mfre);
> -    g_array_free(tables->table_data, mfre);
> +    g_array_free(tables->table_data, true);
>  }
> 
>  typedef
> --
> 1.9.0.msysgit.0
>