[Trivial Patch] Bad tgid and tid lookup for /proc

[Trivial Patch] Bad tgid and tid lookup for /proc

[Kingsley Cheung]

All,

On 2.6.2, one can do the following, which is clearly wrong:

gen2 02:49:45 ~: cat /proc/1/task/$$/stat
1669 (bash) S 1668 1669 1669 34816 1730 256 1480 6479 12 4 8 5 5 17 15 0 1 0 8065 3252224 451 4294967295 134512640 134955932 3221225104 3221222840 4294960144 0 65536 3686404 1266761467 3222442959 0 0 17 0 0 0
gen2 02:50:44 ~: ls /proc/1/task
1
gen2 02:50:47 ~: cat /proc/$$/task/1/stat
1 (init) S 0 0 0 0 -1 256 109 190731 7 116 0 548 706 715 16 0 1 0 785 638976 61 4294967295 134512640 134982008 3221225168 3221222224 134598471 0 0 1467013372 680207875 3222467003 0 0 0 0 0 0
gen2 02:50:54 ~: ls /proc/$$/task
1669

The following trivial patch against 2.6.2 (applies also to 2.6.3-rc1)
fixes it:

--- Linux-2.6.1/fs/proc/base.old.c      Mon Feb  9 12:53:44 2004
+++ Linux-2.6.1/fs/proc/base.c  Mon Feb  9 13:02:52 2004
@@ -1739,14 +1739,13 @@
        read_unlock(&tasklist_lock);
        if (!task)
                goto out;
+       if (!thread_group_leader(task))
+               goto out_drop_task;
 
        inode = proc_pid_make_inode(dir->i_sb, task, PROC_TGID_INO);
 
-
-       if (!inode) {
-               put_task_struct(task);
-               goto out;
-       }
+       if (!inode)
+               goto out_drop_task;
        inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
        inode->i_op = &proc_tgid_base_inode_operations;
        inode->i_fop = &proc_tgid_base_operations;
@@ -1771,6 +1770,8 @@
                goto out;
        }
        return NULL;
+out_drop_task:
+       put_task_struct(task);
 out:
        return ERR_PTR(-ENOENT);
 }
@@ -1779,6 +1780,7 @@
 static struct dentry *proc_task_lookup(struct inode *dir, struct dentry * dentr
y, struct nameidata *nd)
 {
        struct task_struct *task;
+       struct task_struct *leader = proc_task(dir);
        struct inode *inode;
        unsigned tid;
 
@@ -1793,14 +1795,14 @@
        read_unlock(&tasklist_lock);
        if (!task)
                goto out;
+       if (leader->tgid != task->tgid)
+               goto out_drop_task;
 
        inode = proc_pid_make_inode(dir->i_sb, task, PROC_TID_INO);
 
 
-       if (!inode) {
-               put_task_struct(task);
-               goto out;
-       }
+       if (!inode)
+               goto out_drop_task;
        inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
        inode->i_op = &proc_tid_base_inode_operations;
        inode->i_fop = &proc_tid_base_operations;
@@ -1813,6 +1815,8 @@
 
        put_task_struct(task);
        return NULL;
+out_drop_task:
+       put_task_struct(task);
 out:
        return ERR_PTR(-ENOENT);
 }


-- 
		Kingsley

Re: [Trivial Patch] Bad tgid and tid lookup for /proc

[Rusty Russell]

In message <20040209135110.H17768@aurema.com> you write:
> All,
> 
> On 2.6.2, one can do the following, which is clearly wrong:
> 
> gen2 02:49:45 ~: cat /proc/1/task/$$/stat
> 1669 (bash) S 1668 1669 1669 34816 1730 256 1480 6479 12 4 8 5 5 17 15 0 1 0 
8065 3252224 451 4294967295 134512640 134955932 3221225104 3221222840 429496014
4 0 65536 3686404 1266761467 3222442959 0 0 17 0 0 0
> gen2 02:50:44 ~: ls /proc/1/task
> 1

Patch was mangled, and IMHO wasn't exactly trivial.  As I understand
it, you could access any task under /proc/xxx/task.  Your patch seems to
make it that you can access any task under /proc/xxx/task if xxx is a
thread group leader.

A little confused,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [Trivial Patch] Bad tgid and tid lookup for /proc

[Kingsley Cheung]

On Fri, Mar 12, 2004 at 09:09:09AM +1100, Rusty Russell wrote:
> In message <20040209135110.H17768@aurema.com> you write:
> > All,
> > 
> > On 2.6.2, one can do the following, which is clearly wrong:
> > 
> > gen2 02:49:45 ~: cat /proc/1/task/$$/stat
> > 1669 (bash) S 1668 1669 1669 34816 1730 256 1480 6479 12 4 8 5 5 17 15 0 1 0 
> 8065 3252224 451 4294967295 134512640 134955932 3221225104 3221222840 429496014
> 4 0 65536 3686404 1266761467 3222442959 0 0 17 0 0 0
> > gen2 02:50:44 ~: ls /proc/1/task
> > 1
> 
> Patch was mangled, and IMHO wasn't exactly trivial.  As I understand
> it, you could access any task under /proc/xxx/task.  Your patch seems to
> make it that you can access any task under /proc/xxx/task if xxx is a
> thread group leader.
> 
> A little confused,
> Rusty.

Rusty,

Andrew Morton, Peter Chubb, and a few others actually had a discussion
and resolved some issues around that patch.  So thanks to them I guess
there's no need to be confused ;)

Thanks for looking at it anyway.

-- 
		Kingsley