Bug 138860 - [linux] [patch] linux_socketcall() causing buffer overflow
Summary: [linux] [patch] linux_socketcall() causing buffer overflow
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 9.0-CURRENT
Hardware: Any Any
: Normal Affects Only Me
Assignee: Xin LI
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-15 22:20 UTC by Alexander Best
Modified: 2010-02-23 00:50 UTC (History)
0 users

See Also:


Attachments
file.txt (5.54 KB, text/plain)
2009-09-15 22:20 UTC, Alexander Best
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Best 2009-09-15 22:20:08 UTC
the linux test project (ltp) is a set of small scripts and binaries to test if an environment meets all the criteria necessary to be 100% compatible with linux.

running the ltp scripts revealed a buffer overflow caused by linux_socketcall() which emulates linux socketcall() syscall. the buffer overflow gets reported multiple times during a full ltp run, because several tests use the linux socketcall() syscall and thus linux_socketcall(). one of the tests causing the buffer overflow is testcases/kernel/syscalls/bind/bind01. i've attached the source for bind01.

here's the overflow report by REDZONE which gets reported when the `bind01` binary is being run:

REDZONE: Buffer overflow detected. 9 bytes corrupted after 0xca667283 (3 bytes allocated).
Allocation backtrace:
#0 0xc070cc5a at redzone_setup+0x3a
#1 0xc05b9cf3 at malloc+0x1c3
#2 0xc0af993c at linux_getsockaddr+0x3c
#3 0xc0afa51e at linux_socketcall+0x73e
#4 0xc0760ea6 at syscall+0x2a6
#5 0xc0744800 at Xint0x80_syscall+0x20
Free backtrace:
#0 0xc070cbea at redzone_check+0x17a
#1 0xc05b99ad at free+0x5d
#2 0xc0afa556 at linux_socketcall+0x776
#3 0xc0760ea6 at syscall+0x2a6
#4 0xc0744800 at Xint0x80_syscall+0x20

i've marked this PR as high priority because the buffer overflow could pose a security threat and be used to execute harmful code.

cheers.
alex

[1] http://lists.freebsd.org/pipermail/freebsd-emulation/2009-September/006877.html

Fix: problem probably lies in /usr/src/sys/compat/linux/linux_socket.c

Patch attached with submission follows:
How-To-Repeat: cd /usr/ports/emulators/linux_dist-gentoo-stage3 && make install
cd /usr/local/gentoo-stage3
cvs -d:pserver:anonymous@ltp.cvs.sourceforge.net:/cvsroot/ltp login
cvs -z3 -d:pserver:anonymous@ltp.cvs.sourceforge.net:/cvsroot/ltp co ltp
chroot /usr/local/gentoo-stage3 bash
cd ltp && ./configure && make all install
cd testcases/kernel/syscalls/bind
./bind01
Comment 1 Gavin Atkinson freebsd_committer freebsd_triage 2009-10-31 14:43:42 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-emulation

Over to maintainer(s).
Comment 2 Mateusz Guzik 2010-01-22 19:30:25 UTC
Hi,

It looks like the problem is caused by this:

sys/compat/linux/linux_socket.c : do_sa_get() contains the following code:
	if (*osalen < 2 || *osalen > UCHAR_MAX || !osa)
		return (EINVAL);
[..]
	alloclen = *osalen;
[..]
	kosa = malloc(alloclen, mtype, M_WAITOK); // [1]

	if ((error = copyin(osa, kosa, *osalen)))
		goto out;

	bdom = linux_to_bsd_domain(kosa->sa_family);
[..]
	if (bdom == AF_INET)
		alloclen = sizeof(struct sockaddr_in); // [2]

	sa = (struct sockaddr *) kosa;
	sa->sa_family = bdom;
	sa->sa_len = alloclen; // [3]

	*sap = sa;
	*osalen = alloclen;
[..]
--------

