kernel_read result fixes

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 <[email protected]>

Revision: linux-fs--kernel-read-vuln--0--patch-1
Archive: [email protected]
Creator: Andres Salomon <[email protected]>
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: [email protected]/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: [email protected]
Creator: Andres Salomon <[email protected]>
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: [email protected]/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.

@@ -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);
 }

@@ -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);

@@ -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)

@@ -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);
 }
 

@@ -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)

@@ -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)

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 <[email protected]>

This patch is very wrong.

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

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 <[email protected]>