Re: 2.6.13-rc4-mm1: iptables DROP crashes the computer

Re: 2.6.13-rc4-mm1: iptables DROP crashes the computer

[Adrian Bunk]

On Sun, Aug 07, 2005 at 07:12:00PM +0200, Espen Fjellvær Olsen wrote:

> After execing "iptables -A INPUT -j DROP" my computer crashes hard. It
> dosent hang immediately, but after a couple of seconds.
> The machine is an amd64, running a clean x86_64 environment.
> uname -a: Linux gentoo 2.6.13-rc4-mm1 #1 PREEMPT Thu Aug 4 01:01:44
> CEST 2005 x86_64 AMD Athlon(tm) 64 Processor 3400+ AuthenticAMD
>...

Is this reproducible or did it happen only once?

Are there any messages that might give a hint where to search for the 
problem?

You are reporting this against 2.6.13-rc4-mm1, but are attaching a 
.config of 2.6.13-rc5-mm1. Which kernels are affected, and which are 
not?

Does it still happen if you compile your kernel with preemption 
disabled?

Please send the output of ./scripts/ver_linux .

> Mvh / Best regards
> Espen Fjellvær Olsen

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: 2.6.13-rc4-mm1: iptables DROP crashes the computer

[Espen Fjellvær Olsen]

On 07/08/05, Adrian Bunk <bunk@stusta.de> wrote:
> On Sun, Aug 07, 2005 at 07:12:00PM +0200, Espen Fjellvær Olsen wrote:
> 
> > After execing "iptables -A INPUT -j DROP" my computer crashes hard. It
> > dosent hang immediately, but after a couple of seconds.
> > The machine is an amd64, running a clean x86_64 environment.
> > uname -a: Linux gentoo 2.6.13-rc4-mm1 #1 PREEMPT Thu Aug 4 01:01:44
> > CEST 2005 x86_64 AMD Athlon(tm) 64 Processor 3400+ AuthenticAMD
> >...
> 
> Is this reproducible or did it happen only once?

It is reproducible, happens each time when running "iptables -A INPUT
-j DROP", other rules like "iptables -A INPUT -m --state
ESTABLISHED,RELATED -p tcp --dport 22 -j ACCEPT" works well tho.

> Are there any messages that might give a hint where to search for the
> problem?

The kernel log dont give any messages before the crash, and since the
computer crash hard i cant check for relevant messages after the crash
;)
 
> You are reporting this against 2.6.13-rc4-mm1, but are attaching a
> .config of 2.6.13-rc5-mm1. Which kernels are affected, and which are
> not?

Im sorry about that glitch, recently compiled 2.6.13-rc5-mm1, but i
got a kernel panic at boot, havent looked into that yet, but it might
be related to ACPI.

The config for rc4-mm1 and rc5-mm1 is similar.

> Does it still happen if you compile your kernel with preemption
> disabled?

Havent tried this yet, but ill do it right away.

> Please send the output of ./scripts/ver_linux .

If some fields are empty or look unusual you may have an old version.
Compare to the current minimal requirements in Documentation/Changes.

Linux gentoo 2.6.13-rc4-mm1 #1 PREEMPT Thu Aug 4 01:01:44 CEST 2005
x86_64 AMD Athlon(tm) 64 Processor 3400+ AuthenticAMD GNU/Linux

Gnu C                  3.4.4
Gnu make               3.80
binutils               2.16.1
util-linux             2.12q
mount                  2.12q
module-init-tools      3.2-pre7
e2fsprogs              1.38
reiserfsprogs          line
reiser4progs           line
xfsprogs               2.6.25
Linux C Library        2.3.5
Dynamic linker (ldd)   2.3.5
Procps                 3.2.5
Net-tools              1.60
Kbd                    1.12
Sh-utils               5.2.1
udev                   065
Modules Loaded         iptable_filter ip_tables snd_seq_midi
snd_emu10k1_synth snd_emux_synth snd_seq_virmidi snd_seq_midi_event
snd_seq_midi_emul snd_seq snd_pcm_oss snd_mixer_oss rtc ntfs
snd_emu10k1 snd_rawmidi snd_seq_device snd_ac97_codec snd_pcm
snd_timer snd_page_alloc snd_util_mem snd_hwdep snd soundcore
usb_storage uhci_hcd usbcore