*osalen bytes is allocated in [1]. In [2] we override the old value of alloclen
and use it in assignment ([3]).

So if *osalen is lower than sizeof(struct sockaddr_in), we return struct that is
too small and contains faked length.

This defeats checks placed in sys/netinet/in_pcb.c : in_pcbbind_setup() and
leads to overflow by this (line 348 as of r202295):
[..]
bzero(&sin->sin_zero, sizeof(sin->sin_zero));
[..]

--------

Proposed patch:

http://student.agh.edu.pl/~mjguzik/linux_socket.patch

Note: patch also changes return value from EINVAL to EAFNOSUPPORT in case of
linux_to_bsd_domain's failure to match behaviour of other callers.

Briefly tested with wget, Quake 3 and firefox.

--
Mateusz Guzik
Comment 3 Alexander Best 2010-02-09 13:36:38 UTC
thanks a lot. :)

i've tested the patch and running the test case i mentioned in the pr no
longer causes a buffer overflow warning being reported by REDZONE.

somebody please commit this patch.

cheers.
alex
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2010-02-09 21:40:22 UTC
State Changed
From-To: open->analyzed

Submitter says the proposed patch fixed the problem for him.
Comment 5 Xin LI freebsd_committer freebsd_triage 2010-02-09 22:13:45 UTC
Responsible Changed
From-To: freebsd-emulation->delphij

Take.
Comment 6 Xin LI freebsd_committer freebsd_triage 2010-02-09 22:30:54 UTC
State Changed
From-To: analyzed->patched

Patch applied against -HEAD, MFC reminder. 

Thanks for reporting the problem and thanks Mateusz Guzik for working 
on the fix.
Comment 7 dfilter service freebsd_committer freebsd_triage 2010-02-09 22:31:05 UTC
Author: delphij
Date: Tue Feb  9 22:30:51 2010
New Revision: 203728
URL: http://svn.freebsd.org/changeset/base/203728

Log:
   - Return EAFNOSUPPORT instead of EINVAL for unsupported address family,
     this matches the Linux behavior.
   - Check if we have sufficient space allocated for socket structure, which
     fixes a buffer overflow when wrong length is being passed into the
     emulation layer. [1]
  
  PR:		kern/138860
  Submitted by:	Mateusz Guzik <mjguzik gmail com>
  Reported by:	Alexander Best [1]
  MFC after:	2 weeks

Modified:
  head/sys/compat/linux/linux_socket.c

Modified: head/sys/compat/linux/linux_socket.c
==============================================================================
--- head/sys/compat/linux/linux_socket.c	Tue Feb  9 22:15:59 2010	(r203727)
+++ head/sys/compat/linux/linux_socket.c	Tue Feb  9 22:30:51 2010	(r203728)
@@ -128,7 +128,7 @@ do_sa_get(struct sockaddr **sap, const s
 
 	bdom = linux_to_bsd_domain(kosa->sa_family);
 	if (bdom == -1) {
-		error = EINVAL;
+		error = EAFNOSUPPORT;
 		goto out;
 	}
 
@@ -157,8 +157,13 @@ do_sa_get(struct sockaddr **sap, const s
 		}
 	} else
 #endif
-	if (bdom == AF_INET)
+	if (bdom == AF_INET) {
 		alloclen = sizeof(struct sockaddr_in);
+		if (*osalen < alloclen) {
+			error = EINVAL;
+			goto out;
+		}
+	}
 
 	sa = (struct sockaddr *) kosa;
 	sa->sa_family = bdom;
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 8 dfilter service freebsd_committer freebsd_triage 2010-02-23 00:40:16 UTC
Author: delphij
Date: Tue Feb 23 00:40:02 2010
New Revision: 204232
URL: http://svn.freebsd.org/changeset/base/204232

