Messages in this thread | | | Subject | Re: + fix-procfs-task-exe-symlink.patch added to -mm tree | From | Matt Helsley <> | Date | Wed, 30 Jan 2008 15:32:48 -0800 |
| |
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
| |