Re: net: Fix skb csum races when peeking

2015-07-15 Thread David Miller
From: Herbert Xu herb...@gondor.apana.org.au
Date: Mon, 13 Jul 2015 20:01:42 +0800

 When we calculate the checksum on the recv path, we store the
 result in the skb as an optimisation in case we need the checksum
 again down the line.
 
 This is in fact bogus for the MSG_PEEK case as this is done without
 any locking.  So multiple threads can peek and then store the result
 to the same skb, potentially resulting in bogus skb states.
 
 This patch fixes this by only storing the result if the skb is not
 shared.  This preserves the optimisations for the few cases where
 it can be done safely due to locking or other reasons, e.g., SIOCINQ.
 
 Signed-off-by: Herbert Xu herb...@gondor.apana.org.au

Also applied and queued up for -stable, thanks!
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: net: Fix skb csum races when peeking

2015-07-14 Thread Eric Dumazet
On Mon, 2015-07-13 at 20:01 +0800, Herbert Xu wrote:

 ---8---
 When we calculate the checksum on the recv path, we store the
 result in the skb as an optimisation in case we need the checksum
 again down the line.
 
 This is in fact bogus for the MSG_PEEK case as this is done without
 any locking.  So multiple threads can peek and then store the result
 to the same skb, potentially resulting in bogus skb states.
 
 This patch fixes this by only storing the result if the skb is not
 shared.  This preserves the optimisations for the few cases where
 it can be done safely due to locking or other reasons, e.g., SIOCINQ.
 
 Signed-off-by: Herbert Xu herb...@gondor.apana.org.au
 
 diff --git a/net/core/datagram.c b/net/core/datagram.c
 index b80fb91..4967262 100644
 --- a/net/core/datagram.c
 +++ b/net/core/datagram.c
 @@ -622,7 +657,8 @@ __sum16 __skb_checksum_complete_head(struct sk_buff *skb, 
 int len)
   !skb-csum_complete_sw)
   netdev_rx_csum_fault(skb-dev);
   }
 - skb-csum_valid = !sum;
 + if (!skb_shared(skb))
 + skb-csum_valid = !sum;
   return sum;
  }
  EXPORT_SYMBOL(__skb_checksum_complete_head);
 @@ -642,11 +678,13 @@ __sum16 __skb_checksum_complete(struct sk_buff *skb)
   netdev_rx_csum_fault(skb-dev);
   }
  
 - /* Save full packet checksum */
 - skb-csum = csum;
 - skb-ip_summed = CHECKSUM_COMPLETE;
 - skb-csum_complete_sw = 1;
 - skb-csum_valid = !sum;
 + if (!skb_shared(skb)) {
 + /* Save full packet checksum */
 + skb-csum = csum;
 + skb-ip_summed = CHECKSUM_COMPLETE;
 + skb-csum_complete_sw = 1;
 + skb-csum_valid = !sum;
 + }
  
   return sum;
  }

Acked-by: Eric Dumazet eduma...@google.com

Thanks !


--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: net: Fix skb csum races when peeking

2015-07-13 Thread Herbert Xu
On Mon, Jul 13, 2015 at 08:01:42PM +0800, Herbert Xu wrote:
 
 PS we seem to no longer use the hardware checksum in case of
 CHECKSUM_COMPLETE, I wonder why that is?

Nevermind, it's still there.  I was just looking in the wrong place.
-- 
Email: Herbert Xu herb...@gondor.apana.org.au
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html