-- 
Mvh / Best regards
Espen Fjellvær Olsen
espenfjo@gmail.com
Norway

Re: 2.6.13-rc4-mm1: iptables DROP crashes the computer

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

Adrian Bunk wrote:
> On Sun, Aug 07, 2005 at 07:12:00PM +0200, Espen Fjellvær Olsen wrote:
> 
> 
>>After execing "iptables -A INPUT -j DROP" my computer crashes hard. It
>>dosent hang immediately, but after a couple of seconds.
>>The machine is an amd64, running a clean x86_64 environment.
>>uname -a: Linux gentoo 2.6.13-rc4-mm1 #1 PREEMPT Thu Aug 4 01:01:44
>>CEST 2005 x86_64 AMD Athlon(tm) 64 Processor 3400+ AuthenticAMD
>>...

Could be related to the refcnt underflow with conntrack event
notifications enabled. If you have CONFIG_IP_NF_CONNTRACK_EVENTS
enabled please try this patch.


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 10692 bytes --]

[NETFILTER]: Fix multiple problems with the conntrack event cache

refcnt underflow: the reference count is decremented when a conntrack
entry is removed from the hash but it is not incremented when entering
new entries.

missing protection of process context against softirq context: all
cache operations need to locally disable softirqs to avoid races.
Additionally the event cache can't be initialized when a packet enteres
the conntrack code but needs to be initialized whenever we cache an event
and the stored conntrack entry doesn't match the current one.

incorrect flushing of the event cache in ip_ct_iterate_cleanup: without
real locking we can't flush the cache for different CPUs without incurring
races. The cache for different CPUs can only be flushed when no packets
are going through the code. ip_ct_iterate_cleanup doesn't need to drop
all references, so flushing is moved to the cleanup path.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit d20d18715b8425060334e879ebb4835202457b3e
tree 3279243f59162c14f8ac145473e1c3f4c586b2fa
parent df2e0392536ecdd6385f4319f746045fd6fae38f
author Patrick McHardy <kaber@trash.net> Wed, 03 Aug 2005 20:33:25 +0200
committer Patrick McHardy <kaber@trash.net> Wed, 03 Aug 2005 20:33:25 +0200

 include/linux/netfilter_ipv4/ip_conntrack.h      |   29 +++---
 include/linux/netfilter_ipv4/ip_conntrack_core.h |   14 +--
 net/ipv4/netfilter/ip_conntrack_core.c           |  107 ++++++++--------------
 net/ipv4/netfilter/ip_conntrack_standalone.c     |    3 -
 4 files changed, 58 insertions(+), 95 deletions(-)

diff --git a/include/linux/netfilter_ipv4/ip_conntrack.h b/include/linux/netfilter_ipv4/ip_conntrack.h
--- a/include/linux/netfilter_ipv4/ip_conntrack.h
+++ b/include/linux/netfilter_ipv4/ip_conntrack.h
@@ -411,6 +411,7 @@ struct ip_conntrack_stat
 
 #ifdef CONFIG_IP_NF_CONNTRACK_EVENTS
 #include <linux/notifier.h>
