ctdb-common: Fix use-after-free error in comm_fd_handler()

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

comm_write_send() creates a new tevent_req and adds it to the queue
of requests to be processed.  If this tevent_req is freed, then the
queue entry is not removed causing use-after-free error.

If the tevent_req returned by comm_write_send() is freed, then that
request should be removed from the queue and any pending actions based
on that request should also be removed.

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

diff --git a/ctdb/common/comm.c b/ctdb/common/comm.c
index 7f370da9b8f1ec9873319f28cb5134da6b0323ae..12f49702a90fa9ccca72f0c76c43b744c49ec7d7 100644 (file)
--- a/ctdb/common/comm.c
+++ b/ctdb/common/comm.c
@@ -251,14 +251,22 @@ static void comm_read_failed(struct tevent_req *req)
  * Write packets
  */
 
  * Write packets
  */
 

+struct comm_write_entry {
+       struct comm_context *comm;
+       struct tevent_queue_entry *qentry;
+       struct tevent_req *req;
+};
+

 struct comm_write_state {
        struct tevent_context *ev;
        struct comm_context *comm;
 struct comm_write_state {
        struct tevent_context *ev;
        struct comm_context *comm;

+       struct comm_write_entry *entry;

        struct tevent_req *subreq;
        uint8_t *buf;
        size_t buflen, nwritten;
 };
 
        struct tevent_req *subreq;
        uint8_t *buf;
        size_t buflen, nwritten;
 };
 

+static int comm_write_entry_destructor(struct comm_write_entry *entry);

 static void comm_write_trigger(struct tevent_req *req, void *private_data);
 static void comm_write_done(struct tevent_req *subreq);
 
 static void comm_write_trigger(struct tevent_req *req, void *private_data);
 static void comm_write_done(struct tevent_req *subreq);
 


@@ -269,6 +277,7 @@ struct tevent_req *comm_write_send(TALLOC_CTX *mem_ctx,
 {
        struct tevent_req *req;
        struct comm_write_state *state;
 {
        struct tevent_req *req;
        struct comm_write_state *state;

+       struct comm_write_entry *entry;

 
        req = tevent_req_create(mem_ctx, &state, struct comm_write_state);
        if (req == NULL) {
 
        req = tevent_req_create(mem_ctx, &state, struct comm_write_state);
        if (req == NULL) {


@@ -280,15 +289,38 @@ struct tevent_req *comm_write_send(TALLOC_CTX *mem_ctx,
        state->buf = buf;
        state->buflen = buflen;
 
        state->buf = buf;
        state->buflen = buflen;
 

-       if (!tevent_queue_add_entry(comm->queue, ev, req,
-                                   comm_write_trigger, NULL)) {
-               talloc_free(req);
-               return NULL;
+       entry = talloc_zero(state, struct comm_write_entry);
+       if (tevent_req_nomem(entry, req)) {
+               return tevent_req_post(req, ev);

        }
 
        }
 

+       entry->comm = comm;
+       entry->req = req;
+       entry->qentry = tevent_queue_add_entry(comm->queue, ev, req,
+                                              comm_write_trigger, NULL);
+       if (tevent_req_nomem(entry->qentry, req)) {
+               return tevent_req_post(req, ev);
+       }
+
+       state->entry = entry;
+       talloc_set_destructor(entry, comm_write_entry_destructor);
+

        return req;
 }
 
        return req;
 }
 

+static int comm_write_entry_destructor(struct comm_write_entry *entry)
+{
+       struct comm_context *comm = entry->comm;
+
+       if (comm->write_req == entry->req) {
+               comm->write_req = NULL;
+               TEVENT_FD_NOT_WRITEABLE(comm->fde);
+       }
+
+       TALLOC_FREE(entry->qentry);
+       return 0;
+}
+

 static void comm_write_trigger(struct tevent_req *req, void *private_data)
 {
        struct comm_write_state *state = tevent_req_data(
 static void comm_write_trigger(struct tevent_req *req, void *private_data)
 {
        struct comm_write_state *state = tevent_req_data(


@@ -333,6 +365,8 @@ static void comm_write_done(struct tevent_req *subreq)
        }
 
        state->nwritten = nwritten;
        }
 
        state->nwritten = nwritten;

+       state->entry->qentry = NULL;
+       TALLOC_FREE(state->entry);

        tevent_req_done(req);
 }
 
        tevent_req_done(req);
 }
 


@@ -382,8 +416,8 @@ static void comm_fd_handler(struct tevent_context *ev,
                struct comm_write_state *write_state;
 
                if (comm->write_req == NULL) {
                struct comm_write_state *write_state;
 
                if (comm->write_req == NULL) {

-                       /* This should never happen */
-                       abort();
+                       TEVENT_FD_NOT_WRITEABLE(comm->fde);
+                       return;

                }
 
                write_state = tevent_req_data(comm->write_req,
                }
 
                write_state = tevent_req_data(comm->write_req,