[Qemu-devel] test-qmp-commands reads freed memory

Peter Maydell

author Reviewed-by Eric Blake

2014-03-08 14:41 UTC #0 lore raw

The test-qmp-commands test binary seems to read from freed memory. This triggers the MacOSX malloc implementation's assertions. git bisect blames

commit c2216a8a7a587e594f50bebbdf81fcf168444b68

Author: Markus Armbruster <[email protected]>
Date:   Sat Mar 1 08:40:29 2014 +0100

tests/qapi-schema: Cover simple argument types

Signed-off-by Markus Armbruster <[email protected]>

Reviewed-by Eric Blake <[email protected]>

Signed-off-by Luiz Capitulino <[email protected]>

Valgrind will spot it:

cam-vm-266:precise:qemu$ valgrind build/x86/tests/test-qmp-commands ==15391== Memcheck, a memory error detector
==15391== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al. ==15391== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info ==15391== Command: build/x86/tests/test-qmp-commands
==15391==
/0.15/dispatch_cmd: OK
/0.15/dispatch_cmd_error: OK
/0.15/dispatch_cmd_io: ==15391== Invalid read of size 8

==15391==    at 0x1344F6: qobject_decref (qobject.h:97)
==15391==    by 0x134FFD: test_dispatch_cmd_io (test-qmp-commands.c:144)
==15391==    by 0x4E9A65A: ??? (in

/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x4E9A7D5: ??? (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x4E9AB2A: g_test_run_suite (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x13540D: main (test-qmp-commands.c:229)
==15391== Address 0x5ea26a8 is 8 bytes inside a block of size 4,120 free'd
==15391== at 0x4C2A82E: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==15391==    by 0x13B741: qdict_destroy_obj (qdict.c:477)
==15391==    by 0x134580: qobject_decref (qobject.h:100)
==15391==    by 0x134F41: test_dispatch_cmd_io (test-qmp-commands.c:136)
==15391==    by 0x4E9A65A: ??? (in

/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x4E9A7D5: ??? (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x4E9AB2A: g_test_run_suite (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x13540D: main (test-qmp-commands.c:229)
==15391==
==15391== Invalid write of size 8

==15391==    at 0x134502: qobject_decref (qobject.h:97)
==15391==    by 0x134FFD: test_dispatch_cmd_io (test-qmp-commands.c:144)
==15391==    by 0x4E9A65A: ??? (in

/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x4E9A7D5: ??? (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x4E9AB2A: g_test_run_suite (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x13540D: main (test-qmp-commands.c:229)
==15391== Address 0x5ea26a8 is 8 bytes inside a block of size 4,120 free'd
==15391== at 0x4C2A82E: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==15391==    by 0x13B741: qdict_destroy_obj (qdict.c:477)
==15391==    by 0x134580: qobject_decref (qobject.h:100)
==15391==    by 0x134F41: test_dispatch_cmd_io (test-qmp-commands.c:136)
==15391==    by 0x4E9A65A: ??? (in

/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x4E9A7D5: ??? (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x4E9AB2A: g_test_run_suite (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x13540D: main (test-qmp-commands.c:229)
==15391==
==15391== Invalid read of size 8

==15391==    at 0x13450A: qobject_decref (qobject.h:97)
==15391==    by 0x134FFD: test_dispatch_cmd_io (test-qmp-commands.c:144)
==15391==    by 0x4E9A65A: ??? (in

/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x4E9A7D5: ??? (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x4E9AB2A: g_test_run_suite (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x13540D: main (test-qmp-commands.c:229)
==15391== Address 0x5ea26a8 is 8 bytes inside a block of size 4,120 free'd
==15391== at 0x4C2A82E: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==15391==    by 0x13B741: qdict_destroy_obj (qdict.c:477)
==15391==    by 0x134580: qobject_decref (qobject.h:100)
==15391==    by 0x134F41: test_dispatch_cmd_io (test-qmp-commands.c:136)
==15391==    by 0x4E9A65A: ??? (in

/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x4E9A7D5: ??? (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x4E9AB2A: g_test_run_suite (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3200.4)
==15391== by 0x13540D: main (test-qmp-commands.c:229)
==15391==
OK

thanks
-- PMM

Luiz Capitulino

2014-03-08 16:09 UTC #1 lore raw

Quoted 3 lines · On Sat, 8 Mar 2014 14:40:27 +0000 Peter Maydell <[email protected]>…

The test-qmp-commands test binary seems to read from freed memory. This triggers the MacOSX malloc implementation's assertions. git bisect blames

Can you try the patch below? For the clang ones, I'll have to install it etc, so it will take a bit longer.

I wonder how this didn't explode...

tests/test-qmp-commands.c +1 -1

index 8e62c2d..554e222 100644
@@ -141,7 +141,7 @@ static void test_dispatch_cmd_io(void)
 
     ret3 = qobject_to_qint(test_qmp_dispatch(req));
     assert(qint_get_int(ret3) == 66);
-    QDECREF(ret);
+    QDECREF(ret3);
 
     QDECREF(req);
 }

Peter Maydell

author Tested-by

2014-03-08 16:42 UTC #2 lore raw

Quoted 17 lines · Luiz Capitulino

Quoted 3 lines · On Sat, 8 Mar 2014 14:40:27 +0000 Peter Maydell <[email protected]>…
The test-qmp-commands test binary seems to read from freed memory. This triggers the MacOSX malloc implementation's assertions. git bisect blames
Can you try the patch below? For the clang ones, I'll have to install it etc, so it will take a bit longer.
I wonder how this didn't explode...
tests/test-qmp-commands.c +0 -0
index 8e62c2d..554e222 100644

tests/test-qmp-commands.c static void test_dispatch_cmd_io(void) · line 147

Quoted 8 lines

tests/test-qmp-commands.c +1 -1

@@ -141,7 +141,7 @@ static void test_dispatch_cmd_io(void)
 
     ret3 = qobject_to_qint(test_qmp_dispatch(req));
     assert(qint_get_int(ret3) == 66);
-    QDECREF(ret);
+    QDECREF(ret3);
 
     QDECREF(req);
 }

Yep, seems to work (both MacOSX and valgrind are happier).

Tested-by Peter Maydell <[email protected]>

-- PMM

Luiz Capitulino

2014-03-08 17:27 UTC #3 lore raw

PATCH 1/1 tests: test-qmp-commands: Fix double free

The ret variable is freed twice, but on the second time we actually want to free ret3 instead. Don't know why this didn't explode.

Reported-by Peter Maydell <[email protected]>

Tested-by Peter Maydell <[email protected]>

Signed-off-by Luiz Capitulino <[email protected]>

---
 tests/test-qmp-commands.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tests/test-qmp-commands.c +1 -1

index 8e62c2d..554e222 100644
@@ -141,7 +141,7 @@ static void test_dispatch_cmd_io(void)
 
     ret3 = qobject_to_qint(test_qmp_dispatch(req));
     assert(qint_get_int(ret3) == 66);
-    QDECREF(ret);
+    QDECREF(ret3);
 
     QDECREF(req);
 }

Signature · 1 line

1.8.1.4

Eric Blake

Reviewed-by on v1 1/1

2014-03-10 15:50 UTC #4 lore raw

Re: [Qemu-devel] [PATCH] tests: test-qmp-commands: Fix double free

Quoted 10 lines · Luiz Capitulino

The ret variable is freed twice, but on the second time we actually want to free ret3 instead. Don't know why this didn't explode.
Reported-by Peter Maydell <[email protected]>
Tested-by Peter Maydell <[email protected]>
Signed-off-by Luiz Capitulino <[email protected]>
---
 tests/test-qmp-commands.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by Eric Blake <[email protected]>

Signature · 2 lines

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org