ctdb-daemon: Handle failure immediately, do housekeeping later

The callback should never be called before an immediate return.  The
callback might reply to a control and the caller of
ctdb_event_script_callback_v() may not have assigned/stolen the
pointer to control structure into the private data.  Therefore,
calling the callback can dereference an uninitialised pointer to the
control structure when attempting to reply.

An event script isn't being run until the child has been forked.  So
update relevant state and set the destructor after this.

If the child can't be forked then free the state and return with an
error.  The callback will not be called and the caller will process
the error correctly.

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

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

diff --git a/ctdb/server/eventscript.c b/ctdb/server/eventscript.c
index a5f26805a09b7365a9673d7cd51b008376fcbbf2..f555625996ecd60245c93c9c7bd0e3e13019be32 100644 (file)
--- a/ctdb/server/eventscript.c
+++ b/ctdb/server/eventscript.c
@@ -871,14 +871,6 @@ static int ctdb_event_script_callback_v(struct ctdb_context *ctdb,
        state->current = 0;
        state->child = 0;
 
-       if (call == CTDB_EVENT_MONITOR) {
-               ctdb->current_monitor = state;
-       }
-
-       talloc_set_destructor(state, event_script_destructor);
-
-       ctdb->active_events++;
-
        /* Nothing to do? */
        if (state->scripts->num_scripts == 0) {
                int ret = schedule_callback_immediate(ctdb, callback,
@@ -894,11 +886,18 @@ static int ctdb_event_script_callback_v(struct ctdb_context *ctdb,
 
        state->scripts->scripts[0].status = fork_child_for_script(ctdb, state);
        if (state->scripts->scripts[0].status != 0) {
-               /* Callback is called from destructor, with fail result. */
                talloc_free(state);
-               return 0;
+               return -1;
        }
 
+       if (call == CTDB_EVENT_MONITOR) {
+               ctdb->current_monitor = state;
+       }
+
+       ctdb->active_events++;
+
+       talloc_set_destructor(state, event_script_destructor);
+
        if (!timeval_is_zero(&state->timeout)) {
                tevent_add_timer(ctdb->ev, state,
                                 timeval_current_ofs(state->timeout.tv_sec,