Message ID | 1451483472.8255.16.camel@edumazet-glaptop2.roam.corp.google.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Wed, Dec 30, 2015 at 08:51:12AM -0500, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > Backport of this upstream commit into stable kernels : > 89c22d8c3b27 ("net: Fix skb csum races when peeking") > exposed a bug in udp stack vs MSG_PEEK support, when user provides > a buffer smaller than skb payload. > > In this case, > skb_copy_and_csum_datagram_iovec(skb, sizeof(struct udphdr), > msg->msg_iov); > returns -EFAULT. > > This bug does not happen in upstream kernels since Al Viro did a great > job to replace this into : > skb_copy_and_csum_datagram_msg(skb, sizeof(struct udphdr), msg); > This variant is safe vs short buffers. > > For the time being, instead reverting Herbert Xu patch and add back > skb->ip_summed invalid changes, simply store the result of > udp_lib_checksum_complete() so that we avoid computing the checksum a > second time, and avoid the problematic > skb_copy_and_csum_datagram_iovec() call. > > This patch can be applied on recent kernels as it avoids a double > checksumming, then backported to stable kernels as a bug fix. > > Signed-off-by: Eric Dumazet <edumazet@google.com> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Wed, 30 Dec 2015 08:51:12 -0500 > From: Eric Dumazet <edumazet@google.com> > > Backport of this upstream commit into stable kernels : > 89c22d8c3b27 ("net: Fix skb csum races when peeking") > exposed a bug in udp stack vs MSG_PEEK support, when user provides > a buffer smaller than skb payload. > > In this case, > skb_copy_and_csum_datagram_iovec(skb, sizeof(struct udphdr), > msg->msg_iov); > returns -EFAULT. > > This bug does not happen in upstream kernels since Al Viro did a great > job to replace this into : > skb_copy_and_csum_datagram_msg(skb, sizeof(struct udphdr), msg); > This variant is safe vs short buffers. > > For the time being, instead reverting Herbert Xu patch and add back > skb->ip_summed invalid changes, simply store the result of > udp_lib_checksum_complete() so that we avoid computing the checksum a > second time, and avoid the problematic > skb_copy_and_csum_datagram_iovec() call. > > This patch can be applied on recent kernels as it avoids a double > checksumming, then backported to stable kernels as a bug fix. > > Signed-off-by: Eric Dumazet <edumazet@google.com> Applied, thanks Eric. -- 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/ipv4/udp.c b/net/ipv4/udp.c index 8841e984f8bf..ac14ae44390d 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -1271,6 +1271,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock, int peeked, off = 0; int err; int is_udplite = IS_UDPLITE(sk); + bool checksum_valid = false; bool slow; if (flags & MSG_ERRQUEUE) @@ -1296,11 +1297,12 @@ try_again: */ if (copied < ulen || UDP_SKB_CB(skb)->partial_cov) { - if (udp_lib_checksum_complete(skb)) + checksum_valid = !udp_lib_checksum_complete(skb); + if (!checksum_valid) goto csum_copy_err; } - if (skb_csum_unnecessary(skb)) + if (checksum_valid || skb_csum_unnecessary(skb)) err = skb_copy_datagram_msg(skb, sizeof(struct udphdr), msg, copied); else { diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index 9da3287a3923..00775ee27d86 100644 --- a/net/ipv6/udp.c +++ b/net/ipv6/udp.c @@ -402,6 +402,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int peeked, off = 0; int err; int is_udplite = IS_UDPLITE(sk); + bool checksum_valid = false; int is_udp4; bool slow; @@ -433,11 +434,12 @@ try_again: */ if (copied < ulen || UDP_SKB_CB(skb)->partial_cov) { - if (udp_lib_checksum_complete(skb)) + checksum_valid = !udp_lib_checksum_complete(skb); + if (!checksum_valid) goto csum_copy_err; } - if (skb_csum_unnecessary(skb)) + if (checksum_valid || skb_csum_unnecessary(skb)) err = skb_copy_datagram_msg(skb, sizeof(struct udphdr), msg, copied); else {