[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>

diff --git a/arch/sparc64/solaris/socket.c b/arch/sparc64/solaris/socket.c
index 06740582717e4427da136ed80b139b4aa40cbf05..d3a66ea74a7f1e75a9223c8a5812d6ac6898073e 100644 (file)
--- a/arch/sparc64/solaris/socket.c
+++ b/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;
 }

diff --git a/net/compat.c b/net/compat.c
index be5d936dc42396ae0c8a18d77f80b72d04772d83..d99ab969589397f9cc844db57aac0b7ab2f9711f 100644 (file)
--- a/net/compat.c
+++ b/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;
 }