From fbb55b66023677dd6196aa19bc95e0d0bcc35616 Mon Sep 17 00:00:00 2001 From: Amitay Isaacs Date: Mon, 27 Jul 2015 16:51:08 +1000 Subject: [PATCH 1/2] ctdb-banning: If node is already banned, do not run ctdb_local_node_got_banned() This calls release_all_ips() only once on the first ban. If the node gets banned again due to event script timeout while running release_all_ips(), then avoid calling release_all_ips() in re-entrant fashion. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11432 Signed-off-by: Amitay Isaacs Reviewed-by: Martin Schwenke (cherry picked from commit 8eb04d09b119e234c88150e1dc35fc5057f9c926) --- ctdb/server/ctdb_banning.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ctdb/server/ctdb_banning.c b/ctdb/server/ctdb_banning.c index a9d1891..d8f7ab1 100644 --- a/ctdb/server/ctdb_banning.c +++ b/ctdb/server/ctdb_banning.c @@ -80,6 +80,7 @@ void ctdb_local_node_got_banned(struct ctdb_context *ctdb) int32_t ctdb_control_set_ban_state(struct ctdb_context *ctdb, TDB_DATA indata) { struct ctdb_ban_time *bantime = (struct ctdb_ban_time *)indata.dptr; + bool already_banned; DEBUG(DEBUG_INFO,("SET BAN STATE\n")); @@ -107,9 +108,11 @@ int32_t ctdb_control_set_ban_state(struct ctdb_context *ctdb, TDB_DATA indata) return 0; } + already_banned = false; if (ctdb->banning_ctx != NULL) { talloc_free(ctdb->banning_ctx); ctdb->banning_ctx = NULL; + already_banned = true; } if (bantime->time == 0) { @@ -136,7 +139,9 @@ int32_t ctdb_control_set_ban_state(struct ctdb_context *ctdb, TDB_DATA indata) event_add_timed(ctdb->ev, ctdb->banning_ctx, timeval_current_ofs(bantime->time,0), ctdb_ban_node_event, ctdb); - ctdb_local_node_got_banned(ctdb); + if (!already_banned) { + ctdb_local_node_got_banned(ctdb); + } return 0; } -- 2.4.6 From a3a2d84540eac9137fd8894bf52c2f1aba765fdd Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Fri, 24 Jul 2015 15:32:42 +1000 Subject: [PATCH 2/2] ctdb-daemon: Check if updates are in flight when releasing all IPs Some code involved in releasing IPs is not re-entrant. Memory corruption can occur if, for example, overlapping attempts are made to ban a node. We haven't been able to recreate the corruption but this should protect against it. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11432 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit 952a50485f68b3cffdf57da84aa9bb9fde630b7e) --- ctdb/server/ctdb_takeover.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index d5d2b39..efc80b1 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -3128,9 +3128,6 @@ void ctdb_takeover_client_destructor_hook(struct ctdb_client *client) } -/* - release all IPs on shutdown - */ void ctdb_release_all_ips(struct ctdb_context *ctdb) { struct ctdb_vnn *vnn; @@ -3149,6 +3146,20 @@ void ctdb_release_all_ips(struct ctdb_context *ctdb) continue; } + /* Don't allow multiple releases at once. Some code, + * particularly ctdb_tickle_sentenced_connections() is + * not re-entrant */ + if (vnn->update_in_flight) { + DEBUG(DEBUG_WARNING, + (__location__ + " Not releasing IP %s/%u on interface %s, an update is already in progess\n", + ctdb_addr_to_str(&vnn->public_address), + vnn->public_netmask_bits, + ctdb_vnn_iface_string(vnn))); + continue; + } + vnn->update_in_flight = true; + DEBUG(DEBUG_INFO,("Release of IP %s/%u on interface %s node:-1\n", ctdb_addr_to_str(&vnn->public_address), vnn->public_netmask_bits, @@ -3160,6 +3171,7 @@ void ctdb_release_all_ips(struct ctdb_context *ctdb) vnn->public_netmask_bits); release_kill_clients(ctdb, &vnn->public_address); ctdb_vnn_unassign_iface(ctdb, vnn); + vnn->update_in_flight = false; count++; } -- 2.4.6