linux-kernel - Re: + fix-procfs-task-exe-symlink.patch added to -mm tree

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] [day] [month] [year] [list]

Message-Id: <1201735968.4028.11.camel@localhost.localdomain>
Date:	Wed, 30 Jan 2008 15:32:48 -0800
From:	Matt Helsley <matthltc@...ibm.com>
To:	Oleg Nesterov <oleg@...sign.ru>
Cc:	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	dhowells@...hat.com, ebiederm@...ssion.com, hch@....de,
	viro@...iv.linux.org.uk
Subject: Re: + fix-procfs-task-exe-symlink.patch added to -mm tree


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