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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 22 Jun 2009 13:48:44 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Mike Frysinger <vapier@...too.org>
cc:	linux-kernel@...r.kernel.org,
	uclinux-dist-devel@...ckfin.uclinux.org, stable@...nel.org,
	David Howells <dhowells@...hat.com>,
	Bernd Schmidt <bernds_cb1@...nline.de>
Subject: Re: [PATCH] FLAT: fix uninitialized ptr with shared libs

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/