diff mbox

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

Message ID c1b23313f94ba3fd9977c6fba61483edf6489b5f.1444910940.git.sd@queasysnail.net
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Sabrina Dubroca Oct. 15, 2015, 12:25 p.m. UTC
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(-)

Comments

Sabrina Dubroca Oct. 29, 2015, 10 a.m. UTC | #1
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
Josh Hunt Nov. 10, 2015, 11:59 p.m. UTC | #2
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
Greg KH Nov. 11, 2015, 12:03 a.m. UTC | #3
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
--
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
Sabrina Dubroca Nov. 12, 2015, 9:48 a.m. UTC | #4
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,
David Miller Nov. 13, 2015, 9:29 p.m. UTC | #5
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.
--
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
Ben Hutchings Nov. 14, 2015, 5:39 p.m. UTC | #6
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.
diff mbox

Patch

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;
 		}