Messages in this thread Patch in this message | | | Date | Mon, 15 Jun 1998 09:11:47 +0000 (GMT) | From | Tigran Aivazian <> | Subject | BUGFIX: 2.1.106 (and earlier) /proc/<pid>/* does not honour setuid(2). |
| |
Dear Linus,
This is the patch to fs/proc/base.c that fixes the following bug:
a) Let us consider a program that does o setuid(500) /* 500 is some valid uid in /etc/passwd */ o prints pid and uid, hereafter pid is denoted by <pid> o while (1) sleep(5);
b) Let us run this program as user 500 and look at uids of inodes /proc/<pid> and /proc/<pid>/*. They will all be set to 500. This is correct.
c) Now let us run this program as root. The uid of /proc/<pid> directory is 500 but all the files in /proc/<pid>/* retained their old uid of 0 (root). This is incorrect. A similar behaviour will happen if you replace uid with gid and setuid with setgid. The patch below fixes this.
Without writing any special program it is trivial to notice the bug - just do ps aux | grep login and find non-root login, then look at corresponding /proc/<pid>/*.
Yesterday, Vadim and I were looking at this and came to conclusion that by removing the test if(p->dumpable || ino == PROC_PID_INO) from proc_pid_fill_inode() of fs/proc/base.c the problem is solved.
However, I am not 100% sure that this is the right way, simply because I do not know why that check is there in the first place. I mean other proc_dir_entry entries like proc_pid_cwd, proc_pid_root etc. all use proc_pid_fill_inode() as their fill_inode() operation but the function itself tries to avoid their low_ino (and also avoids them completely if they smell like a process that did setuid()/setfsuid()/setreuid() and thus became non-dumpable).
If the if() check of proc_pid_fill_inode() is really required for the reason that escapes my imagination then, perhaps, we need a separate fill_inode() candidate for the other proc entries?
Hmmm... perhaps I should learn how to access the CVS tree on vger and then I would be able to request the history for fs/proc/base.c and discover the purpose that if() serves, unless it was there from 0.99plXX :)
Regards, ------ Tigran A. Aivazian | http://www.sco.com/ Escalations Research Group | Email: tigran@sco.com Santa Cruz Operation Ltd |
Below is the patch --- linux/fs/proc/base.c Wed Mar 11 23:45:53 1998 +++ linux-2.1.106.new/fs/proc/base.c Mon Jun 15 08:45:33 1998 @@ -62,10 +62,8 @@ read_lock(&tasklist_lock); if (fill && (p = find_task_by_pid(pid)) != NULL) { - if (p->dumpable || ino == PROC_PID_INO) { - inode->i_uid = p->euid; - inode->i_gid = p->gid; - } + inode->i_uid = p->euid; + inode->i_gid = p->gid; } read_unlock(&tasklist_lock); }
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu
| |