Messages in this thread Patch in this message | | | Date | Fri, 16 Sep 2005 10:06:53 -0500 | From | Sripathi Kodi <> | Subject | Re: [PATCH 2.6.13.1] Patch for invisible threads |
| |
Al Viro wrote: > Ugh... Considering that all of that is _only_ for /proc/<pid>/task and > that proc_permission() is a couple of function calls, why bother with > proc_task_check_root() instead of just adding proc_task_permission() with > > { > struct dentry *root; > struct vfsmount *vfsmnt; > > if (generic_permission(inode, mask, NULL) != 0) > return -EACCES; > > /* or just open-code it here, for that matter */ > if (proc_task_root_link(inode, &root, &vfsmnt)) > return -ENOENT; > > return proc_check_chroot(root, vfsmnt); > } > > for a body and leaving proc_permission() without any changes at all?
Al, Done. Please find the patch below. I retained proc_task_root_link, because it has significant amount of code in it.
> Right. The real question is whether the current behaviour makes any sense. > I've no objections to your patch + modification above, but I really wonder > if we should keep current rules in that area.
I don't know what would be the right behavior for this area. If you have any ideas for changes we could introduce here, I am ready to code and test it out.
Thanks and regards, Sripathi.
Signed-off-by: Sripathi Kodi <sripathik@in.ibm.com>
--- linux-2.6.13.1-orig/fs/proc/base.c 2005-09-16 17:22:44.000000000 -0500 +++ linux-2.6.13.1/fs/proc/base.c 2005-09-16 17:08:18.000000000 -0500 @@ -291,6 +291,52 @@ static int proc_root_link(struct inode * return result; }
+ +/* Same as proc_root_link, but this addionally tries to get fs from other + * threads in the group */ +static int proc_task_root_link(struct inode *inode, struct dentry **dentry, struct vfsmount **mnt) +{ + struct fs_struct *fs; + int result = -ENOENT; + struct task_struct *leader = proc_task(inode); + + task_lock(leader); + fs = leader->fs; + if (fs) { + atomic_inc(&fs->count); + task_unlock(leader); + } else { + /* Try to get fs from other threads */ + task_unlock(leader); + struct task_struct *task = leader; + read_lock(&tasklist_lock); + if (pid_alive(task)) { + while ((task = next_thread(task)) != leader) { + task_lock(task); + fs = task->fs; + if (fs) { + atomic_inc(&fs->count); + task_unlock(task); + break; + } + task_unlock(task); + } + } + read_unlock(&tasklist_lock); + } + + if (fs) { + read_lock(&fs->lock); + *mnt = mntget(fs->rootmnt); + *dentry = dget(fs->root); + read_unlock(&fs->lock); + result = 0; + put_fs_struct(fs); + } + return result; +} + + #define MAY_PTRACE(task) \ (task == current || \ (task->parent == current && \ @@ -449,14 +495,14 @@ static int proc_oom_score(struct task_st
/* permission checks */
-static int proc_check_root(struct inode *inode) +/* If the process being read is separated by chroot from the reading process, + * don't let the reader access the threads. + */ +static int proc_check_chroot(struct dentry *root, struct vfsmount *vfsmnt) { - struct dentry *de, *base, *root; - struct vfsmount *our_vfsmnt, *vfsmnt, *mnt; + struct dentry *de, *base; + struct vfsmount *our_vfsmnt, *mnt; int res = 0; - - if (proc_root_link(inode, &root, &vfsmnt)) /* Ewww... */ - return -ENOENT; read_lock(¤t->fs->lock); our_vfsmnt = mntget(current->fs->rootmnt); base = dget(current->fs->root); @@ -489,6 +535,16 @@ out: goto exit; }
+static int proc_check_root(struct inode *inode) +{ + struct dentry *root; + struct vfsmount *vfsmnt; + + if (proc_root_link(inode, &root, &vfsmnt)) /* Ewww... */ + return -ENOENT; + return proc_check_chroot(root, vfsmnt); +} + static int proc_permission(struct inode *inode, int mask, struct nameidata *nd) { if (generic_permission(inode, mask, NULL) != 0) @@ -496,6 +552,20 @@ static int proc_permission(struct inode return proc_check_root(inode); }
+static int proc_task_permission(struct inode *inode, int mask, struct nameidata *nd) +{ + struct dentry *root; + struct vfsmount *vfsmnt; + + if (generic_permission(inode, mask, NULL) != 0) + return -EACCES; + + if (proc_task_root_link(inode, &root, &vfsmnt)) + return -ENOENT; + + return proc_check_chroot(root, vfsmnt); +} + extern struct seq_operations proc_pid_maps_op; static int maps_open(struct inode *inode, struct file *file) { @@ -1355,7 +1425,7 @@ static struct inode_operations proc_fd_i
static struct inode_operations proc_task_inode_operations = { .lookup = proc_task_lookup, - .permission = proc_permission, + .permission = proc_task_permission, };
#ifdef CONFIG_SECURITY - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |