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