[PATCH] 2.4: PPC64: 32 bit sys_recvmsg corruption

[Stephen Rothwell]

Hi Marcello,

In the presence of threads, there is a possibility of the kernel being
fooled by the 32 bit sys_recvmsg control data into copying more than it
should into the kernel and corrupting kernel data structures.

We call the 64 bit version of sys_recvmsg which writes control messages
directly to user memory which we then read back and "fix up" for the
differences between 32 and 64 bit structures.  If two threads share the
buffer that we are writing into (and then reading from) it is possible for
the control message headers to be changed from what we expect.  One of the
header fields is the length we need to copy back into the kernel ...

This patch just does some more length checking.

This bug was actually being hit by BIND running at a customer site.  It is
very hard to hit, but (obviously) possible.

Signed-off-by: Stephen Rothwell <sfr at canb.auug.org.au>

Pleaase consider for inclusion into 2.4.30.

A patch similar to this may be required my some of the other 64bit archs.

-- 
Cheers,
Stephen Rothwell                    sfr at canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

diff -ruNp 2.4.30-pre1/arch/ppc64/kernel/sys_ppc32.c 2.4.30-pre1-sfr.1/arch/ppc64/kernel/sys_ppc32.c
--- 2.4.30-pre1/arch/ppc64/kernel/sys_ppc32.c	2005-02-16 10:57:39.000000000 +1100
+++ 2.4.30-pre1-sfr.1/arch/ppc64/kernel/sys_ppc32.c	2005-02-16 11:01:05.000000000 +1100
@@ -3664,7 +3664,8 @@ static void scm_detach_fds32(struct msgh
  *		IPV6_RTHDR	ipv6 routing exthdr	32-bit clean
  *		IPV6_AUTHHDR	ipv6 auth exthdr	32-bit clean
  */
-static void cmsg32_recvmsg_fixup(struct msghdr *kmsg, unsigned long orig_cmsg_uptr)
+static void cmsg32_recvmsg_fixup(struct msghdr *kmsg,
+		unsigned long orig_cmsg_uptr, __kernel_size_t orig_cmsg_len)
 {
 	unsigned char *workbuf, *wp;
 	unsigned long bufsz, space_avail;
@@ -3695,6 +3696,19 @@ static void cmsg32_recvmsg_fixup(struct 
 		__get_user(kcmsg32->cmsg_type, &ucmsg->cmsg_type);
 
 		clen64 = kcmsg32->cmsg_len;
+		if ((clen64 < CMSG_ALIGN(sizeof(*ucmsg)))
+				(clen64 > (orig_cmsg_len + wp - workbuf))) {
+			static int count;
+
+			if (count++ < 20)
+				printk(KERN_WARNING "recvmsg_fixup: "
+					"bad data length %d, level %d, "
+					"type %d, process %d (%s)\n",
+					clen64, kcmsg32->cmsg_level,
+					kcmsg32->cmsg_type,
+					current->pid, current->comm);
+			break;
+		}
 		copy_from_user(CMSG32_DATA(kcmsg32), CMSG_DATA(ucmsg),
 			       clen64 - CMSG_ALIGN(sizeof(*ucmsg)));
 		clen32 = ((clen64 - CMSG_ALIGN(sizeof(*ucmsg))) +
@@ -3751,6 +3765,7 @@ asmlinkage long sys32_recvmsg(int fd, st
 	struct sockaddr *uaddr;
 	int *uaddr_len;
 	unsigned long cmsg_ptr;
+	__kernel_size_t cmsg_len;
 	int err, total_len, len = 0;
 	
 	PPCDBG(PPCDBG_SYS32, "sys32_recvmsg - entered - fd=%x, user_msg@=%p, user_flags=%x \n", fd, user_msg, user_flags);
@@ -3768,6 +3783,7 @@ asmlinkage long sys32_recvmsg(int fd, st
 	total_len = err;
 
 	cmsg_ptr = (unsigned long) kern_msg.msg_control;
+	cmsg_len = kern_msg.msg_controllen;
 	kern_msg.msg_flags = 0;
 
 	sock = sockfd_lookup(fd, &err);
@@ -3793,7 +3809,8 @@ asmlinkage long sys32_recvmsg(int fd, st
 				 * to fix it up before we tack on more stuff.
 				 */
 				if((unsigned long) kern_msg.msg_control != cmsg_ptr)
-					cmsg32_recvmsg_fixup(&kern_msg, cmsg_ptr);
+					cmsg32_recvmsg_fixup(&kern_msg,
+							cmsg_ptr, cmsg_len);
 
 				/* Wheee... */
 				if(sock->passcred)