From 527e971a732d645d411df842ec4f8c401248ca0c Mon Sep 17 00:00:00 2001

From: Tomas Hozza <thozza@redhat.com>

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 <thozza@redhat.com>

---

 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