pacemaker/0002-Fix-fencer-avoid-use-after-free-with-self-fencing-an.patch
Klaus Wenninger 8917fba90b * Tue Jun 7 2022 Klaus Wenninger <kwenning@redhat.com> - 2.1.3-3
- Update for new upstream release tarball: Pacemaker-2.1.3
  for full details, see included ChangeLog file or
  https://github.com/ClusterLabs/pacemaker/releases/tag/Pacemaker-2.1.3
- get target-by-attribute working again
- avoid use-after-free with self-fencing and topology
2022-06-08 11:09:19 +02:00

95 lines
3.4 KiB
Diff

From 1ab6a17d1272968a2d465acbf1e62af35344ce32 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 3 Jun 2022 11:19:04 -0500
Subject: [PATCH] Fix: fencer: avoid use-after-free with self-fencing and
topology
In the case of self-fencing with topology, handle_fence_request() will
overwrite F_STONITH_OPERATION in the original request XML, which invalidates
the request.op pointer created by stonith_command(). The fix is to make
request.op a copy.
Regression introduced in 2.1.3 by 067d655eb
---
daemons/fenced/fenced_commands.c | 4 ++--
include/crm/common/messages_internal.h | 8 ++++++--
lib/common/messages.c | 15 +++++++++++++++
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c
index a43a88f..94aa6b8 100644
--- a/daemons/fenced/fenced_commands.c
+++ b/daemons/fenced/fenced_commands.c
@@ -3498,7 +3498,7 @@ stonith_command(pcmk__client_t *client, uint32_t id, uint32_t flags,
.result = PCMK__UNKNOWN_RESULT,
};
- request.op = crm_element_value(request.xml, F_STONITH_OPERATION);
+ request.op = crm_element_value_copy(request.xml, F_STONITH_OPERATION);
CRM_CHECK(request.op != NULL, return);
if (pcmk_is_set(request.call_options, st_opt_sync_call)) {
@@ -3506,6 +3506,6 @@ stonith_command(pcmk__client_t *client, uint32_t id, uint32_t flags,
}
handle_request(&request);
- pcmk__reset_result(&request.result);
+ pcmk__reset_request(&request);
}
}
diff --git a/include/crm/common/messages_internal.h b/include/crm/common/messages_internal.h
index edbd836..2ba5bd9 100644
--- a/include/crm/common/messages_internal.h
+++ b/include/crm/common/messages_internal.h
@@ -50,11 +50,14 @@ typedef struct {
* generically, but each daemon uses a different XML attribute for it,
* so the daemon is responsible for populating this field.
*
+ * This must be a copy of the XML field, and not just a pointer into xml,
+ * because handlers might modify the original XML.
+ *
* @TODO Create a per-daemon struct with IPC handlers, IPC endpoints, etc.,
* and the name of the XML attribute for IPC commands, then replace this
- * with a convenience function to grab the command.
+ * with a convenience function to copy the command.
*/
- const char *op; // IPC command from xml
+ char *op; // IPC command name
} pcmk__request_t;
#define pcmk__set_request_flags(request, flags_to_set) do { \
@@ -72,6 +75,7 @@ typedef struct {
const char *pcmk__message_name(const char *name);
GHashTable *pcmk__register_handlers(pcmk__server_command_t handlers[]);
xmlNode *pcmk__process_request(pcmk__request_t *request, GHashTable *handlers);
+void pcmk__reset_request(pcmk__request_t *request);
/*!
* \internal
diff --git a/lib/common/messages.c b/lib/common/messages.c
index 4f8777d..1c5f467 100644
--- a/lib/common/messages.c
+++ b/lib/common/messages.c
@@ -276,3 +276,18 @@ pcmk__process_request(pcmk__request_t *request, GHashTable *handlers)
return (*handler)(request);
}
+
+/*!
+ * \internal
+ * \brief Free memory used within a request (but not the request itself)
+ *
+ * \param[in] request Request to reset
+ */
+void
+pcmk__reset_request(pcmk__request_t *request)
+{
+ free(request->op);
+ request->op = NULL;
+
+ pcmk__reset_result(&(request->result));
+}
--
1.8.3.1