Re: Bridge + Conntrack + SKB Recycle: Fragment Reassembly Errors

[Patrick McHardy <kaber@trash.net>]

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

ben@bigfootnetworks.com wrote:
>> Ben, please give this patch a try.
> 
> I have not been able to recreate the issue after applying the patch,
> which is great.

Thanks for testing.

> Is this the only case in which large-ish SKBs might be
> recycled and cause the reassembly overflow?

I'm not aware of any other cases at least.

Dave, attached is the patch again with a proper changelog.


[-- Attachment #2: 01.diff --]
[-- Type: text/x-patch, Size: 1421 bytes --]

commit d45f8b9ff2b7c1c5787348a39d3778931beca7e3
Author: Patrick McHardy <kaber@trash.net>
Date:   Tue Dec 1 17:19:14 2009 +0100

    ip_fragment: also adjust skb->truesize for packets not owned by a socket
    
    When a large packet gets reassembled by ip_defrag(), the head skb
    accounts for all the fragments in skb->truesize. If this packet is
    refragmented again, skb->truesize is not re-adjusted to reflect only
    the head size since its not owned by a socket. If the head fragment
    then gets recycled and reused for another received fragment, it might
    exceed the defragmentation limits due to its large truesize value.
    
    skb_recycle_check() explicitly checks for linear skbs, so any recycled
    skb should reflect its true size in skb->truesize. Change ip_fragment()
    to also adjust the truesize value of skbs not owned by a socket.
    
    Reported-and-tested-by: Ben Menchaca <ben@bigfootnetworks.com>
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index b78e615..e34013a 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -503,8 +503,8 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 			if (skb->sk) {
 				frag->sk = skb->sk;
 				frag->destructor = sock_wfree;
-				truesizes += frag->truesize;
 			}
+			truesizes += frag->truesize;
 		}
 
 		/* Everything is OK. Generate! */