From 806a45e78be86ffb600647238d398d4279057c3a Mon Sep 17 00:00:00 2001
From: Marek Vavrusa <marek.vavrusa@nic.cz>
Date: Fri, 25 May 2012 14:59:43 +0200
Subject: [PATCH] Added missing RCU unlocks, resolved lock order violations.

---
 src/knot/server/xfr-handler.c        |  2 +-
 src/knot/server/zones.c              | 35 +++++++++++++++-------------
 src/libknot/hash/cuckoo-hash-table.c |  2 ++
 src/libknot/nameserver/name-server.c |  2 ++
 4 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/src/knot/server/xfr-handler.c b/src/knot/server/xfr-handler.c
index 136448d7c3..a31d76833c 100644
--- a/src/knot/server/xfr-handler.c
+++ b/src/knot/server/xfr-handler.c
@@ -698,11 +698,11 @@ int xfr_process_event(xfrworker_t *w, int fd, knot_ns_xfr_t *data, uint8_t *buf,
 				              "in %d seconds.\n",
 				              data->msgpref, tmr_s / 1000);
 			}
-			rcu_read_unlock();
 
 			/* Update timers. */
 			server_t *server = (server_t *)knot_ns_get_data(w->ns);
 			zones_timers_update(zone, zd->conf, server->sched);
