linux-kernel - Re: /proc/pid/fd && anon_inode

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

Message-ID: <20130826153301.GA15890@redhat.com>
Date:	Mon, 26 Aug 2013 17:33:01 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Willy Tarreau <w@....eu>, Al Viro <viro@...iv.linux.org.uk>,
	Andy Lutomirski <luto@...capital.net>,
	"security@...nel.org" <security@...nel.org>,
	Ingo Molnar <mingo@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linux FS Devel <linux-fsdevel@...r.kernel.org>,
	Brad Spengler <spender@...ecurity.net>
Subject: Re: /proc/pid/fd && anon_inode_fops

On 08/25, Linus Torvalds wrote:
>
> On Sun, Aug 25, 2013 at 12:48 PM, Oleg Nesterov <oleg@...hat.com> wrote:
> >
> > pid_revalidate() does inode->i_*id = GLOBAL_ROOT_*ID if task_dumpable()
> > fails, but it can fail simply because ->mm = NULL.
> >
> > This means that almost everything in /proc/zombie-pid/ becomes root.
> > Doesn't really hurt, but for what? Looks a bit strange imho.
>
> The zombie case shouldn't be relevant, because a zombie will have
> closed all the file descriptors anyway, so they no longer exist.

I specially mentioned that this is off-topic ;)

> That said, task_dumpable isn't wonderful, and I suspect we could drop
> that logic entirely in the tid-fd case if we just use f_cred.

Probably yes, but I do not understand this S_IFLNK && uid/chmod magic
in tid_fd_revalidate(). And afaics this should not affect readlink()
anyway. So yes, ->f_cred makes more sense to me, but I can't comment.


But, afaics, speaking of task_dumpable() this doesn't matter. Please
forget about /proc/fd or zombies. I can't even understand
proc_pid_make_inode() or pid_revalidate().

	$ id
	uid=1000(tst) gid=100(users) groups=100(users)
	$ cp `which ls` ls
	$ chmod a-r ./ls
	$
	$ ./ls -l /proc/self/
	total 0
	-r-------- 1 root root  0 Aug 26 06:35 auxv
	-r--r--r-- 1 root root  0 Aug 26 06:35 cgroup
	--w------- 1 root root  0 Aug 26 06:35 clear_refs
	-r--r--r-- 1 root root  0 Aug 26 06:35 cmdline
	-rw-r--r-- 1 root root  0 Aug 26 06:35 comm
	-rw-r--r-- 1 root root  0 Aug 26 06:35 coredump_filter
	lrwxrwxrwx 1 root root  0 Aug 26 06:35 cwd -> /home/tst
	-r-------- 1 root root  0 Aug 26 06:35 environ
	lrwxrwxrwx 1 root root  0 Aug 26 06:35 exe -> /home/tst/ls
	dr-x------ 2 root root  0 Aug 26 06:35 fd
	dr-x------ 2 root root  0 Aug 26 06:35 fdinfo
	-rw-r--r-- 1 root root  0 Aug 26 06:35 gid_map
	-r--r--r-- 1 root root  0 Aug 26 06:35 limits
	-r--r--r-- 1 root root  0 Aug 26 06:35 maps
	-rw------- 1 root root  0 Aug 26 06:35 mem
	-r--r--r-- 1 root root  0 Aug 26 06:35 mountinfo
	-r--r--r-- 1 root root  0 Aug 26 06:35 mounts
	-r-------- 1 root root  0 Aug 26 06:35 mountstats
	dr-xr-xr-x 4 tst  users 0 Aug 26 06:35 net
	dr-x--x--x 2 root root  0 Aug 26 06:35 ns
	-rw-r--r-- 1 root root  0 Aug 26 06:35 oom_adj
	-r--r--r-- 1 root root  0 Aug 26 06:35 oom_score
	-rw-r--r-- 1 root root  0 Aug 26 06:35 oom_score_adj
	-r--r--r-- 1 root root  0 Aug 26 06:35 pagemap
	-r--r--r-- 1 root root  0 Aug 26 06:35 personality
	-rw-r--r-- 1 root root  0 Aug 26 06:35 projid_map
	lrwxrwxrwx 1 root root  0 Aug 26 06:35 root -> /
	-r--r--r-- 1 root root  0 Aug 26 06:35 smaps
	-r--r--r-- 1 root root  0 Aug 26 06:35 stack
	-r--r--r-- 1 root root  0 Aug 26 06:35 stat
	-r--r--r-- 1 root root  0 Aug 26 06:35 statm
	-r--r--r-- 1 root root  0 Aug 26 06:35 status
	-r--r--r-- 1 root root  0 Aug 26 06:35 syscall
	dr-xr-xr-x 3 tst  users 0 Aug 26 06:35 task
	-rw-r--r-- 1 root root  0 Aug 26 06:35 uid_map
	-r--r--r-- 1 root root  0 Aug 26 06:35 wchan

For what? Say,

	-r--r--r-- 1 root root  0 Aug 26 06:35 status

but it is S_IRUGO anyway, why do we need to change the owner?

	dr-x------ 2 root root  0 Aug 26 06:35 fd

OK, this means that I can't access this dir from another process.
Not sure we really want this in this case but

	$ ./ls /proc/self/fd
	0  1  2  3

still works, I guess thanks to proc_fd_permission().

However, say,

	-r-------- 1 root root  0 Aug 26 06:35 mountstats

actually becomes unreadable even via /proc/self/.

Imho, this all is confusing. Perhaps it makes sense to "chmod", say,
/proc/pid/maps if !task_dumpable(), but "chown" looks strange.

Oleg.

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