tun: Optimise handling of bogus gso->hdr_len

tun: Optimise handling of bogus gso->hdr_len

[Herbert Xu]

Hi:

tun: Optimise handling of bogus gso->hdr_len

As all current versions of virtio_net generate a value for the
header length that's too small, we should optimise this so that
we don't copy it twice.  This can be done by ensuring that it is
at least as large as the place where we'll write the checksum.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 735bf41..00d00ad 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -563,6 +563,10 @@ static __inline__ ssize_t tun_get_user(struct tun_struct *tun,
 		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
 
+		if ((gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
+		    gso.csum_start + gso.csum_offset + 2 > gso.hdr_len)
+			gso.hdr_len = gso.csum_start + gso.csum_offset + 2;
+
 		if (gso.hdr_len > len)
 			return -EINVAL;
 	}

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

net: Ensure partial checksum offset is inside the skb head

[Herbert Xu]

On Thu, Jun 04, 2009 at 09:06:00PM +1000, Herbert Xu wrote:
> 
> tun: Optimise handling of bogus gso->hdr_len
> 
> As all current versions of virtio_net generate a value for the
> header length that's too small, we should optimise this so that
> we don't copy it twice.  This can be done by ensuring that it is
> at least as large as the place where we'll write the checksum.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

With this applied we can strengthen the partial checksum check:

net: Ensure partial checksum offset is inside the skb head

In skb_partial_csum_set we check to see if the checksum offset
is within the packet.  However, we really should check that it
is within the skb head as that's the only bit we can modify
without copying.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e505b53..643c03d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3016,12 +3016,12 @@ EXPORT_SYMBOL_GPL(skb_tstamp_tx);
  */
 bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off)
 {
-	if (unlikely(start > skb->len - 2) ||
-	    unlikely((int)start + off > skb->len - 2)) {
+	if (unlikely(start > skb_headlen(skb)) ||
+	    unlikely((int)start + off > skb_headlen(skb) - 2)) {
 		if (net_ratelimit())
 			printk(KERN_WARNING
 			       "bad partial csum: csum=%u/%u len=%u\n",
-			       start, off, skb->len);
+			       start, off, skb_headlen(skb));
 		return false;
 	}
 	skb->ip_summed = CHECKSUM_PARTIAL;

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: net: Ensure partial checksum offset is inside the skb head

[Rusty Russell]

On Thu, 4 Jun 2009 08:52:01 pm Herbert Xu wrote:
> On Thu, Jun 04, 2009 at 09:06:00PM +1000, Herbert Xu wrote:
> > tun: Optimise handling of bogus gso->hdr_len
> >
> > As all current versions of virtio_net generate a value for the
> > header length that's too small, we should optimise this so that
> > we don't copy it twice.  This can be done by ensuring that it is
> > at least as large as the place where we'll write the checksum.
> >
> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> With this applied we can strengthen the partial checksum check:
>
> net: Ensure partial checksum offset is inside the skb head
>
> In skb_partial_csum_set we check to see if the checksum offset
> is within the packet.  However, we really should check that it
> is within the skb head as that's the only bit we can modify
> without copying.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Yep, makes sense.  FWIW:

Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Thanks!
Rusty.

Re: net: Ensure partial checksum offset is inside the skb head

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 4 Jun 2009 21:22:01 +1000

> On Thu, Jun 04, 2009 at 09:06:00PM +1000, Herbert Xu wrote:
>> 
>> tun: Optimise handling of bogus gso->hdr_len
>> 
>> As all current versions of virtio_net generate a value for the
>> header length that's too small, we should optimise this so that
>> we don't copy it twice.  This can be done by ensuring that it is
>> at least as large as the place where we'll write the checksum.
>> 
>> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> With this applied we can strengthen the partial checksum check:
> 
> net: Ensure partial checksum offset is inside the skb head
> 
> In skb_partial_csum_set we check to see if the checksum offset
> is within the packet.  However, we really should check that it
> is within the skb head as that's the only bit we can modify
> without copying.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied.

Re: tun: Optimise handling of bogus gso->hdr_len

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 4 Jun 2009 21:06:00 +1000

> Hi:
> 
> tun: Optimise handling of bogus gso->hdr_len
> 
> As all current versions of virtio_net generate a value for the
> header length that's too small, we should optimise this so that
> we don't copy it twice.  This can be done by ensuring that it is
> at least as large as the place where we'll write the checksum.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied.