From 527e971a732d645d411df842ec4f8c401248ca0c Mon Sep 17 00:00:00 2001 From: Tomas Hozza Date: Fri, 18 Oct 2013 10:47:21 +0200 Subject: [PATCH] Dynamically allocate send buffers when sending query This prevents race condition, when the same buffer could be added into multiple bufferlists. One case when this happens is when timeout of sent UDP query expires before send_done() is called. New function isc_buffer_cloneused() has been added, so dynamically allocated copy of used region of a buffer can be created easily. (It should be added into buffer.c but to prevent API change it is in dighost.c) All functions creating a send socket event with send_done() callback have been modified to make dynamically allocated copies of every buffer added into query->sendlist. This list is then bounded to the send socket event. This way the same buffer can not be anymore added to the same bufferlist. Previously allocated copies of buffers are freed in send_done() callback. Signed-off-by: Tomas Hozza --- bin/dig/dighost.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 5 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 0d41529..7899c49 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -362,6 +362,36 @@ struct_tk_list tk_list = { {NULL, NULL, NULL, NULL, NULL}, 0}; "isc_mutex_unlock");\ } +static isc_result_t +isc_buffer_cloneused(isc_mem_t *mctx, isc_buffer_t *src_buffer, isc_buffer_t **dynbuffer) { + /* + * Make 'dynbuffer' refer to a dynamically allocated copy of used region of 'src_buffer'. + */ + isc_result_t result; + isc_region_t used_region; + isc_buffer_t *tmpbuf = NULL; + + REQUIRE(dynbuffer != NULL); + REQUIRE(*dynbuffer == NULL); + REQUIRE(src_buffer != NULL); + REQUIRE(ISC_BUFFER_VALID(src_buffer)); + + result = isc_buffer_allocate(mctx, &tmpbuf, src_buffer->length); + if (result != ISC_R_SUCCESS) + return result; + + isc_buffer_usedregion(src_buffer, &used_region); + result = isc_buffer_copyregion(tmpbuf, &used_region); + if (result != ISC_R_SUCCESS) { + isc_buffer_free(&tmpbuf); + return result; + } + + *dynbuffer = tmpbuf; + + return (ISC_R_SUCCESS); +} + static void cancel_lookup(dig_lookup_t *lookup); @@ -2416,8 +2446,10 @@ send_done(isc_task_t *_task, isc_event_t *event) { for (b = ISC_LIST_HEAD(sevent->bufferlist); b != NULL; - b = ISC_LIST_HEAD(sevent->bufferlist)) + b = ISC_LIST_HEAD(sevent->bufferlist)) { ISC_LIST_DEQUEUE(sevent->bufferlist, b, link); + isc_buffer_free(&b); + } query = event->ev_arg; query->waiting_senddone = ISC_FALSE; @@ -2617,6 +2649,7 @@ send_tcp_connect(dig_query_t *query) { static void send_udp(dig_query_t *query) { dig_lookup_t *l = NULL; + isc_buffer_t *tmpbuf = NULL; isc_result_t result; debug("send_udp(%p)", query); @@ -2663,8 +2696,14 @@ send_udp(dig_query_t *query) { recvcount++; debug("recvcount=%d", recvcount); } + /* + * Make a copy of the query send buffer so it is not reused + * in multiple socket send events. The buffer is freed in send_done(). + */ + result = isc_buffer_cloneused(mctx, &query->sendbuf, &tmpbuf); + check_result(result, "isc_buffer_cloneused"); ISC_LIST_INIT(query->sendlist); - ISC_LIST_ENQUEUE(query->sendlist, &query->sendbuf, link); + ISC_LIST_ENQUEUE(query->sendlist, tmpbuf, link); debug("sending a request"); TIME_NOW(&query->time_sent); INSIST(query->sock != NULL); @@ -2838,6 +2877,7 @@ static void launch_next_query(dig_query_t *query, isc_boolean_t include_question) { isc_result_t result; dig_lookup_t *l; + isc_buffer_t *tmpbuf = NULL; INSIST(!free_now); @@ -2861,9 +2901,17 @@ launch_next_query(dig_query_t *query, isc_boolean_t include_question) { isc_buffer_putuint16(&query->slbuf, (isc_uint16_t) query->sendbuf.used); ISC_LIST_INIT(query->sendlist); ISC_LINK_INIT(&query->slbuf, link); - ISC_LIST_ENQUEUE(query->sendlist, &query->slbuf, link); - if (include_question) - ISC_LIST_ENQUEUE(query->sendlist, &query->sendbuf, link); + + /* need to clone send buffers as they are freed in send_done() */ + result = isc_buffer_cloneused(mctx, &query->slbuf, &tmpbuf); + check_result(result, "isc_buffer_cloneused"); + ISC_LIST_ENQUEUE(query->sendlist, tmpbuf, link); + if (include_question) { + tmpbuf = NULL; + result = isc_buffer_cloneused(mctx, &query->sendbuf, &tmpbuf); + check_result(result, "isc_buffer_cloneused"); + ISC_LIST_ENQUEUE(query->sendlist, tmpbuf, link); + } ISC_LINK_INIT(&query->lengthbuf, link); ISC_LIST_ENQUEUE(query->lengthlist, &query->lengthbuf, link); -- 1.8.3.1