Patchwork Bridge + Conntrack + SKB Recycle: Fragment Reassembly Errors

login
register
mail settings
Submitter Patrick McHardy
Date Dec. 1, 2009, 4:24 p.m.
Message ID <4B154337.8060702@trash.net>
Download mbox | patch
Permalink /patch/39923/
State Accepted
Delegated to: David Miller
Headers show

Comments

Patrick McHardy - Dec. 1, 2009, 4:24 p.m.
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.
David Miller - Dec. 1, 2009, 11:54 p.m.
From: Patrick McHardy <kaber@trash.net>
Date: Tue, 01 Dec 2009 17:24:23 +0100

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

Applied to net-2.6, thanks a lot everyone.

I'll queue this up for -stable as well.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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! */