[PATCH] fix stats in __ip_conntrack_confirm

[PATCH] fix stats in __ip_conntrack_confirm

[Pablo Neira]

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

Hi,

I've been discussing this with Eric. __ip_conntrack_confirm is entered from
both process context and net_rx_softirq context and "++" operations aren't
atomic.

So net_rx_softirq can preempt the calling process while incrementing the 
stats.
I think that we can fix this moving both CONNTRACK_STAT_INC to the locked
section.

regards,
Pablo

Signed-off-by: Pablo Neira Ayuso <pablo@eurodev.net>

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 610 bytes --]

===== net/ipv4/netfilter/ip_conntrack_core.c 1.62 vs edited =====
--- 1.62/net/ipv4/netfilter/ip_conntrack_core.c	Mon Oct  4 01:23:58 2004
+++ edited/net/ipv4/netfilter/ip_conntrack_core.c	Mon Oct 18 21:53:02 2004
@@ -490,13 +490,14 @@
 		add_timer(&ct->timeout);
 		atomic_inc(&ct->ct_general.use);
 		set_bit(IPS_CONFIRMED_BIT, &ct->status);
-		WRITE_UNLOCK(&ip_conntrack_lock);
 		CONNTRACK_STAT_INC(insert);
+		WRITE_UNLOCK(&ip_conntrack_lock);
 		return NF_ACCEPT;
 	}
 
-	WRITE_UNLOCK(&ip_conntrack_lock);
 	CONNTRACK_STAT_INC(insert_failed);
+	WRITE_UNLOCK(&ip_conntrack_lock);
+
 	return NF_DROP;
 }
 

Re: [PATCH] fix stats in __ip_conntrack_confirm

[Patrick McHardy]

Pablo Neira wrote:

> So net_rx_softirq can preempt the calling process while incrementing 
> the stats.
> I think that we can fix this moving both CONNTRACK_STAT_INC to the locked
> section.

Applied, thanks Pablo.

Regards
Patrick