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 <[email protected]>
--- 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |