[PATCH stable <= 3.18] net: add length argument to skb_copy_and_csum_datagram_iovec

[PATCH stable <= 3.18] net: add length argument to skb_copy_and_csum_datagram_iovec

[Sabrina Dubroca]

Without this length argument, we can read past the end of the iovec in
memcpy_toiovec because we have no way of knowing the total length of the
iovec's buffers.

This is needed for stable kernels where 89c22d8c3b27 ("net: Fix skb
csum races when peeking") has been backported but that don't have the
ioviter conversion, which is almost all the stable trees <= 3.18.

This also fixes a kernel crash for NFS servers when the client uses
 -onfsvers=3,proto=udp to mount the export.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
Note: this is based on 3.14.54, as 3.18 doesn't need the hunk for
net/rxrpc/ar-recvmsg.c, but all older stable kernels do.


 include/linux/skbuff.h | 2 +-
 net/core/datagram.c    | 6 +++++-
 net/ipv4/tcp_input.c   | 2 +-
 net/ipv4/udp.c         | 2 +-
 net/ipv6/raw.c         | 2 +-
 net/ipv6/udp.c         | 3 ++-
 net/rxrpc/ar-recvmsg.c | 3 ++-
 7 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ab3133797ff7..220454f32509 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2431,7 +2431,7 @@ unsigned int datagram_poll(struct file *file, struct socket *sock,
 int skb_copy_datagram_iovec(const struct sk_buff *from, int offset,
 			    struct iovec *to, int size);
 int skb_copy_and_csum_datagram_iovec(struct sk_buff *skb, int hlen,
-				     struct iovec *iov);
+				     struct iovec *iov, int len);
 int skb_copy_datagram_from_iovec(struct sk_buff *skb, int offset,
 				 const struct iovec *from, int from_offset,
 				 int len);
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 13bc7dad7990..3437762668af 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -796,6 +796,7 @@ EXPORT_SYMBOL(__skb_checksum_complete);
  *	@skb: skbuff
  *	@hlen: hardware length
  *	@iov: io vector
+ *	@len: amount of data to copy from skb to iov
  *
  *	Caller _must_ check that skb will fit to this iovec.
  *
@@ -805,11 +806,14 @@ EXPORT_SYMBOL(__skb_checksum_complete);
  *			   can be modified!
  */
 int skb_copy_and_csum_datagram_iovec(struct sk_buff *skb,
-				     int hlen, struct iovec *iov)
+				     int hlen, struct iovec *iov, int len)
 {
 	__wsum csum;
 	int chunk = skb->len - hlen;
 
+	if (chunk > len)
+		chunk = len;
+
 	if (!chunk)
 		return 0;
 
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9fbd69efa999..cf6168b897c3 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4934,7 +4934,7 @@ static int tcp_copy_to_iovec(struct sock *sk, struct sk_buff *skb, int hlen)
 		err = skb_copy_datagram_iovec(skb, hlen, tp->ucopy.iov, chunk);
 	else
 		err = skb_copy_and_csum_datagram_iovec(skb, hlen,
-						       tp->ucopy.iov);
+						       tp->ucopy.iov, chunk);
 
 	if (!err) {
 		tp->ucopy.len -= chunk;
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 6970e36ad7b8..8395cf5ec487 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1268,7 +1268,7 @@ try_again:
 	else {
 		err = skb_copy_and_csum_datagram_iovec(skb,
 						       sizeof(struct udphdr),
-						       msg->msg_iov);
+						       msg->msg_iov, copied);
 
 		if (err == -EINVAL)
 			goto csum_copy_err;
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 1f29996e368a..e6c9b4a7ee3c 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -492,7 +492,7 @@ static int rawv6_recvmsg(struct kiocb *iocb, struct sock *sk,
 			goto csum_copy_err;
 		err = skb_copy_datagram_iovec(skb, 0, msg->msg_iov, copied);
 	} else {
-		err = skb_copy_and_csum_datagram_iovec(skb, 0, msg->msg_iov);
+		err = skb_copy_and_csum_datagram_iovec(skb, 0, msg->msg_iov, copied);
 		if (err == -EINVAL)
 			goto csum_copy_err;
 	}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 38625a91ec94..4011ccad6c9f 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -428,7 +428,8 @@ try_again:
 		err = skb_copy_datagram_iovec(skb, sizeof(struct udphdr),
 					      msg->msg_iov, copied);
 	else {
-		err = skb_copy_and_csum_datagram_iovec(skb, sizeof(struct udphdr), msg->msg_iov);
+		err = skb_copy_and_csum_datagram_iovec(skb, sizeof(struct udphdr),
+						       msg->msg_iov, copied);
 		if (err == -EINVAL)
 			goto csum_copy_err;
 	}
diff --git a/net/rxrpc/ar-recvmsg.c b/net/rxrpc/ar-recvmsg.c
index 4949f753686c..83484ebf691d 100644
--- a/net/rxrpc/ar-recvmsg.c
+++ b/net/rxrpc/ar-recvmsg.c
@@ -186,7 +186,8 @@ int rxrpc_recvmsg(struct kiocb *iocb, struct socket *sock,
 						      msg->msg_iov, copy);
 		} else {
 			ret = skb_copy_and_csum_datagram_iovec(skb, offset,
-							       msg->msg_iov);
+							       msg->msg_iov,
+							       copy);
 			if (ret == -EINVAL)
 				goto csum_copy_error;
 		}
-- 
2.6.1

Re: [PATCH stable <= 3.18] net: add length argument to skb_copy_and_csum_datagram_iovec

[Sabrina Dubroca]

2015-10-15, 14:25:03 +0200, Sabrina Dubroca wrote:
> Without this length argument, we can read past the end of the iovec in
> memcpy_toiovec because we have no way of knowing the total length of the
> iovec's buffers.
> 
> This is needed for stable kernels where 89c22d8c3b27 ("net: Fix skb
> csum races when peeking") has been backported but that don't have the
> ioviter conversion, which is almost all the stable trees <= 3.18.
> 
> This also fixes a kernel crash for NFS servers when the client uses
>  -onfsvers=3,proto=udp to mount the export.
> 
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Fixes CVE-2015-8019.
http://www.openwall.com/lists/oss-security/2015/10/29/1

-- 
Sabrina

Re: [PATCH stable <= 3.18] net: add length argument to skb_copy_and_csum_datagram_iovec

[Josh Hunt]

On Thu, Oct 29, 2015 at 5:00 AM, Sabrina Dubroca <sd@queasysnail.net> wrote:
> 2015-10-15, 14:25:03 +0200, Sabrina Dubroca wrote:
>> Without this length argument, we can read past the end of the iovec in
>> memcpy_toiovec because we have no way of knowing the total length of the
>> iovec's buffers.
>>
>> This is needed for stable kernels where 89c22d8c3b27 ("net: Fix skb
>> csum races when peeking") has been backported but that don't have the
>> ioviter conversion, which is almost all the stable trees <= 3.18.
>>
>> This also fixes a kernel crash for NFS servers when the client uses
>>  -onfsvers=3,proto=udp to mount the export.
>>
>> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
>> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>
> Fixes CVE-2015-8019.
> http://www.openwall.com/lists/oss-security/2015/10/29/1
>
> --
> Sabrina
> --
> 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

Greg

Do you have this in your queue? I saw a few other stables pick this
up, but haven't seen it in 3.14 or 3.18 yet. It wasn't clear to me if
this had been fully reviewed yet.

Thanks
-- 
Josh

Re: [PATCH stable <= 3.18] net: add length argument to skb_copy_and_csum_datagram_iovec

[Greg Kroah-Hartman]

On Tue, Nov 10, 2015 at 05:59:26PM -0600, Josh Hunt wrote:
> On Thu, Oct 29, 2015 at 5:00 AM, Sabrina Dubroca <sd@queasysnail.net> wrote:
> > 2015-10-15, 14:25:03 +0200, Sabrina Dubroca wrote:
> >> Without this length argument, we can read past the end of the iovec in
> >> memcpy_toiovec because we have no way of knowing the total length of the
> >> iovec's buffers.
> >>
> >> This is needed for stable kernels where 89c22d8c3b27 ("net: Fix skb
> >> csum races when peeking") has been backported but that don't have the
> >> ioviter conversion, which is almost all the stable trees <= 3.18.
> >>
> >> This also fixes a kernel crash for NFS servers when the client uses
> >>  -onfsvers=3,proto=udp to mount the export.
> >>
> >> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> >> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> >
> > Fixes CVE-2015-8019.
> > http://www.openwall.com/lists/oss-security/2015/10/29/1
> >
> > --
> > Sabrina
> > --
> > 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
> 
> Greg
> 
> Do you have this in your queue? I saw a few other stables pick this
> up, but haven't seen it in 3.14 or 3.18 yet. It wasn't clear to me if
> this had been fully reviewed yet.

I rely on Dave to package up networking stable patches and forward them
on to me, that's why you haven't seen it be picked up yet.

thanks,

greg k-h

Re: [PATCH stable <= 3.18] net: add length argument to skb_copy_and_csum_datagram_iovec

[Sabrina Dubroca]

2015-11-10, 16:03:52 -0800, Greg Kroah-Hartman wrote:
> On Tue, Nov 10, 2015 at 05:59:26PM -0600, Josh Hunt wrote:
> > On Thu, Oct 29, 2015 at 5:00 AM, Sabrina Dubroca <sd@queasysnail.net> wrote:
> > > 2015-10-15, 14:25:03 +0200, Sabrina Dubroca wrote:
> > >> Without this length argument, we can read past the end of the iovec in
> > >> memcpy_toiovec because we have no way of knowing the total length of the
> > >> iovec's buffers.
> > >>
> > >> This is needed for stable kernels where 89c22d8c3b27 ("net: Fix skb
> > >> csum races when peeking") has been backported but that don't have the
> > >> ioviter conversion, which is almost all the stable trees <= 3.18.
> > >>
> > >> This also fixes a kernel crash for NFS servers when the client uses
> > >>  -onfsvers=3,proto=udp to mount the export.
> > >>
> > >> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> > >> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > >
> > > Fixes CVE-2015-8019.
> > > http://www.openwall.com/lists/oss-security/2015/10/29/1
> > >
> > > --
> > > Sabrina
> > > --
> > > 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
> > 
> > Greg
> > 
> > Do you have this in your queue? I saw a few other stables pick this
> > up, but haven't seen it in 3.14 or 3.18 yet. It wasn't clear to me if
> > this had been fully reviewed yet.
> 
> I rely on Dave to package up networking stable patches and forward them
> on to me, that's why you haven't seen it be picked up yet.
> 
> thanks,
> 
> greg k-h

David, can you queue this up?

Thanks,

-- 
Sabrina

Re: [PATCH stable <= 3.18] net: add length argument to skb_copy_and_csum_datagram_iovec

[Eric Dumazet]

On Thu, 2015-11-12 at 10:48 +0100, Sabrina Dubroca wrote:
> 2015-11-10, 16:03:52 -0800, Greg Kroah-Hartman wrote:
> > On Tue, Nov 10, 2015 at 05:59:26PM -0600, Josh Hunt wrote:
> > > On Thu, Oct 29, 2015 at 5:00 AM, Sabrina Dubroca <sd@queasysnail.net> wrote:
> > > > 2015-10-15, 14:25:03 +0200, Sabrina Dubroca wrote:
> > > >> Without this length argument, we can read past the end of the iovec in
> > > >> memcpy_toiovec because we have no way of knowing the total length of the
> > > >> iovec's buffers.
> > > >>
> > > >> This is needed for stable kernels where 89c22d8c3b27 ("net: Fix skb
> > > >> csum races when peeking") has been backported but that don't have the
> > > >> ioviter conversion, which is almost all the stable trees <= 3.18.
> > > >>
> > > >> This also fixes a kernel crash for NFS servers when the client uses
> > > >>  -onfsvers=3,proto=udp to mount the export.
> > > >>
> > > >> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> > > >> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > > >
> > > > Fixes CVE-2015-8019.
> > > > http://www.openwall.com/lists/oss-security/2015/10/29/1
> > > >
> > > > --
> > > > Sabrina
> > > > --
> > > > 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
> > > 
> > > Greg
> > > 
> > > Do you have this in your queue? I saw a few other stables pick this
> > > up, but haven't seen it in 3.14 or 3.18 yet. It wasn't clear to me if
> > > this had been fully reviewed yet.
> > 
> > I rely on Dave to package up networking stable patches and forward them
> > on to me, that's why you haven't seen it be picked up yet.
> > 
> > thanks,
> > 
> > greg k-h
> 
> David, can you queue this up?
> 

Note that the following patch (and corresponding part for ipv6) might
also have solve the issue ?

This would supposedly save some cycles when MSG_PEEK is used and user
provides short buffers.

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 24ec14f9825c..387acab1ab5c 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1272,6 +1272,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
 	int err;
 	int is_udplite = IS_UDPLITE(sk);
 	bool slow;
+	bool checksum_valid = false;
 
 	if (flags & MSG_ERRQUEUE)
 		return ip_recv_error(sk, msg, len, addr_len);
@@ -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 {

Re: [PATCH stable <= 3.18] net: add length argument to skb_copy_and_csum_datagram_iovec

[David Miller]

From: Sabrina Dubroca <sd@queasysnail.net>
Date: Thu, 12 Nov 2015 10:48:22 +0100

> 2015-11-10, 16:03:52 -0800, Greg Kroah-Hartman wrote:
>> On Tue, Nov 10, 2015 at 05:59:26PM -0600, Josh Hunt wrote:
>> > On Thu, Oct 29, 2015 at 5:00 AM, Sabrina Dubroca <sd@queasysnail.net> wrote:
>> > > 2015-10-15, 14:25:03 +0200, Sabrina Dubroca wrote:
>> > >> Without this length argument, we can read past the end of the iovec in
>> > >> memcpy_toiovec because we have no way of knowing the total length of the
>> > >> iovec's buffers.
>> > >>
>> > >> This is needed for stable kernels where 89c22d8c3b27 ("net: Fix skb
>> > >> csum races when peeking") has been backported but that don't have the
>> > >> ioviter conversion, which is almost all the stable trees <= 3.18.
>> > >>
>> > >> This also fixes a kernel crash for NFS servers when the client uses
>> > >>  -onfsvers=3,proto=udp to mount the export.
>> > >>
>> > >> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
>> > >> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> > >
>> > > Fixes CVE-2015-8019.
>> > > http://www.openwall.com/lists/oss-security/2015/10/29/1
>> > >
>> > > --
>> > > Sabrina
>> > > --
>> > > 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
>> > 
>> > Greg
>> > 
>> > Do you have this in your queue? I saw a few other stables pick this
>> > up, but haven't seen it in 3.14 or 3.18 yet. It wasn't clear to me if
>> > this had been fully reviewed yet.
>> 
>> I rely on Dave to package up networking stable patches and forward them
>> on to me, that's why you haven't seen it be picked up yet.
>> 
>> thanks,
>> 
>> greg k-h
> 
> David, can you queue this up?

This doesn't even apply to v3.18.24, the patched call site in
net/rxrpc/ar-recvmsg.c doesn't even exist.

Once you fix this up just submit it to -stable directly, I'm
fine with that for this.  I'm only handling submissions back
to v3.18 (4 releases) anyways.

Thanks.

Re: [PATCH stable <= 3.18] net: add length argument to skb_copy_and_csum_datagram_iovec

[Ben Hutchings]

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

On Thu, 2015-10-15 at 14:25 +0200, Sabrina Dubroca wrote:
> Without this length argument, we can read past the end of the iovec
> in
> memcpy_toiovec because we have no way of knowing the total length of
> the
> iovec's buffers.
> 
> This is needed for stable kernels where 89c22d8c3b27 ("net: Fix skb
> csum races when peeking") has been backported but that don't have the
> ioviter conversion, which is almost all the stable trees <= 3.18.
> 
> This also fixes a kernel crash for NFS servers when the client uses
>  -onfsvers=3,proto=udp to mount the export.
> 
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
> Note: this is based on 3.14.54, as 3.18 doesn't need the hunk for
> net/rxrpc/ar-recvmsg.c, but all older stable kernels do.
[...]

Queued up for 3.2, thanks.

Ben.

-- 
Ben Hutchings
Everything should be made as simple as possible, but not simpler.
                                                           - Albert Einstein

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

Re: [PATCH stable <= 3.18] net: add length argument to skb_copy_and_csum_datagram_iovec

[Sabrina Dubroca]

Hello Eric

2015-11-12, 09:26:42 -0800, Eric Dumazet wrote:
> Note that the following patch (and corresponding part for ipv6) might
> also have solve the issue ?
> 
> This would supposedly save some cycles when MSG_PEEK is used and user
> provides short buffers.

Your patch looks correct to me, feel free to submit it.

Since some stable trees already include my patch, maybe it should be
reverted there to keep all trees in sync and ease future backports?


Thanks,

-- 
Sabrina