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