The branch, master has been updated via 6be7da3 messaging3: Fix running down a messaging_context via c9cced0 poll_funcs_tevent: Fix a valgrind error from 4b09df8 pidl-wireshark: SWITCH_TYPE is not always defined, SwitchType() will try to find a fallback
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 6be7da3ee6c3d833fbf5d075bfabd04bda7e5097 Author: Volker Lendecke <v...@samba.org> Date: Wed Oct 22 15:11:43 2014 +0000 messaging3: Fix running down a messaging_context When you do a talloc_free(msg_ctx), existing waiters can't and don't have to clean up behind themselves properly anymore. The msg_ctx the cleanup function refers to is just gone. Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> Autobuild-User(master): Jeremy Allison <j...@samba.org> Autobuild-Date(master): Fri Oct 24 04:01:32 CEST 2014 on sn-devel-104 commit c9cced03224011ad25f5fc6b7d737fb2cabd3153 Author: Volker Lendecke <v...@samba.org> Date: Wed Oct 22 15:02:49 2014 +0000 poll_funcs_tevent: Fix a valgrind error The valgrind error happened in poll_funcs_tevent_handle_destructor in if (handle->ctx->refcount == 0) handle->ctx was already gone at the time this destructor was called. It happened because during messaging_init the messaging_dgm subsystem was free'ed. The unix_msg context and the poll_funcs_tevent_context are children of messaging_dgm_context. How was poll_funcs_tevent_handle_destructor still called? While working on the new notify subsystem I've added some messaging_read_send tevent_reqs, which register themselves with the dgm_context via messaging_dgm_register_tevent_context. They were not gone yet. When later these were also run down due to another talloc_free somewhere else, this destructor referenced dead memory. This code now protects the poll_funcs_tevent_handle against the poll_funcs_tevent_context going away first with the loop for (h = ctx->handles; h != NULL; h = h->next) { h->ctx = NULL; } in poll_funcs_tevent_context_destructor together with if (handle->ctx == NULL) { return 0; } in poll_funcs_tevent_handle_destructor. A side-effect of this code is that messaging_read_send request won't be satisfied anymore after a reinit_after_fork kicked in. But I think this is the right thing anyway: Every process should register its own message handlers explicitly. Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> ----------------------------------------------------------------------- Summary of changes: source3/lib/messages.c | 16 ++++++++++++++++ source3/lib/poll_funcs/poll_funcs_tevent.c | 28 ++++++++++++++++++++-------- 2 files changed, 36 insertions(+), 8 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/lib/messages.c b/source3/lib/messages.c index aaaee52..d4c580f 100644 --- a/source3/lib/messages.c +++ b/source3/lib/messages.c @@ -267,7 +267,23 @@ static void messaging_recv_cb(const uint8_t *msg, size_t msg_len, static int messaging_context_destructor(struct messaging_context *ctx) { + unsigned i; + messaging_dgm_destroy(); + + for (i=0; i<ctx->num_new_waiters; i++) { + if (ctx->new_waiters[i] != NULL) { + tevent_req_set_cleanup_fn(ctx->new_waiters[i], NULL); + ctx->new_waiters[i] = NULL; + } + } + for (i=0; i<ctx->num_waiters; i++) { + if (ctx->waiters[i] != NULL) { + tevent_req_set_cleanup_fn(ctx->waiters[i], NULL); + ctx->waiters[i] = NULL; + } + } + return 0; } diff --git a/source3/lib/poll_funcs/poll_funcs_tevent.c b/source3/lib/poll_funcs/poll_funcs_tevent.c index ee800ba..565cdaf 100644 --- a/source3/lib/poll_funcs/poll_funcs_tevent.c +++ b/source3/lib/poll_funcs/poll_funcs_tevent.c @@ -19,6 +19,7 @@ #include "poll_funcs_tevent.h" #include "tevent.h" #include "system/select.h" +#include "dlinklist.h" /* * A poll_watch is asked for by the engine using this library via @@ -53,7 +54,7 @@ struct poll_funcs_state { }; struct poll_funcs_tevent_context { - unsigned refcount; + struct poll_funcs_tevent_handle *handles; struct poll_funcs_state *state; unsigned slot; /* index into state->contexts[] */ struct tevent_context *ev; @@ -67,6 +68,7 @@ struct poll_funcs_tevent_context { * multiple times. So we have to share one poll_funcs_tevent_context. */ struct poll_funcs_tevent_handle { + struct poll_funcs_tevent_handle *prev, *next; struct poll_funcs_tevent_context *ctx; }; @@ -352,7 +354,7 @@ static struct poll_funcs_tevent_context *poll_funcs_tevent_context_new( return NULL; } - ctx->refcount = 0; + ctx->handles = NULL; ctx->state = state; ctx->ev = ev; ctx->slot = slot; @@ -385,7 +387,14 @@ fail: static int poll_funcs_tevent_context_destructor( struct poll_funcs_tevent_context *ctx) { + struct poll_funcs_tevent_handle *h; + ctx->state->contexts[ctx->slot] = NULL; + + for (h = ctx->handles; h != NULL; h = h->next) { + h->ctx = NULL; + } + return 0; } @@ -427,7 +436,7 @@ void *poll_funcs_tevent_register(TALLOC_CTX *mem_ctx, struct poll_funcs *f, } handle->ctx = state->contexts[slot]; - handle->ctx->refcount += 1; + DLIST_ADD(handle->ctx->handles, handle); talloc_set_destructor(handle, poll_funcs_tevent_handle_destructor); return handle; fail: @@ -438,14 +447,17 @@ fail: static int poll_funcs_tevent_handle_destructor( struct poll_funcs_tevent_handle *handle) { - if (handle->ctx->refcount == 0) { + if (handle->ctx == NULL) { + return 0; + } + if (handle->ctx->handles == NULL) { abort(); } - handle->ctx->refcount -= 1; - if (handle->ctx->refcount != 0) { - return 0; + DLIST_REMOVE(handle->ctx->handles, handle); + + if (handle->ctx->handles == NULL) { + TALLOC_FREE(handle->ctx); } - TALLOC_FREE(handle->ctx); return 0; } -- Samba Shared Repository