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