diff options
author | Amitay Isaacs <amitay@gmail.com> | 2017-02-06 15:54:55 +1100 |
---|---|---|
committer | Karolin Seeger <kseeger@samba.org> | 2017-02-17 12:26:21 +0100 |
commit | 81130d3ede825f5e9fd3a6e509910211a3844608 (patch) | |
tree | f05b9341366f3308703a481e697adb3f2e997e4d | |
parent | f289980e5531372dd63ec483e265e48efb8cf207 (diff) | |
download | samba-81130d3ede825f5e9fd3a6e509910211a3844608.tar.gz |
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>
(cherry picked from commit 9db7785fc6ffbaad434ee189c0f46c488358aab5)
-rw-r--r-- | ctdb/common/comm.c | 46 |
1 files changed, 40 insertions, 6 deletions
diff --git a/ctdb/common/comm.c b/ctdb/common/comm.c index 1bbb46050b5..6b71c0f0615 100644 --- a/ctdb/common/comm.c +++ b/ctdb/common/comm.c @@ -262,14 +262,22 @@ static void comm_read_failed(struct tevent_req *req) * 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_entry *entry; 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); @@ -280,6 +288,7 @@ struct tevent_req *comm_write_send(TALLOC_CTX *mem_ctx, { 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) { @@ -291,15 +300,38 @@ struct tevent_req *comm_write_send(TALLOC_CTX *mem_ctx, 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; } +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( @@ -344,6 +376,8 @@ static void comm_write_done(struct tevent_req *subreq) } state->nwritten = nwritten; + state->entry->qentry = NULL; + TALLOC_FREE(state->entry); tevent_req_done(req); } @@ -393,8 +427,8 @@ static void comm_fd_handler(struct tevent_context *ev, 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, |