From d64d3873721cfe870d49d73c3744f06260779ce7 Mon Sep 17 00:00:00 2001 From: Andrew Morton Date: Tue, 9 Aug 2005 15:29:19 -0700 Subject: [PATCH] [NET]: Fix memory leak in sys_{send,recv}msg() w/compat From: Dave Johnson 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 Signed-off-by: David S. Miller --- arch/sparc64/solaris/socket.c | 193 ++++++++++++++++++++++++++---------------- net/compat.c | 9 -- 2 files changed, 119 insertions(+), 83 deletions(-) diff --git a/arch/sparc64/solaris/socket.c b/arch/sparc64/solaris/socket.c index 0674058..d3a66ea 100644 --- a/arch/sparc64/solaris/socket.c +++ b/arch/sparc64/solaris/socket.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -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 be5d936..d99ab96 100644 --- 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; } -- 2.7.4