On 08/25, Linus Torvalds wrote: > > On Sun, Aug 25, 2013 at 12:48 PM, Oleg Nesterov <oleg@xxxxxxxxxx> 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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