+			rcu_read_unlock();
 			
 		} else {
 			/* Cleanup */
diff --git a/src/knot/server/zones.c b/src/knot/server/zones.c
index 0143a58e1f..1dacd8f11b 100644
--- a/src/knot/server/zones.c
+++ b/src/knot/server/zones.c
@@ -242,6 +242,7 @@ static uint32_t zones_soa_timer(knot_zone_t *zone,
 
 	knot_zone_contents_t * zc = knot_zone_get_contents((zone));
 	if (!zc) {
+		rcu_read_unlock();
 		return 0;
 	}
 
@@ -295,7 +296,6 @@ static uint32_t zones_soa_expire(knot_zone_t *zone)
  */
 static int zones_expire_ev(event_t *e)
 {
-	rcu_read_lock();
 	dbg_zones("zones: EXPIRE timer event\n");
 	knot_zone_t *zone = (knot_zone_t *)e->data;
 	if (zone == NULL || zone->data == NULL) {
@@ -608,6 +608,7 @@ static int zones_notify_send(event_t *e)
 		log_server_notice("NOTIFY query maximum number of retries "
 				  "for zone '%s' exceeded.\n",
 				  zd->conf->name);
+		rcu_read_unlock();
 		pthread_mutex_lock(&zd->lock);
 		rem_node(&ev->n);
 		evsched_event_free(e->parent, e);
@@ -620,16 +621,15 @@ static int zones_notify_send(event_t *e)
 	int retry_tmr = ev->timeout * 1000;
  
 	/* Reschedule. */
-	conf_read_lock();
 	evsched_schedule(e->parent, e, retry_tmr);
 	dbg_notify("notify: Query RETRY after %u secs (zone '%s')\n",
 	           retry_tmr / 1000, zd->conf->name);
-	conf_read_unlock();
 	
 	/* Prepare buffer for query. */
 	uint8_t *qbuf = malloc(SOCKET_MTU_SZ);
 	if (qbuf == NULL) {
 		log_zone_error("Not enough memory to allocate NOTIFY query.\n");
+		rcu_read_unlock();
 		return KNOTD_ENOMEM;
 	}
 	
@@ -639,9 +639,6 @@ static int zones_notify_send(event_t *e)
 	int ret = notify_create_request(contents, qbuf, &buflen);
 	if (ret == KNOTD_EOK && zd->server) {
 
-		/* Lock RCU. */
-		rcu_read_lock();
-
 		/* Create socket on random port. */
 		int sock = socket_create(ev->addr.family, SOCK_DGRAM);
 		
@@ -666,9 +663,6 @@ static int zones_notify_send(event_t *e)
 			
 		}
 		
-		/* Unlock RCU */
-		rcu_read_unlock();
-		
 		/* Mark as finished to prevent stalling. */
 		evsched_event_finished(e->parent);
 
@@ -1261,6 +1255,7 @@ static int zones_journal_apply(knot_zone_t *zone)
 	knot_zone_contents_t *contents = knot_zone_get_contents(zone);
 	zonedata_t *zd = (zonedata_t *)knot_zone_data(zone);
 	if (!contents || !zd) {
+		rcu_read_unlock();
 		return KNOTD_ENOENT;
 	}
 
@@ -1273,6 +1268,7 @@ static int zones_journal_apply(knot_zone_t *zone)
 	soa_rr = knot_rrset_rdata(soa_rrs);
 	int64_t serial_ret = knot_rdata_soa_serial(soa_rr);
 	if (serial_ret < 0) {
+		rcu_read_unlock();
 		return KNOTD_EINVAL;
 	}
 	uint32_t serial = (uint32_t)serial_ret;
@@ -1310,6 +1306,7 @@ static int zones_journal_apply(knot_zone_t *zone)
 			rcu_read_unlock();
 			apply_ret = xfrin_switch_zone(zone, contents,
 			                              XFR_TYPE_IIN);
+			rcu_read_lock();
 			if (apply_ret == KNOT_EOK) {
 				xfrin_cleanup_successful_update(
 				                        &chsets->changes);
@@ -1329,10 +1326,10 @@ static int zones_journal_apply(knot_zone_t *zone)
 	} else {
 		dbg_zones("zones: failed to load changesets - %s\n",
 		          knotd_strerror(ret));
-		rcu_read_unlock();
 	}
 
 	/* Free changesets and return. */
+	rcu_read_unlock();
 	knot_free_changesets(&chsets);
 	return ret;
 }
@@ -1925,12 +1922,14 @@ int zones_update_db_from_config(const conf_t *conf, knot_nameserver_t *ns,
 	if (*db_old == NULL) {
 		log_server_error("Missing zone database in nameserver structure"
 		                 ".\n");
+		rcu_read_unlock();
 		return KNOTD_ERROR;
 	}
 
 	/* Create new zone DB */
 	knot_zonedb_t *db_new = knot_zonedb_new();
 	if (db_new == NULL) {
+		rcu_read_unlock();
 		return KNOTD_ERROR;
 	}
 
@@ -1964,13 +1963,14 @@ int zones_update_db_from_config(const conf_t *conf, knot_nameserver_t *ns,
 	 * All other have been loaded again so that the old must be destroyed.
 	 */
 	int ret = zones_remove_zones(db_new, *db_old);
+	
+	/* Unlock RCU, messing with any data will not affect us now */
+	rcu_read_unlock();
+	
 	if (ret != KNOTD_EOK) {
 		return ret;
 	}
 
-	/* Unlock RCU, messing with any data will not affect us now */
-	rcu_read_unlock();
-
 	return KNOTD_EOK;
 }
 
@@ -1999,6 +1999,7 @@ int zones_zonefile_sync(knot_zone_t *zone, journal_t *journal)
 	knot_zone_contents_t *contents = knot_zone_get_contents(zone);
 	if (!contents) {
 		pthread_mutex_unlock(&zd->lock);
+		rcu_read_unlock();
 		return KNOTD_EINVAL;
 	}
 
@@ -2013,6 +2014,7 @@ int zones_zonefile_sync(knot_zone_t *zone, journal_t *journal)
 	int64_t serial_ret = knot_rdata_soa_serial(soa_rr);
 	if (serial_ret < 0) {
 		pthread_mutex_unlock(&zd->lock);
+		rcu_read_unlock();
 		return KNOTD_EINVAL;
 	}
 	uint32_t serial_to = (uint32_t)serial_ret;
@@ -2030,9 +2032,9 @@ int zones_zonefile_sync(knot_zone_t *zone, journal_t *journal)
 			dbg_zones("zones: failed to sync '%s' to '%s'\n",
 			          zd->conf->name, zd->conf->file);
 			pthread_mutex_unlock(&zd->lock);
+			rcu_read_unlock();
 			return ret;
 		}
-		
 		conf_read_unlock();
 
 		/* Update journal entries. */
@@ -2433,10 +2435,10 @@ int zones_process_response(knot_nameserver_t *nameserver,
 			log_zone_info("SOA query of '%s' to '%s@%d': Answered, no "
 				      "transfer needed.\n",
 			              zd->conf->name, r_addr, r_port);
-			rcu_read_unlock();
-
+			
 			/* Reinstall timers. */
 			zones_timers_update(zone, zd->conf, sched);
+			rcu_read_unlock();
 			return KNOTD_EOK;
 		}
 		
@@ -3184,6 +3186,7 @@ int zones_timers_update(knot_zone_t *zone, conf_zone_t *cfzone, evsched_t *sch)
 	pthread_mutex_unlock(&zd->lock);
 
 	/* Check XFR/IN master server. */
+	conf_read_lock();
 	if (zd->xfr_in.master.ptr) {
 
 		/* Schedule REFRESH timer. */
diff --git a/src/libknot/hash/cuckoo-hash-table.c b/src/libknot/hash/cuckoo-hash-table.c
index 50a7a0f5b7..9db32bf30e 100644
--- a/src/libknot/hash/cuckoo-hash-table.c
+++ b/src/libknot/hash/cuckoo-hash-table.c
@@ -1037,6 +1037,7 @@ int ck_update_item(const ck_hash_table_t *table, const char *key, size_t length,
 	ck_hash_table_item_t **item = ck_find_item_nc(table, key, length);
 
 	if (item == NULL || (*item) == NULL) {
+		rcu_read_unlock();
 		return -1;
 	}
 
@@ -1060,6 +1061,7 @@ int ck_delete_item(const ck_hash_table_t *table, const char *key, size_t length,
 	ck_hash_table_item_t **place = ck_find_item_nc(table, key, length);
 
 	if (place == NULL) {
+		rcu_read_unlock();
 		return -1;
 	}
 
diff --git a/src/libknot/nameserver/name-server.c b/src/libknot/nameserver/name-server.c
index a9cde8e4ab..3a2f271d1a 100644
--- a/src/libknot/nameserver/name-server.c
+++ b/src/libknot/nameserver/name-server.c
@@ -2509,6 +2509,7 @@ static int ns_ixfr_from_zone(knot_ns_xfr_t *xfr)
 		/*! \todo Probably rename the function. */
 		ns_xfr_send_and_clear(xfr, 1);
 //		socket_close(xfr->session);  /*! \todo Remove for UDP.*/
+		rcu_read_unlock();
 		return res;
 	}
 
@@ -2517,6 +2518,7 @@ static int ns_ixfr_from_zone(knot_ns_xfr_t *xfr)
 		res = ns_ixfr_put_changeset(xfr, &chgsets->sets[i]);
 		if (res != KNOT_EOK) {
 			// answer is sent
+			rcu_read_unlock();
 			return res;
 		}
 	}
-- 
GitLab