[NET]: Fix memory leak in sys_{send,recv}msg() w/compat

From: Dave Johnson <djohnson+linux-kernel@sw.starentnetworks.com>

sendmsg()/recvmsg() syscalls from o32/n32 apps to a 64bit kernel will
cause a kernel memory leak if iov_len > UIO_FASTIOV for each syscall!

This is because both sys_sendmsg() and verify_compat_iovec() kmalloc a
new iovec structure.  Only the one from sys_sendmsg() is free'ed.

I wrote a simple test program to confirm this after identifying the
problem:

http://davej.org/programs/testsendmsg.c

Note that the below fix will break solaris_sendmsg()/solaris_recvmsg() as
it also calls verify_compat_iovec() but expects it to malloc internally.

[ I fixed that. -DaveM ]
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

arch/sparc64/solaris/socket.c

@@ -16,6 +16,7 @@
 #include <linux/net.h>
 #include <linux/compat.h>
 #include <net/compat.h>
+#include <net/sock.h>
 
 #include <asm/uaccess.h>
 #include <asm/string.h>
@@ -297,121 +298,165 @@ asmlinkage int solaris_sendmsg(int fd, struct sol_nmsghdr __user *user_msg, unsi
 {
 	struct socket *sock;
 	char address[MAX_SOCK_ADDR];
-	struct iovec iov[UIO_FASTIOV];
+	struct iovec iovstack[UIO_FASTIOV], *iov = iovstack;
 	unsigned char ctl[sizeof(struct cmsghdr) + 20];
 	unsigned char *ctl_buf = ctl;
-	struct msghdr kern_msg;
-	int err, total_len;
+	struct msghdr msg_sys;
+	int err, ctl_len, iov_size, total_len;
 
-	if(msghdr_from_user32_to_kern(&kern_msg, user_msg))
-		return -EFAULT;
-	if(kern_msg.msg_iovlen > UIO_MAXIOV)
-		return -EINVAL;
-	err = verify_compat_iovec(&kern_msg, iov, address, VERIFY_READ);
-	if (err < 0)
+	err = -EFAULT;
+	if (msghdr_from_user32_to_kern(&msg_sys, user_msg))
+		goto out;
+
+	sock = sockfd_lookup(fd, &err);
+	if (!sock)
 		goto out;
+
+	/* do not move before msg_sys is valid */
+	err = -EMSGSIZE;
+	if (msg_sys.msg_iovlen > UIO_MAXIOV)
+		goto out_put;
+
+	/* Check whether to allocate the iovec area*/
+	err = -ENOMEM;
+	iov_size = msg_sys.msg_iovlen * sizeof(struct iovec);
+	if (msg_sys.msg_iovlen > UIO_FASTIOV) {
+		iov = sock_kmalloc(sock->sk, iov_size, GFP_KERNEL);
+		if (!iov)
+			goto out_put;
+	}
+
+	err = verify_compat_iovec(&msg_sys, iov, address, VERIFY_READ);
+	if (err < 0)
+		goto out_freeiov;
 	total_len = err;
 
-	if(kern_msg.msg_controllen) {
-		struct sol_cmsghdr __user *ucmsg = kern_msg.msg_control;
+	err = -ENOBUFS;
+	if (msg_sys.msg_controllen > INT_MAX)
+		goto out_freeiov;
+
+	ctl_len = msg_sys.msg_controllen;
+	if (ctl_len) {
+		struct sol_cmsghdr __user *ucmsg = msg_sys.msg_control;
 		unsigned long *kcmsg;
 		compat_size_t cmlen;
 
-		if (kern_msg.msg_controllen <= sizeof(compat_size_t))
-			return -EINVAL;
+		err = -EINVAL;
+		if (ctl_len <= sizeof(compat_size_t))
+			goto out_freeiov;
 
-		if(kern_msg.msg_controllen > sizeof(ctl)) {
+		if (ctl_len > sizeof(ctl)) {
 			err = -ENOBUFS;
-			ctl_buf = kmalloc(kern_msg.msg_controllen, GFP_KERNEL);
-			if(!ctl_buf)
+			ctl_buf = kmalloc(ctl_len, GFP_KERNEL);
+			if (!ctl_buf)
 				goto out_freeiov;
 		}
 		__get_user(cmlen, &ucmsg->cmsg_len);
 		kcmsg = (unsigned long *) ctl_buf;
 		*kcmsg++ = (unsigned long)cmlen;
 		err = -EFAULT;
-		if(copy_from_user(kcmsg, &ucmsg->cmsg_level,
-				  kern_msg.msg_controllen - sizeof(compat_size_t)))
+		if (copy_from_user(kcmsg, &ucmsg->cmsg_level,
+				   ctl_len - sizeof(compat_size_t)))
 			goto out_freectl;
-		kern_msg.msg_control = ctl_buf;
+		msg_sys.msg_control = ctl_buf;
 	}
-	kern_msg.msg_flags = solaris_to_linux_msgflags(user_flags);
+	msg_sys.msg_flags = solaris_to_linux_msgflags(user_flags);
 
-	lock_kernel();
-	sock = sockfd_lookup(fd, &err);
-	if (sock != NULL) {
-		if (sock->file->f_flags & O_NONBLOCK)
-			kern_msg.msg_flags |= MSG_DONTWAIT;
-		err = sock_sendmsg(sock, &kern_msg, total_len);
-		sockfd_put(sock);
-	}
-	unlock_kernel();
+	if (sock->file->f_flags & O_NONBLOCK)
+		msg_sys.msg_flags |= MSG_DONTWAIT;
+	err = sock_sendmsg(sock, &msg_sys, total_len);
 
 out_freectl:
-	/* N.B. Use kfree here, as kern_msg.msg_controllen might change? */
-	if(ctl_buf != ctl)
-		kfree(ctl_buf);
+	if (ctl_buf != ctl)    
+		sock_kfree_s(sock->sk, ctl_buf, ctl_len);
 out_freeiov:
-	if(kern_msg.msg_iov != iov)
-		kfree(kern_msg.msg_iov);
-out:
+	if (iov != iovstack)
+		sock_kfree_s(sock->sk, iov, iov_size);
+out_put:
+	sockfd_put(sock);
+out:       
 	return err;
 }
 
 asmlinkage int solaris_recvmsg(int fd, struct sol_nmsghdr __user *user_msg, unsigned int user_flags)
 {
-	struct iovec iovstack[UIO_FASTIOV];
-	struct msghdr kern_msg;
-	char addr[MAX_SOCK_ADDR];
 	struct socket *sock;
+	struct iovec iovstack[UIO_FASTIOV];
 	struct iovec *iov = iovstack;
+	struct msghdr msg_sys;
+	unsigned long cmsg_ptr;
+	int err, iov_size, total_len, len;
+
+	/* kernel mode address */
+	char addr[MAX_SOCK_ADDR];
+
+	/* user mode address pointers */
 	struct sockaddr __user *uaddr;
 	int __user *uaddr_len;
-	unsigned long cmsg_ptr;
-	int err, total_len, len = 0;
 
-	if(msghdr_from_user32_to_kern(&kern_msg, user_msg))
+	if (msghdr_from_user32_to_kern(&msg_sys, user_msg))
 		return -EFAULT;
-	if(kern_msg.msg_iovlen > UIO_MAXIOV)
-		return -EINVAL;
 
-	uaddr = kern_msg.msg_name;
+	sock = sockfd_lookup(fd, &err);
+	if (!sock)
+		goto out;
+
+	err = -EMSGSIZE;
+	if (msg_sys.msg_iovlen > UIO_MAXIOV)
+		goto out_put;
+
+	/* Check whether to allocate the iovec area*/
+	err = -ENOMEM;
+	iov_size = msg_sys.msg_iovlen * sizeof(struct iovec);
+	if (msg_sys.msg_iovlen > UIO_FASTIOV) {
+		iov = sock_kmalloc(sock->sk, iov_size, GFP_KERNEL);
+		if (!iov)
+			goto out_put;
+	}
+
+	/*
+	 *	Save the user-mode address (verify_iovec will change the
+	 *	kernel msghdr to use the kernel address space)
+	 */
+	 
+	uaddr = (void __user *) msg_sys.msg_name;
 	uaddr_len = &user_msg->msg_namelen;
-	err = verify_compat_iovec(&kern_msg, iov, addr, VERIFY_WRITE);
+	err = verify_compat_iovec(&msg_sys, iov, addr, VERIFY_WRITE);
 	if (err < 0)
-		goto out;
+		goto out_freeiov;
 	total_len = err;
 
-	cmsg_ptr = (unsigned long) kern_msg.msg_control;
-	kern_msg.msg_flags = 0;
+	cmsg_ptr = (unsigned long) msg_sys.msg_control;
+	msg_sys.msg_flags = MSG_CMSG_COMPAT;
 
-	lock_kernel();
-	sock = sockfd_lookup(fd, &err);
-	if (sock != NULL) {
-		if (sock->file->f_flags & O_NONBLOCK)
-			user_flags |= MSG_DONTWAIT;
-		err = sock_recvmsg(sock, &kern_msg, total_len, user_flags);
-		if(err >= 0)
-			len = err;
-		sockfd_put(sock);
-	}
-	unlock_kernel();
-
-	if(uaddr != NULL && err >= 0)
-		err = move_addr_to_user(addr, kern_msg.msg_namelen, uaddr, uaddr_len);
-	if(err >= 0) {
-		err = __put_user(linux_to_solaris_msgflags(kern_msg.msg_flags), &user_msg->msg_flags);
-		if(!err) {
-			/* XXX Convert cmsg back into userspace 32-bit format... */
-			err = __put_user((unsigned long)kern_msg.msg_control - cmsg_ptr,
-					 &user_msg->msg_controllen);
-		}
+	if (sock->file->f_flags & O_NONBLOCK)
+		user_flags |= MSG_DONTWAIT;
+
+	err = sock_recvmsg(sock, &msg_sys, total_len, user_flags);
+	if(err < 0)
+		goto out_freeiov;
+
+	len = err;
+
+	if (uaddr != NULL) {
+		err = move_addr_to_user(addr, msg_sys.msg_namelen, uaddr, uaddr_len);
+		if (err < 0)
+			goto out_freeiov;
 	}
+	err = __put_user(linux_to_solaris_msgflags(msg_sys.msg_flags), &user_msg->msg_flags);
+	if (err)
+		goto out_freeiov;
+	err = __put_user((unsigned long)msg_sys.msg_control - cmsg_ptr,
+			 &user_msg->msg_controllen);
+	if (err)
+		goto out_freeiov;
+	err = len;
 
-	if(kern_msg.msg_iov != iov)
-		kfree(kern_msg.msg_iov);
+out_freeiov:
+	if (iov != iovstack)
+		sock_kfree_s(sock->sk, iov, iov_size);
+out_put:
+	sockfd_put(sock);
 out:
-	if(err < 0)
-		return err;
-	return len;
+	return err;
 }

net/compat.c

@@ -91,20 +91,11 @@ int verify_compat_iovec(struct msghdr *kern_msg, struct iovec *kern_iov,
 	} else
 		kern_msg->msg_name = NULL;
 
-	if(kern_msg->msg_iovlen > UIO_FASTIOV) {
-		kern_iov = kmalloc(kern_msg->msg_iovlen * sizeof(struct iovec),
-				   GFP_KERNEL);
-		if(!kern_iov)
-			return -ENOMEM;
-	}
-
 	tot_len = iov_from_user_compat_to_kern(kern_iov,
 					  (struct compat_iovec __user *)kern_msg->msg_iov,
 					  kern_msg->msg_iovlen);
 	if(tot_len >= 0)
 		kern_msg->msg_iov = kern_iov;
-	else if(kern_msg->msg_iovlen > UIO_FASTIOV)
-		kfree(kern_iov);
 
 	return tot_len;
 }