From: Wouter Wijngaards <wouter@nlnetlabs.nl>
Date: Mon, 24 Oct 2011 13:49:59 +0000 (+0000)
Subject: - Fix make_new_space function so that the incoming query is not
X-Git-Tag: release-1.4.14rc1~44
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0f825eb2836b1c7cec8a6bc70a3f26a5fae5485b;p=thirdparty%2Funbound.git

- Fix make_new_space function so that the incoming query is not
  overwritten if a jostled out query causes a waiting query to be
  resumed that then fails and sends an error message.  (Thanks to
  Matthew Lee).


git-svn-id: file:///svn/unbound/trunk@2523 be551aaa-1e26-0410-a405-d3ace91eadb9
---

diff --git a/doc/Changelog b/doc/Changelog
index 28a301680..8602b98a9 100644
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -2,6 +2,10 @@
 	- Fix resolve of partners.extranet.microsoft.com with a fix for the
 	  server selection for choosing out of a (particular) list of bad
 	  choices.
+	- Fix make_new_space function so that the incoming query is not
+	  overwritten if a jostled out query causes a waiting query to be
+	  resumed that then fails and sends an error message.  (Thanks to
+	  Matthew Lee).
 
 21 October 2011: Wouter
 	- fix --enable-allsymbols, fptr wlist is disabled on windows with this 
diff --git a/services/mesh.c b/services/mesh.c
index aee0aa5e6..d9222d69c 100644
--- a/services/mesh.c
+++ b/services/mesh.c
@@ -162,7 +162,8 @@ mesh_create(struct module_stack* stack, struct module_env* env)
 		return NULL;
 	}
 	mesh->histogram = timehist_setup();
-	if(!mesh->histogram) {
+	mesh->qbuf_bak = ldns_buffer_new(env->cfg->msg_buffer_size);
+	if(!mesh->histogram || !mesh->qbuf_bak) {
 		free(mesh);
 		log_err("mesh area alloc: out of memory");
 		return NULL;
@@ -209,6 +210,7 @@ mesh_delete(struct mesh_area* mesh)
 	while(mesh->all.count)
 		mesh_delete_helper(mesh->all.root);
 	timehist_delete(mesh->histogram);
+	ldns_buffer_free(mesh->qbuf_bak);
 	free(mesh);
 }
 
@@ -232,7 +234,7 @@ mesh_delete_all(struct mesh_area* mesh)
 	mesh->jostle_last = NULL;
 }
 
-int mesh_make_new_space(struct mesh_area* mesh)
+int mesh_make_new_space(struct mesh_area* mesh, ldns_buffer* qbuf)
 {
 	struct mesh_state* m = mesh->jostle_first;
 	/* free space is available */
@@ -250,6 +252,8 @@ int mesh_make_new_space(struct mesh_area* mesh)
 				"make space for a new one",
 				m->s.qinfo.qname, m->s.qinfo.qtype,
 				m->s.qinfo.qclass);
+			/* backup the query */
+			if(qbuf) ldns_buffer_copy(mesh->qbuf_bak, qbuf);
 			/* notify supers */
 			if(m->super_set.count > 0) {
 				verbose(VERB_ALGO, "notify supers of failure");
@@ -259,6 +263,9 @@ int mesh_make_new_space(struct mesh_area* mesh)
 			}
 			mesh->stats_jostled ++;
 			mesh_state_delete(&m->s);
+			/* restore the query - note that the qinfo ptr to
+			 * the querybuffer is then correct again. */
+			if(qbuf) ldns_buffer_copy(qbuf, mesh->qbuf_bak);
 			return 1;
 		}
 	}
@@ -280,7 +287,7 @@ void mesh_new_client(struct mesh_area* mesh, struct query_info* qinfo,
 	int added = 0;
 	/* does this create a new reply state? */
 	if(!s || s->list_select == mesh_no_list) {
-		if(!mesh_make_new_space(mesh)) {
+		if(!mesh_make_new_space(mesh, rep->c->buffer)) {
 			verbose(VERB_ALGO, "Too many queries. dropping "
 				"incoming query.");
 			comm_point_drop_reply(rep);
@@ -431,7 +438,7 @@ void mesh_new_prefetch(struct mesh_area* mesh, struct query_info* qinfo,
 			s->s.prefetch_leeway = leeway;
 		return;
 	}
-	if(!mesh_make_new_space(mesh)) {
+	if(!mesh_make_new_space(mesh, NULL)) {
 		verbose(VERB_ALGO, "Too many queries. dropped prefetch.");
 		mesh->stats_dropped ++;
 		return;
@@ -1137,7 +1144,8 @@ mesh_get_mem(struct mesh_area* mesh)
 {
 	struct mesh_state* m;
 	size_t s = sizeof(*mesh) + sizeof(struct timehist) +
-		sizeof(struct th_buck)*mesh->histogram->num;
+		sizeof(struct th_buck)*mesh->histogram->num +
+		sizeof(ldns_buffer) + ldns_buffer_capacity(mesh->qbuf_bak);
 	RBTREE_FOR(m, struct mesh_state*, &mesh->all) {
 		/* all, including m itself allocated in qstate region */
 		s += regional_get_mem(m->s.region);
diff --git a/services/mesh.h b/services/mesh.h
index a6f3b58a9..5f109779a 100644
--- a/services/mesh.h
+++ b/services/mesh.h
@@ -123,6 +123,10 @@ struct mesh_area {
 	/** (extended stats) rcode nodata in replies */
 	size_t ans_nodata;
 
+	/** backup of query if other operations recurse and need the
+	 * network buffers */
+	ldns_buffer* qbuf_bak;
+
 	/** double linked list of the run-to-completion query states.
 	 * These are query states with a reply */
 	struct mesh_state* forever_first;
@@ -535,9 +539,16 @@ int mesh_state_ref_compare(const void* ap, const void* bp);
 /**
  * Make space for another recursion state for a reply in the mesh
  * @param mesh: mesh area
+ * @param qbuf: query buffer to save if recursion is invoked to make space.
+ *    This buffer is necessary, because the following sequence in calls
+ *    can result in an overwrite of the incoming query:
+ *    delete_other_mesh_query - iter_clean - serviced_delete - waiting
+ *    udp query is sent - on error callback - callback sends SERVFAIL reply
+ *    over the same network channel, and shared UDP buffer is overwritten.
+ *    You can pass NULL if there is no buffer that must be backed up.
  * @return false if no space is available.
  */
-int mesh_make_new_space(struct mesh_area* mesh);
+int mesh_make_new_space(struct mesh_area* mesh, ldns_buffer* qbuf);
 
 /**
  * Insert mesh state into a double linked list.  Inserted at end.