Log:
  MFC r203728:
  
   - Return EAFNOSUPPORT instead of EINVAL for unsupported address family,
     this matches the Linux behavior.
   - Check if we have sufficient space allocated for socket structure, which
     fixes a buffer overflow when wrong length is being passed into the
     emulation layer. [1]
  
  PR:		kern/138860
  Submitted by:	Mateusz Guzik <mjguzik gmail com>
  Reported by:	Alexander Best [1]

Modified:
  stable/8/sys/compat/linux/linux_socket.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)
  stable/8/sys/dev/xen/xenpci/   (props changed)
  stable/8/sys/netinet/   (props changed)

Modified: stable/8/sys/compat/linux/linux_socket.c
==============================================================================
--- stable/8/sys/compat/linux/linux_socket.c	Tue Feb 23 00:34:20 2010	(r204231)
+++ stable/8/sys/compat/linux/linux_socket.c	Tue Feb 23 00:40:02 2010	(r204232)
@@ -128,7 +128,7 @@ do_sa_get(struct sockaddr **sap, const s
 
 	bdom = linux_to_bsd_domain(kosa->sa_family);
 	if (bdom == -1) {
-		error = EINVAL;
+		error = EAFNOSUPPORT;
 		goto out;
 	}
 
@@ -157,8 +157,13 @@ do_sa_get(struct sockaddr **sap, const s
 		}
 	} else
 #endif
-	if (bdom == AF_INET)
+	if (bdom == AF_INET) {
 		alloclen = sizeof(struct sockaddr_in);
+		if (*osalen < alloclen) {
+			error = EINVAL;
+			goto out;
+		}
+	}
 
 	sa = (struct sockaddr *) kosa;
 	sa->sa_family = bdom;
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 9 Xin LI freebsd_committer freebsd_triage 2010-02-23 00:41:44 UTC
State Changed
From-To: patched->closed

Patch applied against 8- and 7-STABLE, thanks for reporting and working on patches!
Comment 10 dfilter service freebsd_committer freebsd_triage 2010-02-23 00:41:48 UTC
Author: delphij
Date: Tue Feb 23 00:41:40 2010
New Revision: 204233
URL: http://svn.freebsd.org/changeset/base/204233

Log:
  MFC r203728:
  
   - Return EAFNOSUPPORT instead of EINVAL for unsupported address family,
     this matches the Linux behavior.
   - Check if we have sufficient space allocated for socket structure, which
     fixes a buffer overflow when wrong length is being passed into the
     emulation layer. [1]
  
  PR:		kern/138860
  Submitted by:	Mateusz Guzik <mjguzik gmail com>
  Reported by:	Alexander Best [1]

Modified:
  stable/7/sys/compat/linux/linux_socket.c
Directory Properties:
  stable/7/sys/   (props changed)
  stable/7/sys/cddl/contrib/opensolaris/   (props changed)
  stable/7/sys/contrib/dev/acpica/   (props changed)
  stable/7/sys/contrib/pf/   (props changed)

Modified: stable/7/sys/compat/linux/linux_socket.c
==============================================================================
--- stable/7/sys/compat/linux/linux_socket.c	Tue Feb 23 00:40:02 2010	(r204232)
+++ stable/7/sys/compat/linux/linux_socket.c	Tue Feb 23 00:41:40 2010	(r204233)
@@ -126,7 +126,7 @@ do_sa_get(struct sockaddr **sap, const s
 
 	bdom = linux_to_bsd_domain(kosa->sa_family);
 	if (bdom == -1) {
-		error = EINVAL;
+		error = EAFNOSUPPORT;
 		goto out;
 	}
 
@@ -155,8 +155,13 @@ do_sa_get(struct sockaddr **sap, const s
 		}
 	} else
 #endif
-	if (bdom == AF_INET)
+	if (bdom == AF_INET) {
 		alloclen = sizeof(struct sockaddr_in);
+		if (*osalen < alloclen) {
+			error = EINVAL;
+			goto out;
+		}
+	}
 
 	sa = (struct sockaddr *) kosa;
 	sa->sa_family = bdom;
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"