From fa0e846494792e722d817b9d3d625a4ef4896c96 Mon Sep 17 00:00:00 2001 From: Phil Blundell Date: Wed, 24 Nov 2010 11:49:19 -0800 Subject: econet: disallow NULL remote addr for sendmsg(), fixes CVE-2010-3849 Later parts of econet_sendmsg() rely on saddr != NULL, so return early with EINVAL if NULL was passed otherwise an oops may occur. Signed-off-by: Phil Blundell Signed-off-by: David S. Miller --- net/econet/af_econet.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) (limited to 'net/econet') diff --git a/net/econet/af_econet.c b/net/econet/af_econet.c index f8c1ae4b41f..e366f1bef91 100644 --- a/net/econet/af_econet.c +++ b/net/econet/af_econet.c @@ -297,23 +297,14 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock, mutex_lock(&econet_mutex); - if (saddr == NULL) { - struct econet_sock *eo = ec_sk(sk); - - addr.station = eo->station; - addr.net = eo->net; - port = eo->port; - cb = eo->cb; - } else { - if (msg->msg_namelen < sizeof(struct sockaddr_ec)) { - mutex_unlock(&econet_mutex); - return -EINVAL; - } - addr.station = saddr->addr.station; - addr.net = saddr->addr.net; - port = saddr->port; - cb = saddr->cb; - } + if (saddr == NULL || msg->msg_namelen < sizeof(struct sockaddr_ec)) { + mutex_unlock(&econet_mutex); + return -EINVAL; + } + addr.station = saddr->addr.station; + addr.net = saddr->addr.net; + port = saddr->port; + cb = saddr->cb; /* Look for a device with the right network number. */ dev = net2dev_map[addr.net]; @@ -351,7 +342,6 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock, eb = (struct ec_cb *)&skb->cb; - /* BUG: saddr may be NULL */ eb->cookie = saddr->cookie; eb->sec = *saddr; eb->sent = ec_tx_done; -- cgit v1.2.3-70-g09d2 From 16c41745c7b92a243d0874f534c1655196c64b74 Mon Sep 17 00:00:00 2001 From: Phil Blundell Date: Wed, 24 Nov 2010 11:49:53 -0800 Subject: econet: fix CVE-2010-3850 Add missing check for capable(CAP_NET_ADMIN) in SIOCSIFADDR operation. Signed-off-by: Phil Blundell Signed-off-by: David S. Miller --- net/econet/af_econet.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'net/econet') diff --git a/net/econet/af_econet.c b/net/econet/af_econet.c index e366f1bef91..d41ba8e56c1 100644 --- a/net/econet/af_econet.c +++ b/net/econet/af_econet.c @@ -661,6 +661,9 @@ static int ec_dev_ioctl(struct socket *sock, unsigned int cmd, void __user *arg) err = 0; switch (cmd) { case SIOCSIFADDR: + if (!capable(CAP_NET_ADMIN)) + return -EPERM; + edev = dev->ec_ptr; if (edev == NULL) { /* Magic up a new one. */ -- cgit v1.2.3-70-g09d2 From a27e13d370415add3487949c60810e36069a23a6 Mon Sep 17 00:00:00 2001 From: Phil Blundell Date: Wed, 24 Nov 2010 11:51:47 -0800 Subject: 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: Phil Blundell Signed-off-by: David S. Miller --- net/econet/af_econet.c | 62 +++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 31 deletions(-) (limited to 'net/econet') diff --git a/net/econet/af_econet.c b/net/econet/af_econet.c index d41ba8e56c1..13992e1d272 100644 --- a/net/econet/af_econet.c +++ b/net/econet/af_econet.c @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -276,12 +277,12 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock, #endif #ifdef CONFIG_ECONET_AUNUDP struct msghdr udpmsg; - struct iovec iov[msg->msg_iovlen+1]; + struct iovec iov[2]; struct aunhdr ah; struct sockaddr_in udpdest; __kernel_size_t size; - int i; mm_segment_t oldfs; + char *userbuf; #endif /* @@ -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) { /* Real hardware Econet. We're not worthy etc. */ #ifdef CONFIG_ECONET_NATIVE unsigned short proto = 0; int res; + if (len + 15 > dev->mtu) { + mutex_unlock(&econet_mutex); + return -EMSGSIZE; + } + dev_hold(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, 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. */ memset(&udpdest, 0, sizeof(udpdest)); @@ -436,36 +442,26 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock, /* tack our header on the front of the iovec */ 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_len = size; - for (i = 0; i < msg->msg_iovlen; i++) { - void __user *base = msg->msg_iov[i].iov_base; - size_t iov_len = msg->msg_iov[i].iov_len; - /* Check it now since we switch to KERNEL_DS later. */ - if (!access_ok(VERIFY_READ, base, iov_len)) { - mutex_unlock(&econet_mutex); - return -EFAULT; - } - iov[i+1].iov_base = base; - iov[i+1].iov_len = iov_len; - size += iov_len; + + userbuf = vmalloc(len); + if (userbuf == NULL) { + err = -ENOMEM; + goto error; } + 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) */ if ((skb = sock_alloc_send_skb(sk, 0, msg->msg_flags & MSG_DONTWAIT, - &err)) == NULL) { - mutex_unlock(&econet_mutex); - return err; - } + &err)) == NULL) + goto error_free_buf; eb = (struct ec_cb *)&skb->cb; @@ -481,7 +477,7 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock, udpmsg.msg_name = (void *)&udpdest; udpmsg.msg_namelen = sizeof(udpdest); udpmsg.msg_iov = &iov[0]; - udpmsg.msg_iovlen = msg->msg_iovlen + 1; + udpmsg.msg_iovlen = 2; udpmsg.msg_control = NULL; udpmsg.msg_controllen = 0; udpmsg.msg_flags=0; @@ -489,9 +485,13 @@ static int econet_sendmsg(struct kiocb *iocb, struct socket *sock, oldfs = get_fs(); set_fs(KERNEL_DS); /* More privs :-) */ err = sock_sendmsg(udpsock, &udpmsg, size); set_fs(oldfs); + +error_free_buf: + vfree(userbuf); #else err = -EPROTOTYPE; #endif + error: mutex_unlock(&econet_mutex); return err; -- cgit v1.2.3-70-g09d2 From 0c62fc6dd02c8d793c75ae76a9b6881fc36388ad Mon Sep 17 00:00:00 2001 From: Nelson Elhage Date: Wed, 8 Dec 2010 10:13:55 -0800 Subject: econet: Do the correct cleanup after an unprivileged SIOCSIFADDR. We need to drop the mutex and do a dev_put, so set an error code and break like the other paths, instead of returning directly. Signed-off-by: Nelson Elhage Signed-off-by: David S. Miller --- net/econet/af_econet.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'net/econet') diff --git a/net/econet/af_econet.c b/net/econet/af_econet.c index 13992e1d272..f180371fa41 100644 --- a/net/econet/af_econet.c +++ b/net/econet/af_econet.c @@ -661,8 +661,10 @@ static int ec_dev_ioctl(struct socket *sock, unsigned int cmd, void __user *arg) err = 0; switch (cmd) { case SIOCSIFADDR: - if (!capable(CAP_NET_ADMIN)) - return -EPERM; + if (!capable(CAP_NET_ADMIN)) { + err = -EPERM; + break; + } edev = dev->ec_ptr; if (edev == NULL) { -- cgit v1.2.3-70-g09d2