poll_funcs_tevent: Fix a valgrind error
authorVolker Lendecke <vl@samba.org>
Wed, 22 Oct 2014 15:02:49 +0000 (15:02 +0000)
committerJeremy Allison <jra@samba.org>
Thu, 23 Oct 2014 23:40:16 +0000 (01:40 +0200)
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 <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
source3/lib/poll_funcs/poll_funcs_tevent.c

index ee800badb41d1fb25b85d3e662bbc28959619d6e..565cdaf1cfc57921b002a6f4451642f91d6766f4 100644 (file)
@@ -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;
 }