BUGFIX: 2.1.106 (and earlier) /proc/<pid>/* does not honour setuid(2).

Tigran Aivazian (tigran@sco.COM)
Mon, 15 Jun 1998 09:11:47 +0000 (GMT)


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