Re: Errors in load_elf_binary()?

[Chris Wright <chrisw@osdl.org>]

* Kirill Korotaev (kirillx@7ka.mipt.ru) wrote:
> File: fs/binfmt_elf.c
> 1.
> load_elf_binary()
> 2002/02/05 torvalds   | retval = kernel_read(bprm->file, elf_ex.e_phoff, (char *) elf_phdata, size);
> 2002/02/05 torvalds   | if (retval < 0)
> 2002/02/05 torvalds   |       goto out_free_ph;
> 2003/06/29 alan       |
> 2003/06/29 alan       | files = current->files;           /* Refcounted so ok */
> 2003/06/29 alan       | if(unshare_files() < 0)
> 2003/06/29 alan       |       goto out_free_ph;
> <<<< retval is not set >>>>
> should be something like:
> retval = unshare_files()
> if (retval < 0)
>    goto ....;

yes, this looks like a bug.

> 2.
> load_elf_binary()
> 2002/02/05 torvalds   | out_free_dentry:
> 2002/02/05 torvalds   |       allow_write_access(interpreter);
> 2002/02/05 torvalds   |       fput(interpreter);
> <<<< interpreter can be NULL >>>>
> e.g. we got oopses here when flush_old_exec()
> returns error
> should be something like:
> if (interpreter)
>    fput(interpreter);

yup, this change is already in 2.6.

> 3.
> load_elf_binary()
> Why there is no steal_locks() call in exit path (after label 
> "out_free_fh")? Shouldn't were steal locks back when undoing our changes?

No, on this error path locks never got stolen to begin with.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net