From: Brian Behlendorf <behlendorf1@llnl.gov>
Date: Thu, 31 Jan 2013 20:59:39 +0000 (-0800)
Subject: Fix tsd_get/set() race with tsd_exit/destroy()
X-Git-Tag: spl-0.6.0-rc14~1
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6ef94aa67a48792d98b76152661743e42c23a03f;p=spl

Fix tsd_get/set() race with tsd_exit/destroy()

The tsd_exit() and tsd_destroy() functions remove entries from
hash bins without taking the hash bin lock.  They do take the
table lock, but tsd_get() and tsd_set() only take the hash bin
lock to allow for maximum concurency.

The result is that while tsd_get() and tsd_set() are traversing
the hash bin list it can be modified by another thread in which
happens to hash to the same value.  To avoid this add the needed
locking to tsd_exit() and tsd_destroy().

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #174
---

diff --git a/module/spl/spl-tsd.c b/module/spl/spl-tsd.c
index f5c5795..c63a552 100644
--- a/module/spl/spl-tsd.c
+++ b/module/spl/spl-tsd.c
@@ -531,6 +531,8 @@ tsd_destroy(uint_t *keyp)
 	HLIST_HEAD(work);
 	tsd_hash_table_t *table;
 	tsd_hash_entry_t *dtor_entry, *entry;
+	tsd_hash_bin_t *dtor_entry_bin, *entry_bin;
+	ulong_t hash;
 	SENTRY;
 
 	table = tsd_hash_table;
@@ -554,12 +556,25 @@ tsd_destroy(uint_t *keyp)
 				   tsd_hash_entry_t, he_key_list);
 		ASSERT3U(dtor_entry->he_key, ==, entry->he_key);
 		ASSERT3P(dtor_entry->he_dtor, ==, entry->he_dtor);
+
+		hash = hash_long((ulong_t)entry->he_key *
+		     (ulong_t)entry->he_pid, table->ht_bits);
+		entry_bin = &table->ht_bins[hash];
+
+		spin_lock(&entry_bin->hb_lock);
 		tsd_hash_del(table, entry);
 		hlist_add_head(&entry->he_list, &work);
+		spin_unlock(&entry_bin->hb_lock);
 	}
 
+	hash = hash_long((ulong_t)dtor_entry->he_key *
+	    (ulong_t)dtor_entry->he_pid, table->ht_bits);
+	dtor_entry_bin = &table->ht_bins[hash];
+
+	spin_lock(&dtor_entry_bin->hb_lock);
 	tsd_hash_del(table, dtor_entry);
 	hlist_add_head(&dtor_entry->he_list, &work);
+	spin_unlock(&dtor_entry_bin->hb_lock);
 	spin_unlock(&table->ht_lock);
 
 	tsd_hash_dtor(&work);
@@ -583,6 +598,8 @@ tsd_exit(void)
 	HLIST_HEAD(work);
 	tsd_hash_table_t *table;
 	tsd_hash_entry_t *pid_entry, *entry;
+	tsd_hash_bin_t *pid_entry_bin, *entry_bin;
+	ulong_t hash;
 	SENTRY;
 
 	table = tsd_hash_table;
@@ -599,18 +616,32 @@ tsd_exit(void)
 	/*
 	 * All keys associated with this pid must be linked off of the
 	 * PID_KEY entry.  They are removed from the hash table and
-	 * linked in to a private working to be destroyed.
+	 * linked in to a private working list to be destroyed.
 	 */
+
         while (!list_empty(&pid_entry->he_pid_list)) {
 		entry = list_entry(pid_entry->he_pid_list.next,
 				   tsd_hash_entry_t, he_pid_list);
 		ASSERT3U(pid_entry->he_pid, ==, entry->he_pid);
+
+		hash = hash_long((ulong_t)entry->he_key *
+		    (ulong_t)entry->he_pid, table->ht_bits);
+		entry_bin = &table->ht_bins[hash];
+
+		spin_lock(&entry_bin->hb_lock);
 		tsd_hash_del(table, entry);
 		hlist_add_head(&entry->he_list, &work);
+		spin_unlock(&entry_bin->hb_lock);
 	}
 
+	hash = hash_long((ulong_t)pid_entry->he_key *
+	    (ulong_t)pid_entry->he_pid, table->ht_bits);
+	pid_entry_bin = &table->ht_bins[hash];
+
+	spin_lock(&pid_entry_bin->hb_lock);
 	tsd_hash_del(table, pid_entry);
 	hlist_add_head(&pid_entry->he_list, &work);
+	spin_unlock(&pid_entry_bin->hb_lock);
 	spin_unlock(&table->ht_lock);
 
 	tsd_hash_dtor(&work);