Messages in this thread | | | Date | Mon, 26 Aug 2013 17:33:01 +0200 | From | Oleg Nesterov <> | 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@redhat.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.
| |