[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] json-streamer: Don't leak tokens on incomplete
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] json-streamer: Don't leak tokens on incomplete parse |
Date: |
Mon, 04 Jul 2016 14:21:08 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Changlong Xie <address@hidden> writes:
> On 05/19/2016 05:46 AM, Eric Blake wrote:
>> Valgrind complained about a number of leaks in
>> tests/check-qobject-json:
>>
>> ==12657== definitely lost: 17,247 bytes in 1,234 blocks
>>
>> All of which had the same root cause: on an incomplete parse,
>> we were abandoning the token queue without cleaning up the
>> allocated data within each queue element. Introduced in
>> commit 95385fe, when we switched from QList (which recursively
>> frees contents) to g_queue (which does not).
>>
>> We don't yet require glib 2.32 with its g_queue_free_full(),
>> so open-code it instead.
>>
>> CC: address@hidden
>> Signed-off-by: Eric Blake <address@hidden>
>> ---
>> qobject/json-streamer.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c
>> index 0251685..7164390 100644
>> --- a/qobject/json-streamer.c
>> +++ b/qobject/json-streamer.c
>> @@ -20,9 +20,15 @@
>> #define MAX_TOKEN_COUNT (2ULL << 20)
>> #define MAX_NESTING (1ULL << 10)
>>
>> +static void json_message_free_token(void *token, void *opaque)
>> +{
>> + g_free(token);
>> +}
>> +
>> static void json_message_free_tokens(JSONMessageParser *parser)
>> {
>> if (parser->tokens) {
>> + g_queue_foreach(parser->tokens, json_message_free_token, NULL);
>> g_queue_free(parser->tokens);
>> parser->tokens = NULL;
>> }
>>
>
> It seems this commit make tests/qemu-iotests/071 failed on the last
> master branch.
More direct reproducer:
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
"options": {
"node-name": "drive0",
"driver": "qcow2",
"file": {
"driver": "file",
"filename": "t.qcow2"
}
}
}
}
{ "execute": "blockdev-add",
"arguments": {
"options": {
"driver": "blkverify",
"id": "drive0-verify",
"test": "drive0",
"raw": {
"driver": "file",
"filename": "t.qcow2.base"
}
}
}
}
{ "execute": "human-monitor-command",
"arguments": {
"command-line": 'qemu-io drive0-verify "read 0 512"'
}
}
With t.qcow2 and t.qcow2.base as in 071.
Interesting part of valgrind run:
==29716== at 0xF7E8E71: g_queue_foreach (in
/usr/lib64/libglib-2.0.so.0.4600.2)
==29716== by 0x7CB7C5: json_message_free_tokens (json-streamer.c:31)
==29716== by 0x7CBAC6: json_message_parser_destroy (json-streamer.c:131)
==29716== by 0x3469AE: monitor_qmp_event (monitor.c:4022)
==29716== by 0x476C32: qemu_chr_be_event (qemu-char.c:205)
==29716== by 0x47BE05: tcp_chr_close (qemu-char.c:3175)
==29716== by 0x47E5AB: qemu_chr_free (qemu-char.c:4036)
==29716== by 0x47E62C: qemu_chr_delete (qemu-char.c:4044)
==29716== by 0x47F576: qemu_chr_cleanup (qemu-char.c:4557)
==29716== by 0x12EA65E7: __run_exit_handlers (in /usr/lib64/libc-2.22.so)
==29716== by 0x12EA6634: exit (in /usr/lib64/libc-2.22.so)
==29716== by 0x74762C: blkverify_err (blkverify.c:58)
==29716== Address 0x24270550 is 0 bytes inside a block of size 24 free'd
==29716== at 0x4C29CF0: free (vg_replace_malloc.c:530)
==29716== by 0xF7DE63D: g_free (in /usr/lib64/libglib-2.0.so.0.4600.2)
==29716== by 0xF7F5DCC: g_slice_free1 (in
/usr/lib64/libglib-2.0.so.0.4600.2)
==29716== by 0x7CC327: parser_context_free (json-parser.c:268)
==29716== by 0x7CCFB5: json_parser_parse_err (json-parser.c:577)
==29716== by 0x7CCF48: json_parser_parse (json-parser.c:561)
==29716== by 0x3464A2: handle_qmp_command (monitor.c:3892)
==29716== by 0x7CB9C7: json_message_process_token (json-streamer.c:100)
==29716== by 0x7EDEC2: json_lexer_feed_char (json-lexer.c:319)
==29716== by 0x7EE00A: json_lexer_feed (json-lexer.c:369)
==29716== by 0x7CBA7E: json_message_parser_feed (json-streamer.c:120)
==29716== by 0x346722: monitor_qmp_read (monitor.c:3949)
==29716== Block was alloc'd at
==29716== at 0x4C28BF6: malloc (vg_replace_malloc.c:299)
==29716== by 0xF7DE528: g_malloc (in /usr/lib64/libglib-2.0.so.0.4600.2)
==29716== by 0xF7F5652: g_slice_alloc (in
/usr/lib64/libglib-2.0.so.0.4600.2)
==29716== by 0xF7F5CED: g_slice_alloc0 (in
/usr/lib64/libglib-2.0.so.0.4600.2)
==29716== by 0x7CB9CC: json_message_process_token (json-streamer.c:101)
==29716== by 0x7EDEC2: json_lexer_feed_char (json-lexer.c:319)
==29716== by 0x7EE00A: json_lexer_feed (json-lexer.c:369)
==29716== by 0x7CBA7E: json_message_parser_feed (json-streamer.c:120)
==29716== by 0x346722: monitor_qmp_read (monitor.c:3949)
==29716== by 0x477214: qemu_chr_be_write_impl (qemu-char.c:388)
==29716== by 0x477272: qemu_chr_be_write (qemu-char.c:400)
==29716== by 0x47B4CD: tcp_chr_read (qemu-char.c:2894)
Double free. Can't see offhand how this stuff works. Eric, let's
revert this patch unless you can see a fix.