File pacemaker-crmd-use-after-free-disconnecting-from-CIB.patch of Package pacemaker.14737
commit 360cf350722f9646581e47b4d73d971aa81e6cec
Author: Ken Gaillot <kgaillot@redhat.com>
Date: Fri Apr 21 11:31:01 2017 -0500
Low: crmd: avoid use-after-free when disconnecting from CIB
crmd_exit() freed the CIB connection, then drained the mainloop.
However, the mainloop could call functions (such as do_cib_control() or
cib_native_destroy()) that would use the connection object.
This would lead to a segfault, though the harm was minimal, since the crmd was
already exiting at this point.
Also log a notice comparable to what's done for the crmd's other disconnects.
diff --git a/crmd/cib.c b/crmd/cib.c
index 40fed4a98..41f4f3d6e 100644
--- a/crmd/cib.c
+++ b/crmd/cib.c
@@ -159,19 +159,16 @@ do_cib_replaced(const char *event, xmlNode * msg)
register_fsa_input(C_FSA_INTERNAL, I_ELECTION, NULL);
}
-/* A_CIB_STOP, A_CIB_START, A_CIB_RESTART, */
+/* A_CIB_STOP, A_CIB_START, O_CIB_RESTART */
void
do_cib_control(long long action,
enum crmd_fsa_cause cause,
enum crmd_fsa_state cur_state,
enum crmd_fsa_input current_input, fsa_data_t * msg_data)
{
- struct crm_subsystem_s *this_subsys = cib_subsystem;
+ CRM_ASSERT(fsa_cib_conn != NULL);
- long long stop_actions = A_CIB_STOP;
- long long start_actions = A_CIB_START;
-
- if (action & stop_actions) {
+ if (action & A_CIB_STOP) {
if (fsa_cib_conn->state != cib_disconnected && last_resource_update != 0) {
crm_info("Waiting for resource update %d to complete", last_resource_update);
@@ -181,7 +178,6 @@ do_cib_control(long long action,
crm_info("Disconnecting CIB");
clear_bit(fsa_input_register, R_CIB_CONNECTED);
- CRM_ASSERT(fsa_cib_conn != NULL);
fsa_cib_conn->cmds->del_notify_callback(fsa_cib_conn, T_CIB_DIFF_NOTIFY, do_cib_updated);
@@ -189,15 +185,14 @@ do_cib_control(long long action,
fsa_cib_conn->cmds->set_slave(fsa_cib_conn, cib_scope_local);
fsa_cib_conn->cmds->signoff(fsa_cib_conn);
}
+ crm_notice("Disconnected from the CIB");
}
- if (action & start_actions) {
+ if (action & A_CIB_START) {
int rc = pcmk_ok;
- CRM_ASSERT(fsa_cib_conn != NULL);
-
if (cur_state == S_STOPPING) {
- crm_err("Ignoring request to start %s after shutdown", this_subsys->name);
+ crm_err("Ignoring request to start the CIB after shutdown");
return;
}
diff --git a/crmd/control.c b/crmd/control.c
index af9c2c2f2..a9c0b731a 100644
--- a/crmd/control.c
+++ b/crmd/control.c
@@ -355,8 +355,11 @@ crmd_exit(int rc)
election_fini(fsa_election);
fsa_election = NULL;
- cib_delete(fsa_cib_conn);
- fsa_cib_conn = NULL;
+ /* Tear down the CIB connection, but don't free it yet -- it could be used
+ * when we drain the mainloop later.
+ */
+ cib_free_callbacks(fsa_cib_conn);
+ fsa_cib_conn->cmds->signoff(fsa_cib_conn);
verify_stopped(fsa_state, LOG_WARNING);
clear_bit(fsa_input_register, R_LRM_CONNECTED);
@@ -485,6 +488,9 @@ crmd_exit(int rc)
mainloop_destroy_signal(SIGCHLD);
}
+ cib_delete(fsa_cib_conn);
+ fsa_cib_conn = NULL;
+
/* Graceful */
return rc;
}
diff --git a/include/crm/cib.h b/include/crm/cib.h
index a1246d1b7..ec5260289 100644
--- a/include/crm/cib.h
+++ b/include/crm/cib.h
@@ -172,6 +172,7 @@ cib_t *cib_new_no_shadow(void);
char *get_shadow_file(const char *name);
cib_t *cib_shadow_new(const char *name);
+void cib_free_callbacks(cib_t *cib);
void cib_delete(cib_t * cib);
void cib_dump_pending_callbacks(void);
diff --git a/lib/cib/cib_client.c b/lib/cib/cib_client.c
index 0f533309f..907bb5aef 100644
--- a/lib/cib/cib_client.c
+++ b/lib/cib/cib_client.c
@@ -406,24 +406,37 @@ cib_new_variant(void)
return new_cib;
}
+/*!
+ * \brief Free all callbacks for a CIB connection
+ *
+ * \param[in] cib CIB connection to clean up
+ */
void
-cib_delete(cib_t * cib)
+cib_free_callbacks(cib_t *cib)
{
- GList *list = NULL;
- if(cib) {
- list = cib->notify_list;
- }
+ if (cib) {
+ GList *list = cib->notify_list;
- while (list != NULL) {
- cib_notify_client_t *client = g_list_nth_data(list, 0);
+ while (list != NULL) {
+ cib_notify_client_t *client = g_list_nth_data(list, 0);
- list = g_list_remove(list, client);
- free(client);
+ list = g_list_remove(list, client);
+ free(client);
+ }
}
-
destroy_op_callback_table();
+}
- if(cib) {
+/*!
+ * \brief Free all memory used by CIB connection
+ *
+ * \param[in] cib CIB connection to delete
+ */
+void
+cib_delete(cib_t *cib)
+{
+ cib_free_callbacks(cib);
+ if (cib) {
cib->cmds->free(cib);
}
}