From ed8b2c86ab77aaa3d7fd688c049ad5e1b922a9c6 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Thu, 13 Jan 2022 02:56:55 -0800 Subject: [PATCH] Fix: liblrmd: Avoid double-free during notify operation This commit fixes a regression introduced by 31c7fa8a, causing a double-free in notify operations. lrmd_dispatch_internal() assigns the exit_reason string directly from an XML node to a new lrmd_event_data_t object (without duplicating), and this string gets freed twice. Free #1: pcmk__create_history_xml() (reached via callback) calls lrmd__set_result(), which frees event.exit_reason and sets it to NULL. Free #2: lrmd_ipc_dispatch() frees the XML node, which contains a pointer to the exit_reason string just freed, after lrmd_dispatch_internal() returns. Prior to 31c7fa8a, pcmk__create_history_xml reset event.rc and event.op_status but **not** event.exit_reason. In this commit we simply make a copy of event.exit_reason in lrmd_dispatch_internal() before the callback. This way we don't have to worry about whatever happens in the callback, and we can continue to unset the exit_reason alongside the rc and op_status. The added overhead should be minimal. This commit also makes a copy of output. That's not strictly necessary but adds some futureproofing and allows us to call lrmd__reset_result() at the end of lrmd_dispatch_internal(). Resolves: RHBZ#2039675 Signed-off-by: Reid Wahl --- lib/lrmd/lrmd_client.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/lrmd/lrmd_client.c b/lib/lrmd/lrmd_client.c index ee31bb5ae9..5131a648b7 100644 --- a/lib/lrmd/lrmd_client.c +++ b/lib/lrmd/lrmd_client.c @@ -305,9 +305,10 @@ lrmd_dispatch_internal(lrmd_t * lrmd, xmlNode * msg) event.user_data = crm_element_value(msg, F_LRMD_RSC_USERDATA_STR); event.type = lrmd_event_exec_complete; - // No need to duplicate the memory, so don't use setter functions - event.output = crm_element_value(msg, F_LRMD_RSC_OUTPUT); - event.exit_reason = crm_element_value(msg, F_LRMD_RSC_EXIT_REASON); + /* output and exit_reason may be freed by a callback */ + event.output = crm_element_value_copy(msg, F_LRMD_RSC_OUTPUT); + lrmd__set_result(&event, event.rc, event.op_status, + crm_element_value(msg, F_LRMD_RSC_EXIT_REASON)); event.params = xml2list(msg); } else if (pcmk__str_eq(type, LRMD_OP_NEW_CLIENT, pcmk__str_none)) { @@ -324,6 +325,7 @@ lrmd_dispatch_internal(lrmd_t * lrmd, xmlNode * msg) if (event.params) { g_hash_table_destroy(event.params); } + lrmd__reset_result(&event); } // \return Always 0, to indicate that IPC mainloop source should be kept -- 2.27.0