File pacemaker-crmd-use-after-free-disconnecting-fro... of Package pacemaker.8397

Overview Repositories Revisions Requests Users Attributes Meta

File pacemaker-crmd-use-after-free-disconnecting-from-CIB.patch of Package pacemaker.8397

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);
     }
 }

Places

File pacemaker-crmd-use-after-free-disconnecting-from-CIB.patch of Package pacemaker.8397

Places