Commit 64a07bd8 authored by Steven Rostedt's avatar Steven Rostedt Committed by Linus Torvalds

[PATCH] protect remove_proc_entry

It has been discovered that the remove_proc_entry has a race in the removing
of entries in the proc file system that are siblings.  There's no protection
around the traversing and removing of elements that belong in the same
subdirectory.

This subdirectory list is protected in other areas by the BKL.  So the BKL was
at first used to protect this area too, but unfortunately, remove_proc_entry
may be called with spinlocks held.  The BKL may schedule, so this was not a
solution.

The final solution was to add a new global spin lock to protect this list,
called proc_subdir_lock.  This lock now protects the list in
remove_proc_entry, and I also went around looking for other areas that this
list is modified and added this protection there too.  Care must be taken
since these locations call several functions that may also schedule.

Since I don't see any location that these functions that modify the
subdirectory list are called by interrupts, the irqsave/restore versions of
the spin lock was _not_ used.
Signed-off-by: default avatarSteven Rostedt <rostedt@goodmis.org>
Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent cd7b24bb
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include <linux/idr.h> #include <linux/idr.h>
#include <linux/namei.h> #include <linux/namei.h>
#include <linux/bitops.h> #include <linux/bitops.h>
#include <linux/spinlock.h>
#include <asm/uaccess.h> #include <asm/uaccess.h>
#include "internal.h" #include "internal.h"
...@@ -29,6 +30,8 @@ static ssize_t proc_file_write(struct file *file, const char __user *buffer, ...@@ -29,6 +30,8 @@ static ssize_t proc_file_write(struct file *file, const char __user *buffer,
size_t count, loff_t *ppos); size_t count, loff_t *ppos);
static loff_t proc_file_lseek(struct file *, loff_t, int); static loff_t proc_file_lseek(struct file *, loff_t, int);
DEFINE_SPINLOCK(proc_subdir_lock);
int proc_match(int len, const char *name, struct proc_dir_entry *de) int proc_match(int len, const char *name, struct proc_dir_entry *de)
{ {
if (de->namelen != len) if (de->namelen != len)
...@@ -277,7 +280,9 @@ static int xlate_proc_name(const char *name, ...@@ -277,7 +280,9 @@ static int xlate_proc_name(const char *name,
const char *cp = name, *next; const char *cp = name, *next;
struct proc_dir_entry *de; struct proc_dir_entry *de;
int len; int len;
int rtn = 0;
spin_lock(&proc_subdir_lock);
de = &proc_root; de = &proc_root;
while (1) { while (1) {
next = strchr(cp, '/'); next = strchr(cp, '/');
...@@ -289,13 +294,17 @@ static int xlate_proc_name(const char *name, ...@@ -289,13 +294,17 @@ static int xlate_proc_name(const char *name,
if (proc_match(len, cp, de)) if (proc_match(len, cp, de))
break; break;
} }
if (!de) if (!de) {
return -ENOENT; rtn = -ENOENT;
goto out;
}
cp += len + 1; cp += len + 1;
} }
*residual = cp; *residual = cp;
*ret = de; *ret = de;
return 0; out:
spin_unlock(&proc_subdir_lock);
return rtn;
} }
static DEFINE_IDR(proc_inum_idr); static DEFINE_IDR(proc_inum_idr);
...@@ -380,6 +389,7 @@ struct dentry *proc_lookup(struct inode * dir, struct dentry *dentry, struct nam ...@@ -380,6 +389,7 @@ struct dentry *proc_lookup(struct inode * dir, struct dentry *dentry, struct nam
int error = -ENOENT; int error = -ENOENT;
lock_kernel(); lock_kernel();
spin_lock(&proc_subdir_lock);
de = PDE(dir); de = PDE(dir);
if (de) { if (de) {
for (de = de->subdir; de ; de = de->next) { for (de = de->subdir; de ; de = de->next) {
...@@ -388,12 +398,15 @@ struct dentry *proc_lookup(struct inode * dir, struct dentry *dentry, struct nam ...@@ -388,12 +398,15 @@ struct dentry *proc_lookup(struct inode * dir, struct dentry *dentry, struct nam
if (!memcmp(dentry->d_name.name, de->name, de->namelen)) { if (!memcmp(dentry->d_name.name, de->name, de->namelen)) {
unsigned int ino = de->low_ino; unsigned int ino = de->low_ino;
spin_unlock(&proc_subdir_lock);
error = -EINVAL; error = -EINVAL;
inode = proc_get_inode(dir->i_sb, ino, de); inode = proc_get_inode(dir->i_sb, ino, de);
spin_lock(&proc_subdir_lock);
break; break;
} }
} }
} }
spin_unlock(&proc_subdir_lock);
unlock_kernel(); unlock_kernel();
if (inode) { if (inode) {
...@@ -447,11 +460,13 @@ int proc_readdir(struct file * filp, ...@@ -447,11 +460,13 @@ int proc_readdir(struct file * filp,
filp->f_pos++; filp->f_pos++;
/* fall through */ /* fall through */
default: default:
spin_lock(&proc_subdir_lock);
de = de->subdir; de = de->subdir;
i -= 2; i -= 2;
for (;;) { for (;;) {
if (!de) { if (!de) {
ret = 1; ret = 1;
spin_unlock(&proc_subdir_lock);
goto out; goto out;
} }
if (!i) if (!i)
...@@ -461,12 +476,16 @@ int proc_readdir(struct file * filp, ...@@ -461,12 +476,16 @@ int proc_readdir(struct file * filp,
} }
do { do {
/* filldir passes info to user space */
spin_unlock(&proc_subdir_lock);
if (filldir(dirent, de->name, de->namelen, filp->f_pos, if (filldir(dirent, de->name, de->namelen, filp->f_pos,
de->low_ino, de->mode >> 12) < 0) de->low_ino, de->mode >> 12) < 0)
goto out; goto out;
spin_lock(&proc_subdir_lock);
filp->f_pos++; filp->f_pos++;
de = de->next; de = de->next;
} while (de); } while (de);
spin_unlock(&proc_subdir_lock);
} }
ret = 1; ret = 1;
out: unlock_kernel(); out: unlock_kernel();
...@@ -500,9 +519,13 @@ static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp ...@@ -500,9 +519,13 @@ static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp
if (i == 0) if (i == 0)
return -EAGAIN; return -EAGAIN;
dp->low_ino = i; dp->low_ino = i;
spin_lock(&proc_subdir_lock);
dp->next = dir->subdir; dp->next = dir->subdir;
dp->parent = dir; dp->parent = dir;
dir->subdir = dp; dir->subdir = dp;
spin_unlock(&proc_subdir_lock);
if (S_ISDIR(dp->mode)) { if (S_ISDIR(dp->mode)) {
if (dp->proc_iops == NULL) { if (dp->proc_iops == NULL) {
dp->proc_fops = &proc_dir_operations; dp->proc_fops = &proc_dir_operations;
...@@ -694,6 +717,8 @@ void remove_proc_entry(const char *name, struct proc_dir_entry *parent) ...@@ -694,6 +717,8 @@ void remove_proc_entry(const char *name, struct proc_dir_entry *parent)
if (!parent && xlate_proc_name(name, &parent, &fn) != 0) if (!parent && xlate_proc_name(name, &parent, &fn) != 0)
goto out; goto out;
len = strlen(fn); len = strlen(fn);
spin_lock(&proc_subdir_lock);
for (p = &parent->subdir; *p; p=&(*p)->next ) { for (p = &parent->subdir; *p; p=&(*p)->next ) {
if (!proc_match(len, fn, *p)) if (!proc_match(len, fn, *p))
continue; continue;
...@@ -714,6 +739,7 @@ void remove_proc_entry(const char *name, struct proc_dir_entry *parent) ...@@ -714,6 +739,7 @@ void remove_proc_entry(const char *name, struct proc_dir_entry *parent)
} }
break; break;
} }
spin_unlock(&proc_subdir_lock);
out: out:
return; return;
} }
...@@ -136,9 +136,11 @@ void proc_device_tree_add_node(struct device_node *np, ...@@ -136,9 +136,11 @@ void proc_device_tree_add_node(struct device_node *np,
* properties are quite unimportant for us though, thus we * properties are quite unimportant for us though, thus we
* simply "skip" them here, but we do have to check. * simply "skip" them here, but we do have to check.
*/ */
spin_lock(&proc_subdir_lock);
for (ent = de->subdir; ent != NULL; ent = ent->next) for (ent = de->subdir; ent != NULL; ent = ent->next)
if (!strcmp(ent->name, pp->name)) if (!strcmp(ent->name, pp->name))
break; break;
spin_unlock(&proc_subdir_lock);
if (ent != NULL) { if (ent != NULL) {
printk(KERN_WARNING "device-tree: property \"%s\" name" printk(KERN_WARNING "device-tree: property \"%s\" name"
" conflicts with node in %s\n", pp->name, " conflicts with node in %s\n", pp->name,
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include <linux/config.h> #include <linux/config.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/fs.h> #include <linux/fs.h>
#include <linux/spinlock.h>
#include <asm/atomic.h> #include <asm/atomic.h>
/* /*
...@@ -92,6 +93,8 @@ extern struct proc_dir_entry *proc_bus; ...@@ -92,6 +93,8 @@ extern struct proc_dir_entry *proc_bus;
extern struct proc_dir_entry *proc_root_driver; extern struct proc_dir_entry *proc_root_driver;
extern struct proc_dir_entry *proc_root_kcore; extern struct proc_dir_entry *proc_root_kcore;
extern spinlock_t proc_subdir_lock;
extern void proc_root_init(void); extern void proc_root_init(void);
extern void proc_misc_init(void); extern void proc_misc_init(void);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment