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

From: Linus Torvalds
Date: Mon Jun 22 2009 - 16:52:33 EST

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


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);
+ allow_write_access(bprm.file);
+ fput(bprm.file);

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at