Bad tgid and tid lookup for /proc

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:

@@ -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

In message <[email protected]> you write:

8065 3252224 451 4294967295 134512640 134955932 3221225104 3221222840 429496014 4 0 65536 3686404 1266761467 3222442959 0 0 17 0 0 0

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.

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