Re: [PATCH] FLAT: fix uninitialized ptr with shared libs

[Linus Torvalds]

On Mon, 22 Jun 2009, Mike Frysinger wrote:
>
> should be cleaned up to apply to the master branch

Hmm. Can somebody explain why we even bother to test bprm.cred for NULL 
that second time?

Or why we test bprm.file, for that matter? How could it possibly suddenly 
become NULL?

Finally, if that bprm.cred _is_ NULL in the first test, then 'res' will be 
NULL (from the previous statement), and with the "goto out" we'll return 
_success_ from this function when we failed to allocate a cred.

IOW, the whole patch really seems to be total and utter crap. Why didn't 
people spend a bit more effort lookin at it?

IOW, shouldn't the patch be something like the appended?

UNTESTED. I did not compile this, or the previous patch. I have not tried 
it. I'm not going to commit it. I'm hoping to get a patch back that is 
tested and/or explains my concerns with the previous one..

		Linus

---
 fs/binfmt_flat.c |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 697f6b5..e92f229 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -828,15 +828,22 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
 	if (IS_ERR(bprm.file))
 		return res;
 
+	bprm.cred = prepare_exec_creds();
+	res = -ENOMEM;
+	if (!bprm.cred)
+		goto out;
+
 	res = prepare_binprm(&bprm);
 
 	if (res <= (unsigned long)-4096)
 		res = load_flat_file(&bprm, libs, id, NULL);
-	if (bprm.file) {
-		allow_write_access(bprm.file);
-		fput(bprm.file);
-		bprm.file = NULL;
-	}
+
+	abort_creds(bprm.cred);
+
+out:
+	allow_write_access(bprm.file);
+	fput(bprm.file);
+
 	return(res);
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/