From 50c7a272441f32aff2c8f1a98afa3e5b451d0e99 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Mon, 7 Dec 2020 09:32:27 +0100 Subject: [PATCH] qemu_monitor_json: Don't leak "option" in qemuMonitorJSONGetCommandLineOptions() In recent commit of bf8bd93df0 (and friends) we switched the way we process queried command line arguments: from string lists to virJSONValue stored in a hash table. To achieve this qemuMonitorJSONGetCommandLineOptions() helper was introduced which executes the "query-command-line-options" monitor command and then calls virJSONValueArrayForeachSteal() to process the output. The array process function is also given qemuMonitorJSONGetCommandLineOptionsWorker() as the callback which is called over each item of the returned array. This callback then steals "parameters" attribute of each array iteam storing it in the hash table, but it leaves behind "option" attribute (because it's g_strdup()-ed). After all of this, the callback returns 0 which is a signal to the array processing function that the callback took ownership of the array item. But this is not true. While it removed "parameters" it did not take the rest ("option" for instance). And therefore, it leads to a memory leak: 5,347 (1,656 direct, 3,691 indirect) bytes in 69 blocks are definitely lost in loss record 2,752 of 2,794 at 0x483BEC5: calloc (vg_replace_malloc.c:760) by 0x4E25A10: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6400.5) by 0x4943317: virJSONValueNewObject (virjson.c:569) by 0x4945692: virJSONParserHandleStartMap (virjson.c:1768) by 0x5825A86: yajl_do_parse (in /usr/lib64/libyajl.so.2.1.0) by 0x4945BFA: virJSONValueFromString (virjson.c:1896) by 0xAF5C115: qemuMonitorJSONIOProcessLine (qemu_monitor_json.c:224) by 0xAF5C45E: qemuMonitorJSONIOProcess (qemu_monitor_json.c:279) by 0xAF4BB6C: qemuMonitorIOProcess (qemu_monitor.c:342) by 0xAF4C444: qemuMonitorIO (qemu_monitor.c:574) by 0x4FEF846: socket_source_dispatch (in /usr/lib64/libgio-2.0.so.0.6400.5) by 0x4E1F727: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.6400.5) The callback must return 1 so that the array item is properly freed. Fixes: ebeff6cd57d07c89d42e191ed0085a9dd89835c5 Signed-off-by: Michal Privoznik Reviewed-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 4db00e284a..8480338f46 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6357,7 +6357,7 @@ qemuMonitorJSONGetCommandLineOptionsWorker(size_t pos G_GNUC_UNUSED, g_hash_table_insert(options, g_strdup(name), parameters); parameters = NULL; - return 0; + return 1; } -- 2.47.3