diff mbox

hw/i386/acpi-build.c: Fix memory leak in acpi_build_tables_cleanup()

Message ID 1414591622-5620-1-git-send-email-zodiac@ispras.ru
State New
Headers show

Commit Message

Nikita Belov Oct. 29, 2014, 2:07 p.m. UTC
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>
---
 hw/i386/acpi-build.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

--
1.9.0.msysgit.0

Comments

Christian Borntraeger Oct. 30, 2014, 8:02 p.m. UTC | #1
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
>
diff mbox

Patch

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