Skip to contentSkip to conversation

netfilter: fix the hash random initializing race

PATCH In review Changli Gao 2 participants 2 messages Aug 20, 2010 – Sep 16, 2010

Subsystems (3)
Networking Network drivers netfilter
lore
Share & tools
lore

Changli Gao

author
#0 lore raw

PATCH v2 1/2 netfilter: fix the hash random initializing race

nf_conntrack_alloc() isn't called with nf_conntrack_lock locked, so hash random initializing code maybe executed more than once on different CPUs.

Signed-off-by Changli Gao <[email protected]>

---
v2: fix the coding style for multi-line comment

 net/netfilter/nf_conntrack_core.c |   19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
net/netfilter/nf_conntrack_core.c +13 -6
index df3eedb..4c0ad9b 100644
@@ -65,8 +65,7 @@ EXPORT_SYMBOL_GPL(nf_conntrack_max);
 DEFINE_PER_CPU(struct nf_conn, nf_conntrack_untracked);
 EXPORT_PER_CPU_SYMBOL(nf_conntrack_untracked);
 
-static int nf_conntrack_hash_rnd_initted;
-static unsigned int nf_conntrack_hash_rnd;
+static unsigned int nf_conntrack_hash_rnd __read_mostly;
 
 static u_int32_t __hash_conntrack(const struct nf_conntrack_tuple *tuple,
 				  u16 zone, unsigned int size, unsigned int rnd)

@@ -574,10 +573,18 @@ struct nf_conn *nf_conntrack_alloc(struct net *net, u16 zone,
 {
 	struct nf_conn *ct;
 
-	if (unlikely(!nf_conntrack_hash_rnd_initted)) {
-		get_random_bytes(&nf_conntrack_hash_rnd,
-				sizeof(nf_conntrack_hash_rnd));
-		nf_conntrack_hash_rnd_initted = 1;
+	if (unlikely(!nf_conntrack_hash_rnd)) {
+		unsigned int rand;
+
+		/*
+		 * Why not initialize nf_conntrack_rnd in a "init()" function ?
+		 * Because there isn't enough entropy when system initializing,
+		 * and we initialize it as late as possible.
+		 */
+		do {
+			get_random_bytes(&rand, sizeof(rand));
+		} while (!rand);
+		cmpxchg(&nf_conntrack_hash_rnd, 0, rand);
 	}
 
 	/* We don't want any race condition at early drop stage */
#1 lore raw
Quoted 3 lines · Changli Gao

nf_conntrack_alloc() isn't called with nf_conntrack_lock locked, so hash random initializing code maybe executed more than once on different CPUs.

Applied, thanks. However please remember to use proper subject lines, we have lots of hashes in netfilter, so without stating that its about nf_conntrack, its hard to relate this to something without reading the full changelog.