Re: skb_ip_make_writable and skbs not owned by a socket

[Patrick McHardy <kaber@trash.net>]

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

Patrick McHardy wrote:

> A different question is why we can't simply do this:
>
> (cut-n-paste, I would cut at least one goto in a patch)
>
> int skb_ip_make_writable(struct sk_buff **pskb, unsigned int 
> writable_len)
> {
>        struct sk_buff *nskb;
>
>        if (writable_len > (*pskb)->len)
>                return 0;
>
>        /* Not exclusive use of packet?  Must copy. */
>        if (skb_shared(*pskb) || skb_cloned(*pskb))
>                goto copy_skb;
>
>        if (skb_headlen(skb) <= writable_len)
>                return 1;

This should have been >=, but is not necessary anyway, pskb_may_pull
already does the same test. Real patch attached.



[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1591 bytes --]

===== net/core/netfilter.c 1.37 vs edited =====
--- 1.37/net/core/netfilter.c	2004-11-13 14:41:07 +01:00
+++ edited/net/core/netfilter.c	2005-01-03 02:50:29 +01:00
@@ -678,7 +678,6 @@
 int skb_ip_make_writable(struct sk_buff **pskb, unsigned int writable_len)
 {
 	struct sk_buff *nskb;
-	unsigned int iplen;
 
 	if (writable_len > (*pskb)->len)
 		return 0;
@@ -687,35 +686,7 @@
 	if (skb_shared(*pskb) || skb_cloned(*pskb))
 		goto copy_skb;
 
-	/* Alexey says IP hdr is always modifiable and linear, so ok. */
-	if (writable_len <= (*pskb)->nh.iph->ihl*4)
-		return 1;
-
-	iplen = writable_len - (*pskb)->nh.iph->ihl*4;
-
-	/* DaveM says protocol headers are also modifiable. */
-	switch ((*pskb)->nh.iph->protocol) {
-	case IPPROTO_TCP: {
-		struct tcphdr _hdr, *hp;
-		hp = skb_header_pointer(*pskb, (*pskb)->nh.iph->ihl*4,
-					sizeof(_hdr), &_hdr);
-		if (hp == NULL)
-			goto copy_skb;
-		if (writable_len <= (*pskb)->nh.iph->ihl*4 + hp->doff*4)
-			goto pull_skb;
-		goto copy_skb;
-	}
-	case IPPROTO_UDP:
-		if (writable_len<=(*pskb)->nh.iph->ihl*4+sizeof(struct udphdr))
-			goto pull_skb;
-		goto copy_skb;
-	case IPPROTO_ICMP:
-		if (writable_len
-		    <= (*pskb)->nh.iph->ihl*4 + sizeof(struct icmphdr))
-			goto pull_skb;
-		goto copy_skb;
-	/* Insert other cases here as desired */
-	}
+	return pskb_may_pull(*pskb, writable_len);
 
 copy_skb:
 	nskb = skb_copy(*pskb, GFP_ATOMIC);
@@ -730,9 +701,6 @@
 	kfree_skb(*pskb);
 	*pskb = nskb;
 	return 1;
-
-pull_skb:
-	return pskb_may_pull(*pskb, writable_len);
 }
 EXPORT_SYMBOL(skb_ip_make_writable);
 #endif /*CONFIG_INET*/