[PATCH] kernel_read result fixes

[PATCH] kernel_read result fixes

[Andres Salomon]

[-- Attachment #1.1: Type: text/plain, Size: 518 bytes --]

Hi,

A few potential vulnerabilities were pointed out by Katrina Tsipenyuk in
<http://seclists.org/lists/linux-kernel/2004/Dec/1878.html>.  I haven't
seen any discussion or fixes of the issue yet, so here's a patch
(against 2.6.9).  The fixes are along the same lines as the previous
binfmt_elf fixes.  There's one additional place (inside fs/binfmt_som.c)
that a fix could be applied, but since that doesn't compile anyways, I
didn't see a point in patching it.


-- 
Andres Salomon <dilinger@voxel.net>

[-- Attachment #1.2: kernel_read-result-validation.patch --]
[-- Type: text/x-patch, Size: 3577 bytes --]

Revision: linux-fs--kernel-read-vuln--0--patch-1
Archive: dilinger@voxel.net--2004-public
Creator: Andres Salomon <dilinger@voxel.net>
Date: Thu Dec 23 23:10:11 EST 2004
Standard-date: 2004-12-24 04:10:11 GMT
Modified-files: binfmt_em86.c binfmt_misc.c binfmt_script.c
    compat.c exec.c
New-patches: dilinger@voxel.net--2004-public/linux-fs--kernel-read-vuln--0--patch-1
Summary: fix bugs mentioned in advisory
Keywords: 

http://seclists.org/lists/bugtraq/2004/Dec/0214.html

This fixes all 6 places mentioned in the advisory.  Most are in binfmt_loader
callbacks, called from exec::do_execve; they fail w/ -EIO if the kernel_read
succeeded, but for some reason a short read was done.


Revision: linux-fs--kernel-read-vuln--0--patch-2
Archive: dilinger@voxel.net--2004-public
Creator: Andres Salomon <dilinger@voxel.net>
Date: Thu Dec 23 23:33:01 EST 2004
Standard-date: 2004-12-24 04:33:01 GMT
Modified-files: binfmt_flat.c
New-patches: dilinger@voxel.net--2004-public/linux-fs--kernel-read-vuln--0--patch-2
Summary: fix another place where kernel_read isn't sufficiently checked 
Keywords: 

I don't know what was up w/ this original check (checking for a res between
-4096 and 0, non-inclusive), but it seems..  off.  Better to check specifically
for BINPRM_BUF_SIZE.

--- orig/fs/binfmt_em86.c
+++ mod/fs/binfmt_em86.c
@@ -89,8 +89,11 @@
 	bprm->file = file;
 
 	retval = prepare_binprm(bprm);
-	if (retval < 0)
+	if (retval != BINPRM_BUF_SIZE) {
+		if (retval >= 0)
+			retval = -EIO;
 		return retval;
+	}
 
 	return search_binary_handler(bprm, regs);
 }


--- orig/fs/binfmt_flat.c
+++ mod/fs/binfmt_flat.c
@@ -780,9 +780,11 @@
 		return res;
 
 	res = prepare_binprm(&bprm);
-
-	if (res <= (unsigned long)-4096)
+	if (res == BINPRM_BUF_SIZE)
 		res = load_flat_file(&bprm, libs, id, NULL);
+	else if (res >= 0)
+		res = -EIO;
+
 	if (bprm.file) {
 		allow_write_access(bprm.file);
 		fput(bprm.file);


--- orig/fs/binfmt_misc.c
+++ mod/fs/binfmt_misc.c
@@ -195,8 +195,11 @@
 	} else
 		retval = prepare_binprm (bprm);
 
-	if (retval < 0)
+	if (retval != BINPRM_BUF_SIZE) {
+		if (retval >= 0)
+			retval = -EIO;
 		goto _error;
+	}
 
 	retval = search_binary_handler (bprm, regs);
 	if (retval < 0)


--- orig/fs/binfmt_script.c
+++ mod/fs/binfmt_script.c
@@ -91,8 +91,11 @@
 
 	bprm->file = file;
 	retval = prepare_binprm(bprm);
-	if (retval < 0)
+	if (retval != BINPRM_BUF_SIZE) {
+		if (retval >= 0)
+			retval = -EIO;
 		return retval;
+	}
 	return search_binary_handler(bprm,regs);
 }
 


--- orig/fs/compat.c
+++ mod/fs/compat.c
@@ -1426,8 +1426,11 @@
 		goto out;
 
 	retval = prepare_binprm(bprm);
-	if (retval < 0)
+	if (retval != BINPRM_BUF_SIZE) {
+		if (retval >= 0)
+			retval = -EIO;
 		goto out;
+	}
 
 	retval = copy_strings_kernel(1, &bprm->filename, bprm);
 	if (retval < 0)


--- orig/fs/exec.c
+++ mod/fs/exec.c
@@ -1024,8 +1024,11 @@
 		bprm->file = file;
 		bprm->loader = loader;
 		retval = prepare_binprm(bprm);
-		if (retval<0)
+		if (retval != BINPRM_BUF_SIZE) {
+			if (retval >= 0)
+				retval = -EIO;
 			return retval;
+		}
 		/* should call search_binary_handler recursively here,
 		   but it does not matter */
 	    }
@@ -1139,8 +1142,11 @@
 		goto out;
 
 	retval = prepare_binprm(bprm);
-	if (retval < 0)
+	if (retval != BINPRM_BUF_SIZE) {
+		if (retval >= 0)
+			retval = -EIO;
 		goto out;
+	}
 
 	retval = copy_strings_kernel(1, &bprm->filename, bprm);
 	if (retval < 0)




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] kernel_read result fixes

[Andres Salomon]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

On Fri, 2004-12-24 at 02:24 -0500, Andres Salomon wrote:
> Hi,
> 
> A few potential vulnerabilities were pointed out by Katrina Tsipenyuk in
> <http://seclists.org/lists/linux-kernel/2004/Dec/1878.html>.  I haven't
> seen any discussion or fixes of the issue yet, so here's a patch
> (against 2.6.9).  The fixes are along the same lines as the previous
> binfmt_elf fixes.  There's one additional place (inside fs/binfmt_som.c)
> that a fix could be applied, but since that doesn't compile anyways, I
> didn't see a point in patching it.
> 
> 

Ok, you can ignore this; I believe the original advisory is bogus.
prepare_binprm ensures a 128 byte buffer that kernel_read data is copied
to; in case something smaller is copied in, the rest of the space is
zero'd out.  Thus, <128 reads are fine, and in many cases (as in
binfmt_script w/ tiny scripts less than 128 bytes in total) perfectly
valid.


-- 
Andres Salomon <dilinger@voxel.net>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] kernel_read result fixes

[Andrew Morton]

Andres Salomon <dilinger@voxel.net> wrote:
>
> A few potential vulnerabilities were pointed out by Katrina Tsipenyuk in
>  <http://seclists.org/lists/linux-kernel/2004/Dec/1878.html>.  I haven't
>  seen any discussion or fixes of the issue yet, so here's a patch
>  (against 2.6.9).  The fixes are along the same lines as the previous
>  binfmt_elf fixes.  There's one additional place (inside fs/binfmt_som.c)
>  that a fix could be applied, but since that doesn't compile anyways, I
>  didn't see a point in patching it.

This patch is very wrong.

--- 25/fs/exec.c~kernel_read-result-fixes	2004-12-27 00:39:57.999820768 -0800
+++ 25-akpm/fs/exec.c	2004-12-27 00:39:58.007819552 -0800
@@ -1028,8 +1028,11 @@ int search_binary_handler(struct linux_b
 		bprm->file = file;
 		bprm->loader = loader;
 		retval = prepare_binprm(bprm);
-		if (retval<0)
+		if (retval != BINPRM_BUF_SIZE) {
+			if (retval >= 0)
+				retval = -EIO;
 			return retval;
+		}

prepare_binprm() can and will return values less than 128 if the
executable's file is less than 128 bytes in size.

	linux:/home/akpm> egrep
	zsh: Input/output error: egrep
	linux:/home/akpm> cat /bin/egrep
	#!/bin/sh
	exec /bin/grep -E ${1+"$@"}
	linux:/home/akpm> 

Re: [PATCH] kernel_read result fixes

[Andres Salomon]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

On Wed, 2004-12-29 at 23:25 -0800, Andrew Morton wrote:
> Andres Salomon <dilinger@voxel.net> wrote:
> >
> > A few potential vulnerabilities were pointed out by Katrina Tsipenyuk in
> >  <http://seclists.org/lists/linux-kernel/2004/Dec/1878.html>.  I haven't
> >  seen any discussion or fixes of the issue yet, so here's a patch
> >  (against 2.6.9).  The fixes are along the same lines as the previous
> >  binfmt_elf fixes.  There's one additional place (inside fs/binfmt_som.c)
> >  that a fix could be applied, but since that doesn't compile anyways, I
> >  didn't see a point in patching it.
> 
> This patch is very wrong.
> 

Yep, I already followed up saying that.  I assume you're just going
through your inbox after vacation now; it should be there.  :)


-- 
Andres Salomon <dilinger@voxel.net>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]