ctdb-recoverd: Avoid dereferencing NULL rec->nodemap

Inside the nested event loop in ctdb_ctrl_getnodemap(), various
asynchronous handlers may dereference rec->nodemap, which will be
NULL.

One example is lost_reclock_handler(), which causes rec->nodemap to be
unconditionally dereferenced in list_of_nodes() via this call chain:

  list_of_nodes()
  list_of_active_nodes()
  set_recovery_mode()
  force_election()
  lost_reclock_handler()

Instead of attempting to trace all of the cases, just avoid leaving
rec->nodemap set to NULL.  Attempting to use an old value is generally
harmless, especially since it will be the same as the new value in
most cases.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14324

Reported-by: Volker Lendecke <vl@samba.org>
Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>
Autobuild-User(master): Martin Schwenke <martins@samba.org>
Autobuild-Date(master): Tue Mar 24 01:22:45 UTC 2020 on sn-devel-184

diff --git a/ctdb/server/ctdb_recoverd.c b/ctdb/server/ctdb_recoverd.c
index 14b6e0fca26eabf34cadcf7b6f2d487564b4bd63..ee083e92fb110b04bb5313dc5ad50434999b13ab 100644 (file)
--- a/ctdb/server/ctdb_recoverd.c
+++ b/ctdb/server/ctdb_recoverd.c
@@ -2353,13 +2353,13 @@ static void main_loop(struct ctdb_context *ctdb, struct ctdb_recoverd *rec,
        pnn = ctdb_get_pnn(ctdb);
 
        /* get nodemap */
-       TALLOC_FREE(rec->nodemap);
-       ret = ctdb_ctrl_getnodemap(ctdb, CONTROL_TIMEOUT(), pnn, rec, &rec->nodemap);
+       ret = ctdb_ctrl_getnodemap(ctdb, CONTROL_TIMEOUT(), pnn, rec, &nodemap);
        if (ret != 0) {
-               DEBUG(DEBUG_ERR, (__location__ " Unable to get nodemap from node %u\n", pnn));
+               DBG_ERR("Unable to get nodemap from node %"PRIu32"\n", pnn);
                return;
        }
-       nodemap = rec->nodemap;
+       talloc_free(rec->nodemap);
+       rec->nodemap = nodemap;
 
        /* remember our own node flags */
        rec->node_flags = nodemap->nodes[pnn].flags;