From adf40341a7a818336939631b0b8d399699013744 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Sun, 11 May 2008 14:28:33 +1000 Subject: [PATCH] ctdb->methods becomes NULL when we shutdown the transport. If we shutdown the transport and CTDB later decides to send a command out for queueing, the call to ctdb->methods->allocate_pkt() will SEGV. This could trigger for example when we are in the process of shuttind down CTDBD and have already shutdown the transport but we are still waiting for the "shutdown" eventscripts to finish. If the event scripts now take much much longer to execute for some reason, this race condition becomes much more probable. Decorate all dereferencing of ctdb->methods-> with a check that ctdb->menthods is non-NULL (This used to be ctdb commit c4c2c53918da6fb566d6e9cbd6b02e61ae2921e7) --- ctdb/server/ctdb_control.c | 4 +++- ctdb/server/ctdb_daemon.c | 16 ++++++++++++++++ ctdb/server/ctdb_recover.c | 9 ++++++++- ctdb/server/ctdb_recoverd.c | 8 ++++++-- ctdb/server/ctdb_server.c | 10 ++++++++++ 5 files changed, 43 insertions(+), 4 deletions(-) diff --git a/ctdb/server/ctdb_control.c b/ctdb/server/ctdb_control.c index 6c8a4fc1346..6a2a9ce6c94 100644 --- a/ctdb/server/ctdb_control.c +++ b/ctdb/server/ctdb_control.c @@ -276,7 +276,9 @@ static int32_t ctdb_control_dispatch(struct ctdb_context *ctdb, ctdb_stop_keepalive(ctdb); ctdb_stop_monitoring(ctdb); ctdb_release_all_ips(ctdb); - ctdb->methods->shutdown(ctdb); + if (ctdb->methods != NULL) { + ctdb->methods->shutdown(ctdb); + } ctdb_event_script(ctdb, "shutdown"); DEBUG(DEBUG_NOTICE,("Received SHUTDOWN command. Stopping CTDB daemon.\n")); exit(0); diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c index 93af56c1762..326ab60e0d4 100644 --- a/ctdb/server/ctdb_daemon.c +++ b/ctdb/server/ctdb_daemon.c @@ -71,6 +71,11 @@ static void print_exit_message(void) /* called when the "startup" event script has finished */ static void ctdb_start_transport(struct ctdb_context *ctdb) { + if (ctdb->methods == NULL) { + DEBUG(DEBUG_ALERT,(__location__ " startup event finished but transport is DOWN.\n")); + ctdb_fatal(ctdb, "transport is not initialized but startup completed"); + } + /* start the transport running */ if (ctdb->methods->start(ctdb) != 0) { DEBUG(DEBUG_ALERT,("transport failed to start!\n")); @@ -689,6 +694,11 @@ int ctdb_start_daemon(struct ctdb_context *ctdb, bool do_fork) return -1; } + if (ctdb->methods == NULL) { + DEBUG(DEBUG_ALERT,(__location__ " Can not initialize transport. ctdb->methods is NULL\n")); + ctdb_fatal(ctdb, "transport is unavailable. can not initialize."); + } + /* initialise the transport */ if (ctdb->methods->initialise(ctdb) != 0) { ctdb_fatal(ctdb, "transport failed to initialise"); @@ -743,6 +753,12 @@ struct ctdb_req_header *_ctdb_transport_allocate(struct ctdb_context *ctdb, length = MAX(length, slength); size = (length+(CTDB_DS_ALIGNMENT-1)) & ~(CTDB_DS_ALIGNMENT-1); + if (ctdb->methods == NULL) { + DEBUG(DEBUG_ERR,(__location__ " Unable to allocate transport packet for operation %u of length %u. Transport is DOWN.\n", + operation, (unsigned)length)); + return NULL; + } + hdr = (struct ctdb_req_header *)ctdb->methods->allocate_pkt(mem_ctx, size); if (hdr == NULL) { DEBUG(DEBUG_ERR,("Unable to allocate transport packet for operation %u of length %u\n", diff --git a/ctdb/server/ctdb_recover.c b/ctdb/server/ctdb_recover.c index 7a96733e92f..3da3b56f2c7 100644 --- a/ctdb/server/ctdb_recover.c +++ b/ctdb/server/ctdb_recover.c @@ -180,7 +180,9 @@ ctdb_reload_nodes_event(struct event_context *ev, struct timed_event *te, int ctdb_tcp_init(struct ctdb_context *); /* shut down the transport */ - ctdb->methods->shutdown(ctdb); + if (ctdb->methods != NULL) { + ctdb->methods->shutdown(ctdb); + } /* start the transport again */ ctdb_load_nodes_file(ctdb); @@ -189,6 +191,11 @@ ctdb_reload_nodes_event(struct event_context *ev, struct timed_event *te, DEBUG(DEBUG_CRIT, (__location__ " Failed to init TCP\n")); exit(1); } + + if (ctdb->methods == NULL) { + DEBUG(DEBUG_ALERT,(__location__ " Can not restart transport. ctdb->methods==NULL\n")); + ctdb_fatal(ctdb, "can not reinitialize transport."); + } ctdb->methods->initialise(ctdb); ctdb->methods->start(ctdb); diff --git a/ctdb/server/ctdb_recoverd.c b/ctdb/server/ctdb_recoverd.c index 95dd94f4b68..7aca7cb4dd5 100644 --- a/ctdb/server/ctdb_recoverd.c +++ b/ctdb/server/ctdb_recoverd.c @@ -2790,7 +2790,9 @@ static void ctdb_check_recd(struct event_context *ev, struct timed_event *te, ctdb_stop_keepalive(ctdb); ctdb_stop_monitoring(ctdb); ctdb_release_all_ips(ctdb); - ctdb->methods->shutdown(ctdb); + if (ctdb->methods != NULL) { + ctdb->methods->shutdown(ctdb); + } ctdb_event_script(ctdb, "shutdown"); exit(10); @@ -2831,7 +2833,9 @@ int ctdb_start_recoverd(struct ctdb_context *ctdb) close(fd[1]); /* shutdown the transport */ - ctdb->methods->shutdown(ctdb); + if (ctdb->methods) { + ctdb->methods->shutdown(ctdb); + } /* get a new event context */ talloc_free(ctdb->ev); diff --git a/ctdb/server/ctdb_server.c b/ctdb/server/ctdb_server.c index 546d6185a58..a4c09a9a4e8 100644 --- a/ctdb/server/ctdb_server.c +++ b/ctdb/server/ctdb_server.c @@ -345,6 +345,11 @@ void ctdb_node_dead(struct ctdb_node *node) node->ctdb->name, node->name, node->ctdb->num_connected)); ctdb_daemon_cancel_controls(node->ctdb, node); + if (node->ctdb->methods == NULL) { + DEBUG(DEBUG_ALERT,(__location__ " Can not restart transport. ctdb->methods==NULL\n")); + ctdb_fatal(node->ctdb, "can not restart transport."); + } + node->ctdb->methods->restart(node); } @@ -484,6 +489,11 @@ void ctdb_queue_packet(struct ctdb_context *ctdb, struct ctdb_req_header *hdr) if (hdr->destnode == ctdb->pnn) { ctdb_defer_packet(ctdb, hdr); } else { + if (ctdb->methods == NULL) { + DEBUG(DEBUG_ALERT, (__location__ " Can not queue packet. Transport is DOWN\n")); + return; + } + node->tx_cnt++; if (ctdb->methods->queue_pkt(node, (uint8_t *)hdr, hdr->length) != 0) { ctdb_fatal(ctdb, "Unable to queue packet\n");