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

login
register
mail settings
Submitter Herbert Xu
Date June 4, 2009, 11:22 a.m.
Message ID <20090604112201.GA28820@gondor.apana.org.au>
Download mbox | patch
Permalink /patch/28099/
State Accepted
Delegated to: David Miller
Headers show

Comments

Herbert Xu - June 4, 2009, 11:22 a.m.
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>


Cheers,
Rusty Russell - June 5, 2009, 4:22 a.m.
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.
--
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
David Miller - June 8, 2009, 7:23 a.m.
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.
--
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

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;