[bind] Fix race condition on send buffers in dighost.c (#794940)
Tomas Hozza
thozza at fedoraproject.org
Fri Oct 18 09:58:50 UTC 2013
commit 3ddaff2ea91e32b08286a5df826d1df6e6d65eb3
Author: Tomas Hozza <thozza at redhat.com>
Date: Fri Oct 18 11:48:03 2013 +0200
Fix race condition on send buffers in dighost.c (#794940)
Signed-off-by: Tomas Hozza <thozza at redhat.com>
bind.spec | 8 ++-
bind99-ISC-Bugs-34870.patch | 135 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 142 insertions(+), 1 deletions(-)
---
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 <thozza at redhat.com> 32:9.9.4-3
+- Fix race condition on send buffers in dighost.c (#794940)
+
* Tue Oct 08 2013 Tomas Hozza <thozza at redhat.com> 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 <thozza at 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 at 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
+
More information about the scm-commits
mailing list