From 3ddaff2ea91e32b08286a5df826d1df6e6d65eb3 Mon Sep 17 00:00:00 2001 From: Tomas Hozza Date: Oct 18 2013 09:48:03 +0000 Subject: Fix race condition on send buffers in dighost.c (#794940) Signed-off-by: Tomas Hozza --- diff --git a/bind.spec b/bind.spec index c9c9e74..cf76487 100644 --- a/bind.spec +++ b/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 32:9.9.4-3 +- Fix race condition on send buffers in dighost.c (#794940) + * Tue Oct 08 2013 Tomas Hozza 32:9.9.4-2 - install isc/errno2result.h header diff --git a/bind99-ISC-Bugs-34870.patch b/bind99-ISC-Bugs-34870.patch new file mode 100644 index 0000000..158794a --- /dev/null +++ b/bind99-ISC-Bugs-34870.patch @@ -0,0 +1,135 @@ +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 +