On Wed, 2008-01-30 at 14:06 +0300, Oleg Nesterov wrote:
<snip>
> Err, I was double wrong. It _is_ trivial to set ->exe_file before exec_mmap(),
>
> flush_old_exec:
>
> + get_file(bprm->file);
> + set_mm_exe_file(bprm->mm, bprm->file);
> retval = exec_mmap(bprm->mm);
> if (retval)
> goto mmap_failed;
>
> bprm->mm = NULL; /* We're using it now */
>
> If exec_mmap() fails, the caller (do_execve) has to mmput(bprm->mm)
> anyway, and this imply set_mm_exe_file(NULL). This way set_mm_exe_file()
> doesn't need any locking.
>
> Not that this is relly important, but still.
I've got the patch written and will be testing this shortly.
> However. I didn't notice this patch plays with #ifdef CONFIG_PROC_FS.
There are portions in there to deal with the presence or lack of proc
fs. In include/linux/mm_types.h for example. Also, I placed some of the
functions in fs/proc/base.c and avoided the need for #ifdefs in the .c
files.
> Without CONFIG_PROC_FS we seem to leak bprm->file, I'd suggest to move
> get_file(bprm->file) into set_mm_exe_file().
Good catch. Also I noticed that I was still setting the exe_file field
in fork.c regardles of CONFIG_PROC_FS.
So based on your feedback I'm working on 3 patches:
1 - fix the two CONFIG_PROC_FS issues
a) breaks compilation when CONFIG_PROC_FS is not defined
b) struct file leak when CONFIG_PROC_FS is not defined
2 - reuse mmap semaphore
3 - move the mm initialization bits in the exec path to close the window
where userspace could see a "NULL" exe_file.
I will post each after it passes testing.
Thanks again!
Cheers,
-Matt Helsley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
