Fix race condition on send buffers in dighost.c (#794940)

Signed-off-by: Tomas Hozza <thozza@redhat.com>

bind.spec

@@ -26,7 +26,7 @@ Summary:  The Berkeley Internet Name Domain (BIND) DNS (Domain Name System) serv
 Name:     bind
 License:  ISC
 Version:  9.9.4
-Release:  2%{?PATCHVER}%{?PREVER}%{?dist}
+Release:  3%{?PATCHVER}%{?PREVER}%{?dist}
 Epoch:    32
 Url:      http://www.isc.org/products/BIND/
 Buildroot:%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
@@ -82,6 +82,8 @@ Patch137:bind99-rrl.patch
 # Install dns/update.h header for bind-dyndb-ldap plugin
 Patch138:bind-9.9.3-include-update-h.patch
 Patch139:bind99-ISC-Bugs-34738.patch
+# reported upstream -> [ISC-Bugs #34870]
+Patch140:bind99-ISC-Bugs-34870.patch
 
 # SDB patches
 Patch11: bind-9.3.2b2-sdbsrc.patch
@@ -281,6 +283,7 @@ popd
 %patch137 -p1 -b .rrl
 %patch138 -p1 -b .update
 %patch139 -p1 -b .journal
+%patch140 -p1 -b .send_buffer
 
 %if %{SDB}
 %patch101 -p1 -b .old-api
@@ -784,6 +787,9 @@ rm -rf ${RPM_BUILD_ROOT}
 %endif
 
 %changelog
+* Fri Oct 18 2013 Tomas Hozza <thozza@redhat.com> 32:9.9.4-3
+- Fix race condition on send buffers in dighost.c (#794940)
+
 * Tue Oct 08 2013 Tomas Hozza <thozza@redhat.com> 32:9.9.4-2
 - install isc/errno2result.h header
 

bind99-ISC-Bugs-34870.patch

@@ -0,0 +1,135 @@
+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
+