memory leak related to bridging, conntrack and frags in 2.6.0

memory leak related to bridging, conntrack and frags in 2.6.0

[Steve Hill]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1040 bytes --]


Sorry for the cross-post - I thought this would be of interest to all the 
lists and also wasn't sure where the best people to help hang out.

With both conntrack and bridging turned on in the 2.6.0test11 kernel, 
sending fragmented packets over the bridge reveals a memory leak 
(specifically, forwarding packets from any interface to a bridge).  The 
memory that is leaking seems to be being allocated on line 299 on 
net/bridge/br_netfilter.c:

        if ((nf_bridge = nf_bridge_alloc(skb)) == NULL)
                return NF_DROP;
  
Only the first fragment gets freed later on.

The patch attached fixes the problem by freeing nf_bridge when the 
packets are defragmented, however I am sure this is not the right place 
to do this.  Where would the skb's for the fragments usually get freed?

-- 

- Steve Hill
Senior Software Developer                        Email: steve@navaho.co.uk
Navaho Technologies Ltd.                           Tel: +44-870-7034015

        ... Alcohol and calculus don't mix - Don't drink and derive! ...


[-- Attachment #2: Type: TEXT/PLAIN, Size: 640 bytes --]

diff -urN linux-2.6.0-test11.vanilla/net/ipv4/ip_fragment.c linux-2.6.0-test11.bridge/net/ipv4/ip_fragment.c
--- linux-2.6.0-test11.vanilla/net/ipv4/ip_fragment.c	2003-12-12 19:27:07.000000000 +0000
+++ linux-2.6.0-test11.bridge/net/ipv4/ip_fragment.c	2003-12-12 19:42:50.000000000 +0000
@@ -592,6 +592,12 @@
 	atomic_sub(head->truesize, &ip_frag_mem);
 
 	for (fp=head->next; fp; fp = fp->next) {
+#ifdef CONFIG_BRIDGE_NETFILTER
+		if (fp->nf_bridge) {
+			nf_bridge_put(fp->nf_bridge);
+			fp->nf_bridge = NULL;
+		};
+#endif
 		head->data_len += fp->len;
 		head->len += fp->len;
 		if (head->ip_summed != fp->ip_summed)

Re: memory leak related to bridging, conntrack and frags in 2.6.0

[Bart De Schuymer]

On Friday 12 December 2003 20:55, Steve Hill wrote:
> Sorry for the cross-post - I thought this would be of interest to all the
> lists and also wasn't sure where the best people to help hang out.

The right mailing list for this is netdev@oss.sgi.com.

> With both conntrack and bridging turned on in the 2.6.0test11 kernel,
> sending fragmented packets over the bridge reveals a memory leak
> (specifically, forwarding packets from any interface to a bridge).  The
> memory that is leaking seems to be being allocated on line 299 on
> net/bridge/br_netfilter.c:
>
>         if ((nf_bridge = nf_bridge_alloc(skb)) == NULL)
>                 return NF_DROP;

Thanks for the good diagnose.

> Only the first fragment gets freed later on.
>
> The patch attached fixes the problem by freeing nf_bridge when the
> packets are defragmented, however I am sure this is not the right place
> to do this.  Where would the skb's for the fragments usually get freed?

I believe they are freed in skbuff.c::skb_release_data().

I think the place where you've put the fix doesn't cover all cases. If 
ip_frag_destroy() is called, there will still be a memory leak. So I think 
the right place for this is in skb_release_data. But consulting with the 
netdev list seems appropriate :)

You can change
+		if (fp->nf_bridge) {
+			nf_bridge_put(fp->nf_bridge);
+			fp->nf_bridge = NULL;
+		};
into
+		nf_bridge_put(fp->nf_bridge);

cheers,
Bart