[PATCH stable-3.2 stable-3.12] net: fix checksum check in skb_copy_and_csum_datagram_iovec()

[PATCH stable-3.2 stable-3.12] net: fix checksum check in skb_copy_and_csum_datagram_iovec()

[Michal Kubecek]

Recent fix "net: add length argument to
skb_copy_and_csum_datagram_iovec" added to some pre-3.19 stable
branches, namely

  stable-3.2.y: commit 127500d724f8
  stable-3.12.y: commit 3e1ac3aafbd0

doesn't handle truncated reads correctly. If read length is shorter than
incoming datagram (but non-zero) and first segment of target iovec is
sufficient for read length, skb_copy_and_csum_datagram() is used to copy
checksum the data while copying it. For truncated reads this means only
the copied part is checksummed (rather than the whole datagram) so that
the check almost always fails.

Add checksum of the remaining part so that the proper checksum of the
whole datagram is computed and checked. Special care must be taken if
the copied length is odd.

For zero read length, we don't have to copy anything but we still should
check the checksum so that a peek doesn't return with a datagram which
is invalid and wouldn't be returned by an actual read.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 net/core/datagram.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/net/core/datagram.c b/net/core/datagram.c
index f22f120771ef..af4bf368257c 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -809,13 +809,14 @@ int skb_copy_and_csum_datagram_iovec(struct sk_buff *skb,
 				     int hlen, struct iovec *iov, int len)
 {
 	__wsum csum;
-	int chunk = skb->len - hlen;
+	int full_chunk = skb->len - hlen;
+	int chunk = min_t(int, full_chunk, len);
 
-	if (chunk > len)
-		chunk = len;
-
-	if (!chunk)
+	if (!chunk) {
+		if (__skb_checksum_complete(skb))
+			goto csum_error;
 		return 0;
+	}
 
 	/* Skip filled elements.
 	 * Pretty silly, look at memcpy_toiovec, though 8)
@@ -833,6 +834,21 @@ int skb_copy_and_csum_datagram_iovec(struct sk_buff *skb,
 		if (skb_copy_and_csum_datagram(skb, hlen, iov->iov_base,
 					       chunk, &csum))
 			goto fault;
+		if (full_chunk > chunk) {
+			if (chunk % 2) {
+				__be16 odd = 0;
+
+				if (skb_copy_bits(skb, hlen + chunk,
+						  (char *)&odd + 1, 1))
+					goto fault;
+				csum = add32_with_carry(odd, csum);
+				csum = skb_checksum(skb, hlen + chunk + 1,
+						    full_chunk - chunk - 1,
+						    csum);
+			} else
+				csum = skb_checksum(skb, hlen + chunk,
+						    full_chunk - chunk, csum);
+		}
 		if (csum_fold(csum))
 			goto csum_error;
 		if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE))
-- 
2.6.4

Re: [PATCH stable-3.2 stable-3.12] net: fix checksum check in skb_copy_and_csum_datagram_iovec()

[Sabrina Dubroca]

Hello Michal,

2015-12-28, 15:01:57 +0100, Michal Kubecek wrote:
> Recent fix "net: add length argument to
> skb_copy_and_csum_datagram_iovec" added to some pre-3.19 stable
> branches, namely
> 
>   stable-3.2.y: commit 127500d724f8
>   stable-3.12.y: commit 3e1ac3aafbd0
> 
> doesn't handle truncated reads correctly. If read length is shorter than
> incoming datagram (but non-zero) and first segment of target iovec is
> sufficient for read length, skb_copy_and_csum_datagram() is used to copy
> checksum the data while copying it. For truncated reads this means only
> the copied part is checksummed (rather than the whole datagram) so that
> the check almost always fails.

I just ran into this issue too, sorry I didn't notice it earlier :(


> Add checksum of the remaining part so that the proper checksum of the
> whole datagram is computed and checked. Special care must be taken if
> the copied length is odd.
> 
> For zero read length, we don't have to copy anything but we still should
> check the checksum so that a peek doesn't return with a datagram which
> is invalid and wouldn't be returned by an actual read.
> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> ---
>  net/core/datagram.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index f22f120771ef..af4bf368257c 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -809,13 +809,14 @@ int skb_copy_and_csum_datagram_iovec(struct sk_buff *skb,
>  				     int hlen, struct iovec *iov, int len)
>  {
>  	__wsum csum;
> -	int chunk = skb->len - hlen;
> +	int full_chunk = skb->len - hlen;
> +	int chunk = min_t(int, full_chunk, len);
>  
> -	if (chunk > len)
> -		chunk = len;
> -
> -	if (!chunk)
> +	if (!chunk) {
> +		if (__skb_checksum_complete(skb))
> +			goto csum_error;
>  		return 0;
> +	}
>  
>  	/* Skip filled elements.
>  	 * Pretty silly, look at memcpy_toiovec, though 8)
> @@ -833,6 +834,21 @@ int skb_copy_and_csum_datagram_iovec(struct sk_buff *skb,
>  		if (skb_copy_and_csum_datagram(skb, hlen, iov->iov_base,
>  					       chunk, &csum))
>  			goto fault;
> +		if (full_chunk > chunk) {
> +			if (chunk % 2) {
> +				__be16 odd = 0;
> +
> +				if (skb_copy_bits(skb, hlen + chunk,
> +						  (char *)&odd + 1, 1))
> +					goto fault;
> +				csum = add32_with_carry(odd, csum);
> +				csum = skb_checksum(skb, hlen + chunk + 1,
> +						    full_chunk - chunk - 1,
> +						    csum);
> +			} else
> +				csum = skb_checksum(skb, hlen + chunk,
> +						    full_chunk - chunk, csum);
> +		}
>  		if (csum_fold(csum))
>  			goto csum_error;
>  		if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE))
> -- 
> 2.6.4


This adds quite a bit of complexity.  I am considering a revert of my
buggy patch, and use what Eric Dumazet suggested instead:

https://patchwork.ozlabs.org/patch/543562/

What do you think?
Eric, would you submit your patch formally?


Thanks

-- 
Sabrina

Re: [PATCH stable-3.2 stable-3.12] net: fix checksum check in skb_copy_and_csum_datagram_iovec()

[Michal Kubecek]

On Mon, Dec 28, 2015 at 03:29:42PM +0100, Sabrina Dubroca wrote:
> 2015-12-28, 15:01:57 +0100, Michal Kubecek wrote:
> > Recent fix "net: add length argument to
> > skb_copy_and_csum_datagram_iovec" added to some pre-3.19 stable
> > branches, namely
> > 
> >   stable-3.2.y: commit 127500d724f8
> >   stable-3.12.y: commit 3e1ac3aafbd0
> > 
> > doesn't handle truncated reads correctly. If read length is shorter than
> > incoming datagram (but non-zero) and first segment of target iovec is
> > sufficient for read length, skb_copy_and_csum_datagram() is used to copy
> > checksum the data while copying it. For truncated reads this means only
> > the copied part is checksummed (rather than the whole datagram) so that
> > the check almost always fails.
> 
> I just ran into this issue too, sorry I didn't notice it earlier :(
> 
> > Add checksum of the remaining part so that the proper checksum of the
> > whole datagram is computed and checked. Special care must be taken if
> > the copied length is odd.
> > 
> > For zero read length, we don't have to copy anything but we still should
> > check the checksum so that a peek doesn't return with a datagram which
> > is invalid and wouldn't be returned by an actual read.
> > 
> > Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> > ---
> >  net/core/datagram.c | 26 +++++++++++++++++++++-----
> >  1 file changed, 21 insertions(+), 5 deletions(-)
> > 
> > diff --git a/net/core/datagram.c b/net/core/datagram.c
> > index f22f120771ef..af4bf368257c 100644
> > --- a/net/core/datagram.c
> > +++ b/net/core/datagram.c
> > @@ -809,13 +809,14 @@ int skb_copy_and_csum_datagram_iovec(struct sk_buff *skb,
> >  				     int hlen, struct iovec *iov, int len)
> >  {
> >  	__wsum csum;
> > -	int chunk = skb->len - hlen;
> > +	int full_chunk = skb->len - hlen;
> > +	int chunk = min_t(int, full_chunk, len);
> >  
> > -	if (chunk > len)
> > -		chunk = len;
> > -
> > -	if (!chunk)
> > +	if (!chunk) {
> > +		if (__skb_checksum_complete(skb))
> > +			goto csum_error;
> >  		return 0;
> > +	}
> >  
> >  	/* Skip filled elements.
> >  	 * Pretty silly, look at memcpy_toiovec, though 8)
> > @@ -833,6 +834,21 @@ int skb_copy_and_csum_datagram_iovec(struct sk_buff *skb,
> >  		if (skb_copy_and_csum_datagram(skb, hlen, iov->iov_base,
> >  					       chunk, &csum))
> >  			goto fault;
> > +		if (full_chunk > chunk) {
> > +			if (chunk % 2) {
> > +				__be16 odd = 0;
> > +
> > +				if (skb_copy_bits(skb, hlen + chunk,
> > +						  (char *)&odd + 1, 1))
> > +					goto fault;
> > +				csum = add32_with_carry(odd, csum);
> > +				csum = skb_checksum(skb, hlen + chunk + 1,
> > +						    full_chunk - chunk - 1,
> > +						    csum);
> > +			} else
> > +				csum = skb_checksum(skb, hlen + chunk,
> > +						    full_chunk - chunk, csum);
> > +		}
> >  		if (csum_fold(csum))
> >  			goto csum_error;
> >  		if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE))
> > -- 
> > 2.6.4
> 
> 
> This adds quite a bit of complexity.

I'm not really happy about it either. :-( Most of the complexity comes
from the corner case when one 16-bit word is divided between the copied
part and the rest - but I can't see a nicer way to handle it.

There is another option: in the case of truncated read, we could simply
take the first branch where copying is separated from checksumming. This
would be less efficient, obviously, but I must admit I have no idea how
much.

> I am considering a revert of my
> buggy patch, and use what Eric Dumazet suggested instead:
> 
> https://patchwork.ozlabs.org/patch/543562/
> 
> What do you think?

I believe that would work. I have a little bad feeling about such
solution as it would keep the function broken and just rely on not
hitting it in the case when it matters. But it worked that way for quite
some time so it's probably safe.

                                                          Michal Kubecek

Re: [PATCH stable-3.2 stable-3.12] net: fix checksum check in skb_copy_and_csum_datagram_iovec()

[Eric Dumazet]

On Mon, 2015-12-28 at 16:38 +0100, Michal Kubecek wrote:
> On Mon, Dec 28, 2015 at 03:29:42PM +0100, Sabrina Dubroca wrote:
> > 2015-12-28, 15:01:57 +0100, Michal Kubecek wrote:
> > > Recent fix "net: add length argument to
> > > skb_copy_and_csum_datagram_iovec" added to some pre-3.19 stable
> > > branches, namely
> > > 
> > >   stable-3.2.y: commit 127500d724f8
> > >   stable-3.12.y: commit 3e1ac3aafbd0
> > > 
> > > doesn't handle truncated reads correctly. If read length is shorter than
> > > incoming datagram (but non-zero) and first segment of target iovec is
> > > sufficient for read length, skb_copy_and_csum_datagram() is used to copy
> > > checksum the data while copying it. For truncated reads this means only
> > > the copied part is checksummed (rather than the whole datagram) so that
> > > the check almost always fails.
> > 
> > I just ran into this issue too, sorry I didn't notice it earlier :(
> > 
> > > Add checksum of the remaining part so that the proper checksum of the
> > > whole datagram is computed and checked. Special care must be taken if
> > > the copied length is odd.
> > > 
> > > For zero read length, we don't have to copy anything but we still should
> > > check the checksum so that a peek doesn't return with a datagram which
> > > is invalid and wouldn't be returned by an actual read.
> > > 
> > > Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> > > ---
> > >  net/core/datagram.c | 26 +++++++++++++++++++++-----
> > >  1 file changed, 21 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/net/core/datagram.c b/net/core/datagram.c
> > > index f22f120771ef..af4bf368257c 100644
> > > --- a/net/core/datagram.c
> > > +++ b/net/core/datagram.c
> > > @@ -809,13 +809,14 @@ int skb_copy_and_csum_datagram_iovec(struct sk_buff *skb,
> > >  				     int hlen, struct iovec *iov, int len)
> > >  {
> > >  	__wsum csum;
> > > -	int chunk = skb->len - hlen;
> > > +	int full_chunk = skb->len - hlen;
> > > +	int chunk = min_t(int, full_chunk, len);
> > >  
> > > -	if (chunk > len)
> > > -		chunk = len;
> > > -
> > > -	if (!chunk)
> > > +	if (!chunk) {
> > > +		if (__skb_checksum_complete(skb))
> > > +			goto csum_error;
> > >  		return 0;
> > > +	}
> > >  
> > >  	/* Skip filled elements.
> > >  	 * Pretty silly, look at memcpy_toiovec, though 8)
> > > @@ -833,6 +834,21 @@ int skb_copy_and_csum_datagram_iovec(struct sk_buff *skb,
> > >  		if (skb_copy_and_csum_datagram(skb, hlen, iov->iov_base,
> > >  					       chunk, &csum))
> > >  			goto fault;
> > > +		if (full_chunk > chunk) {
> > > +			if (chunk % 2) {
> > > +				__be16 odd = 0;
> > > +
> > > +				if (skb_copy_bits(skb, hlen + chunk,
> > > +						  (char *)&odd + 1, 1))
> > > +					goto fault;
> > > +				csum = add32_with_carry(odd, csum);
> > > +				csum = skb_checksum(skb, hlen + chunk + 1,
> > > +						    full_chunk - chunk - 1,
> > > +						    csum);
> > > +			} else
> > > +				csum = skb_checksum(skb, hlen + chunk,
> > > +						    full_chunk - chunk, csum);
> > > +		}
> > >  		if (csum_fold(csum))
> > >  			goto csum_error;
> > >  		if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE))
> > > -- 
> > > 2.6.4
> > 
> > 
> > This adds quite a bit of complexity.
> 
> I'm not really happy about it either. :-( Most of the complexity comes
> from the corner case when one 16-bit word is divided between the copied
> part and the rest - but I can't see a nicer way to handle it.
> 
> There is another option: in the case of truncated read, we could simply
> take the first branch where copying is separated from checksumming. This
> would be less efficient, obviously, but I must admit I have no idea how
> much.
> 
> > I am considering a revert of my
> > buggy patch, and use what Eric Dumazet suggested instead:
> > 
> > https://patchwork.ozlabs.org/patch/543562/
> > 
> > What do you think?
> 
> I believe that would work. I have a little bad feeling about such
> solution as it would keep the function broken and just rely on not
> hitting it in the case when it matters. But it worked that way for quite
> some time so it's probably safe.

For the record, this is what we are using here at Google on our prod
kernel.

Sabrina, I certainly can send the patch for net-next kernel, as this
does not fix a bug for current kernels, but backporting it would be
indeed a way to fix the issue for old kernels.

[PATCH net-next] udp: properly support MSG_PEEK with truncated buffers

[Eric Dumazet]

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>
---
 net/ipv4/udp.c |    6 ++++--
 net/ipv6/udp.c |    6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

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 {

Re: [PATCH net-next] udp: properly support MSG_PEEK with truncated buffers

[Herbert Xu]

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>
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH net-next] udp: properly support MSG_PEEK with truncated buffers

[David Miller]

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.

Re: [PATCH stable-3.2 stable-3.12] net: fix checksum check in skb_copy_and_csum_datagram_iovec()

[Jiri Slaby]

On 12/28/2015, 03:01 PM, Michal Kubecek wrote:
> Recent fix "net: add length argument to
> skb_copy_and_csum_datagram_iovec" added to some pre-3.19 stable
> branches, namely
> 
>   stable-3.2.y: commit 127500d724f8
>   stable-3.12.y: commit 3e1ac3aafbd0

Applied this fix to 3.12. Thanks!

> doesn't handle truncated reads correctly. If read length is shorter than
> incoming datagram (but non-zero) and first segment of target iovec is
> sufficient for read length, skb_copy_and_csum_datagram() is used to copy
> checksum the data while copying it. For truncated reads this means only
> the copied part is checksummed (rather than the whole datagram) so that
> the check almost always fails.
> 
> Add checksum of the remaining part so that the proper checksum of the
> whole datagram is computed and checked. Special care must be taken if
> the copied length is odd.
> 
> For zero read length, we don't have to copy anything but we still should
> check the checksum so that a peek doesn't return with a datagram which
> is invalid and wouldn't be returned by an actual read.
> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> ---
>  net/core/datagram.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index f22f120771ef..af4bf368257c 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -809,13 +809,14 @@ int skb_copy_and_csum_datagram_iovec(struct sk_buff *skb,
>  				     int hlen, struct iovec *iov, int len)
>  {
>  	__wsum csum;
> -	int chunk = skb->len - hlen;
> +	int full_chunk = skb->len - hlen;
> +	int chunk = min_t(int, full_chunk, len);
>  
> -	if (chunk > len)
> -		chunk = len;
> -
> -	if (!chunk)
> +	if (!chunk) {
> +		if (__skb_checksum_complete(skb))
> +			goto csum_error;
>  		return 0;
> +	}
>  
>  	/* Skip filled elements.
>  	 * Pretty silly, look at memcpy_toiovec, though 8)
> @@ -833,6 +834,21 @@ int skb_copy_and_csum_datagram_iovec(struct sk_buff *skb,
>  		if (skb_copy_and_csum_datagram(skb, hlen, iov->iov_base,
>  					       chunk, &csum))
>  			goto fault;
> +		if (full_chunk > chunk) {
> +			if (chunk % 2) {
> +				__be16 odd = 0;
> +
> +				if (skb_copy_bits(skb, hlen + chunk,
> +						  (char *)&odd + 1, 1))
> +					goto fault;
> +				csum = add32_with_carry(odd, csum);
> +				csum = skb_checksum(skb, hlen + chunk + 1,
> +						    full_chunk - chunk - 1,
> +						    csum);
> +			} else
> +				csum = skb_checksum(skb, hlen + chunk,
> +						    full_chunk - chunk, csum);
> +		}
>  		if (csum_fold(csum))
>  			goto csum_error;
>  		if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE))
> 


-- 
js
suse labs

Re: [PATCH stable-3.2 stable-3.12] net: fix checksum check in skb_copy_and_csum_datagram_iovec()

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

On Tue, 2016-01-05 at 17:36 +0100, Jiri Slaby wrote:
> On 12/28/2015, 03:01 PM, Michal Kubecek wrote:
> > Recent fix "net: add length argument to
> > skb_copy_and_csum_datagram_iovec" added to some pre-3.19 stable
> > branches, namely
> > 
> >   stable-3.2.y: commit 127500d724f8
> >   stable-3.12.y: commit 3e1ac3aafbd0
> 
> Applied this fix to 3.12. Thanks!
[...]

You don't want this, you want Eric's fix (commit 197c949e7, "udp:
properly support MSG_PEEK with truncated buffers") although that's not
upstream yet.

Ben.

-- 
Ben Hutchings
Tomorrow will be cancelled due to lack of interest.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH stable-3.2 stable-3.12] net: fix checksum check in skb_copy_and_csum_datagram_iovec()

[Jiri Slaby]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 01/05/2016, 05:40 PM, Ben Hutchings wrote:
> On Tue, 2016-01-05 at 17:36 +0100, Jiri Slaby wrote:
>> On 12/28/2015, 03:01 PM, Michal Kubecek wrote:
>>> Recent fix "net: add length argument to 
>>> skb_copy_and_csum_datagram_iovec" added to some pre-3.19
>>> stable branches, namely
>>> 
>>> stable-3.2.y: commit 127500d724f8 stable-3.12.y: commit
>>> 3e1ac3aafbd0
>> 
>> Applied this fix to 3.12. Thanks!
> [...]
> 
> You don't want this, you want Eric's fix (commit 197c949e7, "udp: 
> properly support MSG_PEEK with truncated buffers") although that's
> not upstream yet.

Dropped then. Thanks!


- -- 
js
suse labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJWi/IlAAoJEL0lsQQGtHBJ+HkP/RSxEleW2xhvRXBVz9S5iTgR
U5dDY3gdpajVuouc8S8ZC8o6/BT+rx0LwzrCcb8RDkIOPad3dFVrcPn5uwZvCZ+b
JSNKvNLTLgtn/jvSu5ZhjlL0F4zXu2Pl37KZpaimwEgGAEmfKAvLL52mdHWURGRq
AD7qD5UJxybejt8VNpExX6Oo+Q6nsRJtbrXXSRbalpeCwUhMxu+A/0vWFQ+bFR4M
GPMw6GqhCoCTy1fcpI4csyvEJImg91PAnNc2+37vOUqxrpr4hqBtNnVerampaI1N
u6QGl5WxfqvX+sDeYv7JDCR+2D750EixcfI6PYFdINSgTBn/7veKTsN1buj9JzlU
LBVFKXLILX3pOj16rIiwVtmqapinyJB9c3vCUDP19opn7LtkR16a5wDOTD3T59zs
KX6FZ0mO3uRacJNpVqeAcYQ8MVFIN6JRCCokJoTgFmia19rSpxV3Th1MoFy/bafX
yc5T+n0CYguTVirME2hn2vt2JHvUzUCcabktT0chY8EhXTc6XMBefRGWnhL3raBe
zRCn8tnGUbawr46ZbwMggpavfzUg3lGE6Y5/g5l5dNmdrLfU5SnNmid769Uvlrad
MFf56DMfMFHmojcqOukcN1v97jvmaK9eY2xXRvuoGnzYexL2JCiPEKfB21NQfZhW
NZSya8PXfCtyAnUS+56r
=hGxr
-----END PGP SIGNATURE-----

Re: [PATCH stable-3.2 stable-3.12] net: fix checksum check in skb_copy_and_csum_datagram_iovec()

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 804 bytes --]

On Tue, 2016-01-05 at 16:40 +0000, Ben Hutchings wrote:
> On Tue, 2016-01-05 at 17:36 +0100, Jiri Slaby wrote:
> > On 12/28/2015, 03:01 PM, Michal Kubecek wrote:
> > > Recent fix "net: add length argument to
> > > skb_copy_and_csum_datagram_iovec" added to some pre-3.19 stable
> > > branches, namely
> > > 
> > >   stable-3.2.y: commit 127500d724f8
> > >   stable-3.12.y: commit 3e1ac3aafbd0
> > 
> > Applied this fix to 3.12. Thanks!
> [...]
> 
> You don't want this, you want Eric's fix (commit 197c949e7, "udp:
> properly support MSG_PEEK with truncated buffers") although that's not
> upstream yet.

It's now upstream as commit 197c949e7798fbf28cfadc69d9ca0c2abbf93191.

Ben.

-- 
Ben Hutchings
Unix is many things to many people,
but it's never been everything to anybody.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH stable-3.2 stable-3.12] net: fix checksum check in skb_copy_and_csum_datagram_iovec()

[Eric Dumazet]

On Wed, 2016-01-13 at 11:02 +0000, Ben Hutchings wrote:
> On Tue, 2016-01-05 at 16:40 +0000, Ben Hutchings wrote:
> > On Tue, 2016-01-05 at 17:36 +0100, Jiri Slaby wrote:
> > > On 12/28/2015, 03:01 PM, Michal Kubecek wrote:
> > > > Recent fix "net: add length argument to
> > > > skb_copy_and_csum_datagram_iovec" added to some pre-3.19 stable
> > > > branches, namely
> > > > 
> > > >   stable-3.2.y: commit 127500d724f8
> > > >   stable-3.12.y: commit 3e1ac3aafbd0
> > > 
> > > Applied this fix to 3.12. Thanks!
> > [...]
> > 
> > You don't want this, you want Eric's fix (commit 197c949e7, "udp:
> > properly support MSG_PEEK with truncated buffers") although that's not
> > upstream yet.
> 
> It's now upstream as commit 197c949e7798fbf28cfadc69d9ca0c2abbf93191.
> 
> Ben.
> 

Indeed, thanks guys for your backports !

Re: [PATCH stable-3.2 stable-3.12] net: fix checksum check in skb_copy_and_csum_datagram_iovec()

[Luis Henriques]

On Wed, Jan 13, 2016 at 11:02:00AM +0000, Ben Hutchings wrote:
> On Tue, 2016-01-05 at 16:40 +0000, Ben Hutchings wrote:
> > On Tue, 2016-01-05 at 17:36 +0100, Jiri Slaby wrote:
> > > On 12/28/2015, 03:01 PM, Michal Kubecek wrote:
> > > > Recent fix "net: add length argument to
> > > > skb_copy_and_csum_datagram_iovec" added to some pre-3.19 stable
> > > > branches, namely
> > > > 
> > > >   stable-3.2.y: commit 127500d724f8
> > > >   stable-3.12.y: commit 3e1ac3aafbd0
> > > 
> > > Applied this fix to 3.12. Thanks!
> > [...]
> > 
> > You don't want this, you want Eric's fix (commit 197c949e7, "udp:
> > properly support MSG_PEEK with truncated buffers") although that's not
> > upstream yet.
> 
> It's now upstream as commit 197c949e7798fbf28cfadc69d9ca0c2abbf93191.
> 

Thanks, I'm queuing it for the 3.16 kernel as well.

Cheers,
--
Luís

> Ben.
> 
> -- 
> Ben Hutchings
> Unix is many things to many people,
> but it's never been everything to anybody.