+#include <linux/interrupt.h>
 
 struct ip_conntrack_ecache {
 	struct ip_conntrack *ct;
@@ -445,26 +446,24 @@ ip_conntrack_expect_unregister_notifier(
 	return notifier_chain_unregister(&ip_conntrack_expect_chain, nb);
 }
 
+extern void ip_ct_deliver_cached_events(const struct ip_conntrack *ct);
+extern void __ip_ct_event_cache_init(struct ip_conntrack *ct);
+
 static inline void 
 ip_conntrack_event_cache(enum ip_conntrack_events event,
 			 const struct sk_buff *skb)
 {
-	struct ip_conntrack_ecache *ecache = 
-					&__get_cpu_var(ip_conntrack_ecache);
-
-	if (unlikely((struct ip_conntrack *) skb->nfct != ecache->ct)) {
-		if (net_ratelimit()) {
-			printk(KERN_ERR "ctevent: skb->ct != ecache->ct !!!\n");
-			dump_stack();
-		}
-	}
+	struct ip_conntrack *ct = (struct ip_conntrack *)skb->nfct;
+	struct ip_conntrack_ecache *ecache;
+	
+	local_bh_disable();
+	ecache = &__get_cpu_var(ip_conntrack_ecache);
+	if (ct != ecache->ct)
+		__ip_ct_event_cache_init(ct);
 	ecache->events |= event;
+	local_bh_enable();
 }
 
-extern void 
-ip_conntrack_deliver_cached_events_for(const struct ip_conntrack *ct);
-extern void ip_conntrack_event_cache_init(const struct sk_buff *skb);
-
 static inline void ip_conntrack_event(enum ip_conntrack_events event,
 				      struct ip_conntrack *ct)
 {
@@ -483,9 +482,7 @@ static inline void ip_conntrack_event_ca
 					    const struct sk_buff *skb) {}
 static inline void ip_conntrack_event(enum ip_conntrack_events event, 
 				      struct ip_conntrack *ct) {}
-static inline void ip_conntrack_deliver_cached_events_for(
-						struct ip_conntrack *ct) {}
-static inline void ip_conntrack_event_cache_init(const struct sk_buff *skb) {}
+static inline void ip_ct_deliver_cached_events(const struct ip_conntrack *ct) {}
 static inline void 
 ip_conntrack_expect_event(enum ip_conntrack_expect_events event, 
 			  struct ip_conntrack_expect *exp) {}
diff --git a/include/linux/netfilter_ipv4/ip_conntrack_core.h b/include/linux/netfilter_ipv4/ip_conntrack_core.h
--- a/include/linux/netfilter_ipv4/ip_conntrack_core.h
+++ b/include/linux/netfilter_ipv4/ip_conntrack_core.h
@@ -44,18 +44,14 @@ static inline int ip_conntrack_confirm(s
 	struct ip_conntrack *ct = (struct ip_conntrack *)(*pskb)->nfct;
 	int ret = NF_ACCEPT;
 
-	if (ct && !is_confirmed(ct))
-		ret = __ip_conntrack_confirm(pskb);
-	ip_conntrack_deliver_cached_events_for(ct);
-
+	if (ct) {
+		if (!is_confirmed(ct))
+			ret = __ip_conntrack_confirm(pskb);
+		ip_ct_deliver_cached_events(ct);
+	}
 	return ret;
 }
 
-#ifdef CONFIG_IP_NF_CONNTRACK_EVENTS
-struct ip_conntrack_ecache;
-extern void __ip_ct_deliver_cached_events(struct ip_conntrack_ecache *ec);
-#endif
-
 extern void __ip_ct_expect_unlink_destroy(struct ip_conntrack_expect *exp);
 
 extern struct list_head *ip_conntrack_hash;
diff --git a/net/ipv4/netfilter/ip_conntrack_core.c b/net/ipv4/netfilter/ip_conntrack_core.c
--- a/net/ipv4/netfilter/ip_conntrack_core.c
+++ b/net/ipv4/netfilter/ip_conntrack_core.c
@@ -85,73 +85,62 @@ struct notifier_block *ip_conntrack_expe
 
 DEFINE_PER_CPU(struct ip_conntrack_ecache, ip_conntrack_ecache);
 
-static inline void __deliver_cached_events(struct ip_conntrack_ecache *ecache)
+/* deliver cached events and clear cache entry - must be called with locally
+ * disabled softirqs */
+static inline void
+__ip_ct_deliver_cached_events(struct ip_conntrack_ecache *ecache)
 {
+	DEBUGP("ecache: delivering events for %p\n", ecache->ct);
 	if (is_confirmed(ecache->ct) && !is_dying(ecache->ct) && ecache->events)
 		notifier_call_chain(&ip_conntrack_chain, ecache->events,
 				    ecache->ct);
 	ecache->events = 0;
-}
-
-void __ip_ct_deliver_cached_events(struct ip_conntrack_ecache *ecache)
-{
-	__deliver_cached_events(ecache);
+	ip_conntrack_put(ecache->ct);
+	ecache->ct = NULL;
 }
 
 /* Deliver all cached events for a particular conntrack. This is called
  * by code prior to async packet handling or freeing the skb */
-void 
-ip_conntrack_deliver_cached_events_for(const struct ip_conntrack *ct)
+void ip_ct_deliver_cached_events(const struct ip_conntrack *ct)
 {
-	struct ip_conntrack_ecache *ecache = 
-					&__get_cpu_var(ip_conntrack_ecache);
-
-	if (!ct)
-		return;
-
-	if (ecache->ct == ct) {
-		DEBUGP("ecache: delivering event for %p\n", ct);
-		__deliver_cached_events(ecache);
-	} else {
-		if (net_ratelimit())
-			printk(KERN_WARNING "ecache: want to deliver for %p, "
-				"but cache has %p\n", ct, ecache->ct);
-	}
-
-	/* signalize that events have already been delivered */
-	ecache->ct = NULL;
+	struct ip_conntrack_ecache *ecache;
+	
+	local_bh_disable();
+	ecache = &__get_cpu_var(ip_conntrack_ecache);
+	if (ecache->ct == ct)
+		__ip_ct_deliver_cached_events(ecache);
+	local_bh_enable();
 }
 
-/* Deliver cached events for old pending events, if current conntrack != old */
-void ip_conntrack_event_cache_init(const struct sk_buff *skb)
+void __ip_ct_event_cache_init(struct ip_conntrack *ct)
 {
-	struct ip_conntrack *ct = (struct ip_conntrack *) skb->nfct;
-	struct ip_conntrack_ecache *ecache = 
-					&__get_cpu_var(ip_conntrack_ecache);
+	struct ip_conntrack_ecache *ecache;
 
 	/* take care of delivering potentially old events */
-	if (ecache->ct != ct) {
-		enum ip_conntrack_info ctinfo;
-		/* we have to check, since at startup the cache is NULL */
-		if (likely(ecache->ct)) {
-			DEBUGP("ecache: entered for different conntrack: "
-			       "ecache->ct=%p, skb->nfct=%p. delivering "
-			       "events\n", ecache->ct, ct);
-			__deliver_cached_events(ecache);
+	ecache = &__get_cpu_var(ip_conntrack_ecache);
+	BUG_ON(ecache->ct == ct);
+	if (ecache->ct)
+		__ip_ct_deliver_cached_events(ecache);
+	/* initialize for this conntrack/packet */
+	ecache->ct = ct;
+	nf_conntrack_get(&ct->ct_general);
+}
+
+/* flush the event cache - touches other CPU's data and must not be called while
+ * packets are still passing through the code */
+static void ip_ct_event_cache_flush(void)
+{
+	struct ip_conntrack_ecache *ecache;
+	int cpu;
+
+	for_each_cpu(cpu) {
+		ecache = &per_cpu(ip_conntrack_ecache, cpu);
+		if (ecache->ct)
 			ip_conntrack_put(ecache->ct);
-		} else {
-			DEBUGP("ecache: entered for conntrack %p, "
-				"cache was clean before\n", ct);
-		}
-
-		/* initialize for this conntrack/packet */
-		ecache->ct = ip_conntrack_get(skb, &ctinfo);
-		/* ecache->events cleared by __deliver_cached_devents() */
-	} else {
-		DEBUGP("ecache: re-entered for conntrack %p.\n", ct);
 	}
 }
-
+#else
+static inline void ip_ct_event_cache_flush(void) {}
 #endif /* CONFIG_IP_NF_CONNTRACK_EVENTS */
 
 DEFINE_PER_CPU(struct ip_conntrack_stat, ip_conntrack_stat);
@@ -873,8 +862,6 @@ unsigned int ip_conntrack_in(unsigned in
 
 	IP_NF_ASSERT((*pskb)->nfct);
 
-	ip_conntrack_event_cache_init(*pskb);
-
 	ret = proto->packet(ct, *pskb, ctinfo);
 	if (ret < 0) {
 		/* Invalid: inverse of the return code tells
@@ -1273,23 +1260,6 @@ ip_ct_iterate_cleanup(int (*iter)(struct
 
 		ip_conntrack_put(ct);
 	}
-
-#ifdef CONFIG_IP_NF_CONNTRACK_EVENTS
-	{
-		/* we need to deliver all cached events in order to drop
-		 * the reference counts */
-		int cpu;
-		for_each_cpu(cpu) {
-			struct ip_conntrack_ecache *ecache = 
-					&per_cpu(ip_conntrack_ecache, cpu);
-			if (ecache->ct) {
-				__ip_ct_deliver_cached_events(ecache);
-				ip_conntrack_put(ecache->ct);
-				ecache->ct = NULL;
-			}
-		}
-	}
-#endif
 }
 
 /* Fast function for those who don't want to parse /proc (and I don't
@@ -1376,6 +1346,7 @@ void ip_conntrack_flush()
            delete... */
 	synchronize_net();
 
+	ip_ct_event_cache_flush();
  i_see_dead_people:
 	ip_ct_iterate_cleanup(kill_all, NULL);
 	if (atomic_read(&ip_conntrack_count) != 0) {
diff --git a/net/ipv4/netfilter/ip_conntrack_standalone.c b/net/ipv4/netfilter/ip_conntrack_standalone.c
--- a/net/ipv4/netfilter/ip_conntrack_standalone.c
+++ b/net/ipv4/netfilter/ip_conntrack_standalone.c
@@ -401,7 +401,6 @@ static unsigned int ip_confirm(unsigned 
 			       const struct net_device *out,
 			       int (*okfn)(struct sk_buff *))
 {
-	ip_conntrack_event_cache_init(*pskb);
 	/* We've seen it coming out the other side: confirm it */
 	return ip_conntrack_confirm(pskb);
 }
@@ -419,7 +418,6 @@ static unsigned int ip_conntrack_help(un
 	ct = ip_conntrack_get(*pskb, &ctinfo);
 	if (ct && ct->helper) {
 		unsigned int ret;
-		ip_conntrack_event_cache_init(*pskb);
 		ret = ct->helper->help(pskb, ct, ctinfo);
 		if (ret != NF_ACCEPT)
 			return ret;
@@ -978,6 +976,7 @@ EXPORT_SYMBOL_GPL(ip_conntrack_chain);
 EXPORT_SYMBOL_GPL(ip_conntrack_expect_chain);
 EXPORT_SYMBOL_GPL(ip_conntrack_register_notifier);
 EXPORT_SYMBOL_GPL(ip_conntrack_unregister_notifier);
+EXPORT_SYMBOL_GPL(__ip_ct_event_cache_init);
 EXPORT_PER_CPU_SYMBOL_GPL(ip_conntrack_ecache);
 #endif
 EXPORT_SYMBOL(ip_conntrack_protocol_register);

Re: 2.6.13-rc4-mm1: iptables DROP crashes the computer

[Espen Fjellvær Olsen]

On 07/08/05, Patrick McHardy <kaber@trash.net> wrote:
> Could be related to the refcnt underflow with conntrack event
> notifications enabled. If you have CONFIG_IP_NF_CONNTRACK_EVENTS
> enabled please try this patch.
> 

I can confirm that that patch solved my problems, thank you :)

-- 
Mvh / Best regards
Espen Fjellvær Olsen
espenfjo@gmail.com
Norway