ctdb-client: Fix access after free error

State is stolen onto tmp_ctx above so can't be referenced after
tmp_ctx is freed.  So, state->status has to be looked at earlier.

Moving it immediately before the talloc_free(tmp_ctx) isn't sufficient
because invoking the callback appears to cause a recursive call to
ctdb_control_recv(), which also frees state.

Referencing it at the top seems safe.

==23982== Invalid read of size 4
==23982==    at 0x4204AE: ctdb_control_recv (ctdb_client.c:1181)
==23982==    by 0x420645: invoke_control_callback (ctdb_client.c:971)
==23982==    by 0x5E675EC: tevent_common_loop_timer_delay (tevent_timed.c:341)
==23982==    by 0x5E68639: epoll_event_loop_once (tevent_epoll.c:911)
==23982==    by 0x5E66BD6: std_event_loop_once (tevent_standard.c:114)
==23982==    by 0x5E622EC: _tevent_loop_once (tevent.c:533)
==23982==    by 0x4255F7: ctdb_client_async_wait (ctdb_client.c:3385)
==23982==    by 0x42578A: ctdb_client_async_control (ctdb_client.c:3442)
==23982==    by 0x41B405: ctdb_get_nodes_files (ctdb.c:5488)
==23982==    by 0x41B405: check_all_node_files_are_identical (ctdb.c:5530)
==23982==    by 0x41B405: control_reload_nodes_file (ctdb.c:5673)
==23982==    by 0x404DBA: main (ctdb.c:6008)
==23982==  Address 0x7e98d9c is 108 bytes inside a block of size 168 free'd
==23982==    at 0x4C2CDFB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==23982==    by 0x5652692: _tc_free_internal (talloc.c:1125)
==23982==    by 0x5652692: _tc_free_children_internal (talloc.c:1570)
==23982==    by 0x564B952: _tc_free_internal (talloc.c:1081)
==23982==    by 0x564B952: _talloc_free_internal (talloc.c:1151)
==23982==    by 0x564B952: _talloc_free (talloc.c:1693)
==23982==    by 0x4204C9: ctdb_control_recv (ctdb_client.c:1182)
==23982==    by 0x4207AA: async_callback (ctdb_client.c:3350)
==23982==    by 0x4204AD: ctdb_control_recv (ctdb_client.c:1179)
==23982==    by 0x420645: invoke_control_callback (ctdb_client.c:971)
==23982==    by 0x5E675EC: tevent_common_loop_timer_delay (tevent_timed.c:341)
==23982==    by 0x5E68639: epoll_event_loop_once (tevent_epoll.c:911)
==23982==    by 0x5E66BD6: std_event_loop_once (tevent_standard.c:114)
==23982==    by 0x5E622EC: _tevent_loop_once (tevent.c:533)
==23982==    by 0x4255F7: ctdb_client_async_wait (ctdb_client.c:3385)
==23982==  Block was alloc'd at
==23982==    at 0x4C2BBCF: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==23982==    by 0x564DBEC: __talloc_with_prefix (talloc.c:675)
==23982==    by 0x564DBEC: __talloc (talloc.c:716)
==23982==    by 0x564DBEC: _talloc_named_const (talloc.c:873)
==23982==    by 0x564DBEC: _talloc_zero (talloc.c:2318)
==23982==    by 0x42017F: ctdb_control_send (ctdb_client.c:1086)
==23982==    by 0x425746: ctdb_client_async_control (ctdb_client.c:3431)
==23982==    by 0x41B405: ctdb_get_nodes_files (ctdb.c:5488)
==23982==    by 0x41B405: check_all_node_files_are_identical (ctdb.c:5530)
==23982==    by 0x41B405: control_reload_nodes_file (ctdb.c:5673)
==23982==    by 0x404DBA: main (ctdb.c:6008)
==23982==

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

diff --git a/ctdb/client/ctdb_client.c b/ctdb/client/ctdb_client.c
index 19502be184192295205c9d48d38c629fedf9dbb7..6748e692a62e306e3dc4edb85e974750f14d5ce0 100644 (file)
--- a/ctdb/client/ctdb_client.c
+++ b/ctdb/client/ctdb_client.c
@@ -1170,6 +1170,7 @@ int ctdb_control_recv(struct ctdb_context *ctdb,
        }
 
        if (state->errormsg) {
+               int s = (state->status == 0 ? -1 : state->status);
                DEBUG(DEBUG_ERR,("ctdb_control error: '%s'\n", state->errormsg));
                if (errormsg) {
                        (*errormsg) = talloc_move(mem_ctx, &state->errormsg);
@@ -1178,7 +1179,7 @@ int ctdb_control_recv(struct ctdb_context *ctdb,
                        state->async.fn(state);
                }
                talloc_free(tmp_ctx);
-               return (state->status == 0 ? -1 : state->status);
+               return s;
        }
 
        if (outdata) {