Message ID | 20150713120142.GA9787@gondor.apana.org.au |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
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.
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 <herbert@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 <edumazet@google.com> Thanks ! -- 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
From: Herbert Xu <herbert@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 <herbert@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }