Skip to contentSkip to conversation

: Don't call ip_conntrack_put with ip_conntrack_lock held

PATCH In review Patrick McHardy 2 participants 2 messages Sep 23, 2003 – Sep 24, 2003

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 : Don't call ip_conntrack_put with ip_conntrack_lock held

This patch changes destroy_conntrack so ip_conntrack_put for the master conntrack is called without the ip_conntrack_lock held. There is currently no situation in which destroy_conntrack could be called with a conntrack that has a master with a refcount of 1 so it is not a problem now (at least I couldn't spot one) but it seems right anyway.

Best regards,
Patrick

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:
# ChangeSet 1.1136 -> 1.1137
# net/ipv4/netfilter/ip_conntrack_core.c 1.26 -> 1.27
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/09/23 [email protected] 1.1137
# [NETFILTER]: Don't call ip_conntrack_put with ip_conntrack_lock held # --------------------------------------------
#

net/ipv4/netfilter/ip_conntrack_core.c +5 -2
diff -Nru a/net/ipv4/netfilter/ip_conntrack_core.c b/net/ipv4/netfilter/ip_conntrack_core.c
@@ -308,7 +308,7 @@
 static void
 destroy_conntrack(struct nf_conntrack *nfct)
 {
-	struct ip_conntrack *ct = (struct ip_conntrack *)nfct;
+	struct ip_conntrack *ct = (struct ip_conntrack *)nfct, *master = NULL;
 	struct ip_conntrack_protocol *proto;
 
 	DEBUGP("destroy_conntrack(%p)\n", ct);
@@ -335,11 +335,14 @@
 			/* can't call __unexpect_related here,
 			 * since it would screw up expect_list */
 			list_del(&ct->master->expected_list);
-			ip_conntrack_put(ct->master->expectant);
+			master = ct->master->expectant;
 		}
 		kfree(ct->master);
 	}
 	WRITE_UNLOCK(&ip_conntrack_lock);
+
+	if (master)
+		ip_conntrack_put(master);
 
 	DEBUGP("destroy_conntrack: returning ct=%p to slab\n", ct);
 	kmem_cache_free(ip_conntrack_cachep, ct);
#1 lore raw
Quoted 1 line · Patrick McHardy

This patch changes destroy_conntrack so ip_conntrack_put for the master

ok, I've put it in 'pending'.
Thanks again.

Quoted 2 lines

Best regards,
Patrick

Signature · 5 lines
- Harald Welte <[email protected]>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie