BUG in 2.2.22 skb_realloc_headroom()

BUG in 2.2.22 skb_realloc_headroom()

[Nicolas S. Dade]

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

Kernel: 2.2.22
File: net/core/skbuff.c

skb_realloc_headroom() panics when new headroom is smaller
than existing headroom. Specifically the skb_put() fails
and calls skb_over_panic() because the new buffer is too
small.

When skb_realloc_headroom() is called from skb_cow(), it
can be called when the existing headroom size is >=
the desired headroom but the packet in question is cloned.

Then skb_realloc_headroom() allocates

 skb_alloc( skb->truesize + new_headroom - old_headroom )

but if the old_headroom > new_headroom then the resulting
buffer is too small to hold new_headroom + skb->len.

I found this when running tethereal (thus causing the packets
to be cloned for libpcap) and passing data from an acenic,
which allocates 48 bytes of headroom in its skbuff's, to
another ethernet device, which needs only 14 (rounded to 16)
bytes of headroom for the ethernet header.

Here's how I think it should be fixed:

-- 
-- Nicolas Dade    http://nsd.dyndns.org/

[-- Attachment #2: skb_realloc_headroom.diff --]
[-- Type: text/plain, Size: 750 bytes --]

--- linux-2.2.22/net/core/skbuff.c.orig	Tue Oct 29 21:13:24 2002
+++ linux-2.2.22/net/core/skbuff.c	Tue Oct 29 21:15:14 2002
@@ -7,6 +7,7 @@
  *	Version:	$Id: skbuff.c,v 1.55 1999/02/23 08:12:27 davem Exp $
  *
  *	Fixes:	
+ *              Nicolas Dade    :       Fixed skb_realloc_headroom to smaller headroom
  *		Alan Cox	:	Fixed the worst of the load balancer bugs.
  *		Dave Platt	:	Interrupt stacking fix.
  *	Richard Kooijman	:	Timestamp fixes.
@@ -316,13 +317,12 @@
 {
 	struct sk_buff *n;
 	unsigned long offset;
-	int headroom = skb_headroom(skb);
 
 	/*
 	 *	Allocate the copy buffer
 	 */
  	 
-	n=alloc_skb(skb->truesize+newheadroom-headroom, GFP_ATOMIC);
+	n=alloc_skb(skb->len+newheadroom, GFP_ATOMIC);
 	if(n==NULL)
 		return NULL;
 

Re: BUG in 2.2.22 skb_realloc_headroom()

[James Morris]

On Tue, 29 Oct 2002, Nicolas S. Dade wrote:

> skb_realloc_headroom() panics when new headroom is smaller
> than existing headroom.

Would you please test out the patch below?

Thanks,

- James
-- 
James Morris
<jmorris@intercode.com.au>


diff -urN -X dontdiff linux-2.2.22.orig/net/core/skbuff.c linux-2.2.22.skbrealloc/net/core/skbuff.c
--- linux-2.2.22.orig/net/core/skbuff.c	Wed Sep 25 00:06:26 2002
+++ linux-2.2.22.skbrealloc/net/core/skbuff.c	Wed Oct 30 21:25:02 2002
@@ -316,13 +316,16 @@
 {
 	struct sk_buff *n;
 	unsigned long offset;
-	int headroom = skb_headroom(skb);
+	int delta = newheadroom - skb_headroom(skb);
+
+	if (delta <= 0)
+		delta = 0;
 
 	/*
 	 *	Allocate the copy buffer
 	 */
  	 
-	n=alloc_skb(skb->truesize+newheadroom-headroom, GFP_ATOMIC);
+	n=alloc_skb(skb->truesize + delta, GFP_ATOMIC);
 	if(n==NULL)
 		return NULL;
 

Re: BUG in 2.2.22 skb_realloc_headroom()

[Nicolas S. Dade]

On Wed, Oct 30, 2002 at 09:36:11PM +1100, James Morris wrote:
> On Tue, 29 Oct 2002, Nicolas S. Dade wrote:
> 
> > skb_realloc_headroom() panics when new headroom is smaller
> > than existing headroom.
> 
> Would you please test out the patch below?

It works.

It seems a pity to allocate more than needed when
realloc'ing to a smaller headroom; that's why I
used skb->len+headroom as the new length, but
you know this already.

-- 
-- Nicolas Dade    http://nsd.dyndns.org/