Commit a27e13d3 authored by Phil Blundell's avatar Phil Blundell Committed by David S. Miller

econet: fix CVE-2010-3848

Don't declare variable sized array of iovecs on the stack since this
could cause stack overflow if msg->msgiovlen is large.  Instead, coalesce
the user-supplied data into a new buffer and use a single iovec for it.
Signed-off-by: default avatarPhil Blundell <philb@gnu.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 16c41745
...@@ -31,6 +31,7 @@ ...@@ -31,6 +31,7 @@
#include <linux/skbuff.h> #include <linux/skbuff.h>
#include <linux/udp.h> #include <linux/udp.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/vmalloc.h>
#include <net/sock.h> #include <net/sock.h>
#include <net/inet_common.h> #include <net/inet_common.h>
#include <linux/stat.h> #include <linux/stat.h>
...@@ -276,12 +277,12 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock, ...@@ -276,12 +277,12 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
#endif #endif
#ifdef CONFIG_ECONET_AUNUDP #ifdef CONFIG_ECONET_AUNUDP
struct msghdr udpmsg; struct msghdr udpmsg;
struct iovec iov[msg->msg_iovlen+1]; struct iovec iov[2];
struct aunhdr ah; struct aunhdr ah;
struct sockaddr_in udpdest; struct sockaddr_in udpdest;
__kernel_size_t size; __kernel_size_t size;
int i;
mm_segment_t oldfs; mm_segment_t oldfs;
char *userbuf;
#endif #endif
/* /*
...@@ -319,17 +320,17 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock, ...@@ -319,17 +320,17 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
} }
} }
if (len + 15 > dev->mtu) {
mutex_unlock(&econet_mutex);
return -EMSGSIZE;
}
if (dev->type == ARPHRD_ECONET) { if (dev->type == ARPHRD_ECONET) {
/* Real hardware Econet. We're not worthy etc. */ /* Real hardware Econet. We're not worthy etc. */
#ifdef CONFIG_ECONET_NATIVE #ifdef CONFIG_ECONET_NATIVE
unsigned short proto = 0; unsigned short proto = 0;
int res; int res;
if (len + 15 > dev->mtu) {
mutex_unlock(&econet_mutex);
return -EMSGSIZE;
}
dev_hold(dev); dev_hold(dev);
skb = sock_alloc_send_skb(sk, len+LL_ALLOCATED_SPACE(dev), skb = sock_alloc_send_skb(sk, len+LL_ALLOCATED_SPACE(dev),
...@@ -405,6 +406,11 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock, ...@@ -405,6 +406,11 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
return -ENETDOWN; /* No socket - can't send */ return -ENETDOWN; /* No socket - can't send */
} }
if (len > 32768) {
err = -E2BIG;
goto error;
}
/* Make up a UDP datagram and hand it off to some higher intellect. */ /* Make up a UDP datagram and hand it off to some higher intellect. */
memset(&udpdest, 0, sizeof(udpdest)); memset(&udpdest, 0, sizeof(udpdest));
...@@ -436,36 +442,26 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock, ...@@ -436,36 +442,26 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
/* tack our header on the front of the iovec */ /* tack our header on the front of the iovec */
size = sizeof(struct aunhdr); size = sizeof(struct aunhdr);
/*
* XXX: that is b0rken. We can't mix userland and kernel pointers
* in iovec, since on a lot of platforms copy_from_user() will
* *not* work with the kernel and userland ones at the same time,
* regardless of what we do with set_fs(). And we are talking about
* econet-over-ethernet here, so "it's only ARM anyway" doesn't
* apply. Any suggestions on fixing that code? -- AV
*/
iov[0].iov_base = (void *)&ah; iov[0].iov_base = (void *)&ah;
iov[0].iov_len = size; iov[0].iov_len = size;
for (i = 0; i < msg->msg_iovlen; i++) {
void __user *base = msg->msg_iov[i].iov_base; userbuf = vmalloc(len);
size_t iov_len = msg->msg_iov[i].iov_len; if (userbuf == NULL) {
/* Check it now since we switch to KERNEL_DS later. */ err = -ENOMEM;
if (!access_ok(VERIFY_READ, base, iov_len)) { goto error;
mutex_unlock(&econet_mutex);
return -EFAULT;
}
iov[i+1].iov_base = base;
iov[i+1].iov_len = iov_len;
size += iov_len;
} }
iov[1].iov_base = userbuf;
iov[1].iov_len = len;
err = memcpy_fromiovec(userbuf, msg->msg_iov, len);
if (err)
goto error_free_buf;
/* Get a skbuff (no data, just holds our cb information) */ /* Get a skbuff (no data, just holds our cb information) */
if ((skb = sock_alloc_send_skb(sk, 0, if ((skb = sock_alloc_send_skb(sk, 0,
msg->msg_flags & MSG_DONTWAIT, msg->msg_flags & MSG_DONTWAIT,
&err)) == NULL) { &err)) == NULL)
mutex_unlock(&econet_mutex); goto error_free_buf;
return err;
}
eb = (struct ec_cb *)&skb->cb; eb = (struct ec_cb *)&skb->cb;
...@@ -481,7 +477,7 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock, ...@@ -481,7 +477,7 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
udpmsg.msg_name = (void *)&udpdest; udpmsg.msg_name = (void *)&udpdest;
udpmsg.msg_namelen = sizeof(udpdest); udpmsg.msg_namelen = sizeof(udpdest);
udpmsg.msg_iov = &iov[0]; udpmsg.msg_iov = &iov[0];
udpmsg.msg_iovlen = msg->msg_iovlen + 1; udpmsg.msg_iovlen = 2;
udpmsg.msg_control = NULL; udpmsg.msg_control = NULL;
udpmsg.msg_controllen = 0; udpmsg.msg_controllen = 0;
udpmsg.msg_flags=0; udpmsg.msg_flags=0;
...@@ -489,9 +485,13 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock, ...@@ -489,9 +485,13 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock,
oldfs = get_fs(); set_fs(KERNEL_DS); /* More privs :-) */ oldfs = get_fs(); set_fs(KERNEL_DS); /* More privs :-) */
err = sock_sendmsg(udpsock, &udpmsg, size); err = sock_sendmsg(udpsock, &udpmsg, size);
set_fs(oldfs); set_fs(oldfs);
error_free_buf:
vfree(userbuf);
#else #else
err = -EPROTOTYPE; err = -EPROTOTYPE;
#endif #endif
error:
mutex_unlock(&econet_mutex); mutex_unlock(&econet_mutex);
return err; return err;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment