diff options
author | Alexey Dobriyan <adobriyan@sw.ru> | 2007-07-15 23:39:00 -0700 |
---|---|---|
committer | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2007-07-16 09:05:39 -0700 |
commit | 786d7e1612f0b0adb6046f19b906609e4fe8b1ba (patch) | |
tree | 9d5f1623c19c9d3f84606ea160d57cd3c8c97ea9 /fs/proc/generic.c | |
parent | 5568b0e8028d966ddb16f0be44a9df1fcbd1dc8d (diff) | |
download | lwn-786d7e1612f0b0adb6046f19b906609e4fe8b1ba.tar.gz lwn-786d7e1612f0b0adb6046f19b906609e4fe8b1ba.zip |
Fix rmmod/read/write races in /proc entries
Fix following races:
===========================================
1. Write via ->write_proc sleeps in copy_from_user(). Module disappears
meanwhile. Or, more generically, system call done on /proc file, method
supplied by module is called, module dissapeares meanwhile.
pde = create_proc_entry()
if (!pde)
return -ENOMEM;
pde->write_proc = ...
open
write
copy_from_user
pde = create_proc_entry();
if (!pde) {
remove_proc_entry();
return -ENOMEM;
/* module unloaded */
}
*boom*
==========================================
2. bogo-revoke aka proc_kill_inodes()
remove_proc_entry vfs_read
proc_kill_inodes [check ->f_op validness]
[check ->f_op->read validness]
[verify_area, security permissions checks]
->f_op = NULL;
if (file->f_op->read)
/* ->f_op dereference, boom */
NOTE, NOTE, NOTE: file_operations are proxied for regular files only. Let's
see how this scheme behaves, then extend if needed for directories.
Directories creators in /proc only set ->owner for them, so proxying for
directories may be unneeded.
NOTE, NOTE, NOTE: methods being proxied are ->llseek, ->read, ->write,
->poll, ->unlocked_ioctl, ->ioctl, ->compat_ioctl, ->open, ->release.
If your in-tree module uses something else, yell on me. Full audit pending.
[akpm@linux-foundation.org: build fix]
Signed-off-by: Alexey Dobriyan <adobriyan@sw.ru>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'fs/proc/generic.c')
-rw-r--r-- | fs/proc/generic.c | 32 |
1 files changed, 31 insertions, 1 deletions
diff --git a/fs/proc/generic.c b/fs/proc/generic.c index 8a40e15f5ecb..4f8e53568b22 100644 --- a/fs/proc/generic.c +++ b/fs/proc/generic.c @@ -20,6 +20,7 @@ #include <linux/namei.h> #include <linux/bitops.h> #include <linux/spinlock.h> +#include <linux/completion.h> #include <asm/uaccess.h> #include "internal.h" @@ -613,6 +614,9 @@ static struct proc_dir_entry *proc_create(struct proc_dir_entry **parent, ent->namelen = len; ent->mode = mode; ent->nlink = nlink; + ent->pde_users = 0; + spin_lock_init(&ent->pde_unload_lock); + ent->pde_unload_completion = NULL; out: return ent; } @@ -734,9 +738,35 @@ void remove_proc_entry(const char *name, struct proc_dir_entry *parent) de = *p; *p = de->next; de->next = NULL; + + spin_lock(&de->pde_unload_lock); + /* + * Stop accepting new callers into module. If you're + * dynamically allocating ->proc_fops, save a pointer somewhere. + */ + de->proc_fops = NULL; + /* Wait until all existing callers into module are done. */ + if (de->pde_users > 0) { + DECLARE_COMPLETION_ONSTACK(c); + + if (!de->pde_unload_completion) + de->pde_unload_completion = &c; + + spin_unlock(&de->pde_unload_lock); + spin_unlock(&proc_subdir_lock); + + wait_for_completion(de->pde_unload_completion); + + spin_lock(&proc_subdir_lock); + goto continue_removing; + } + spin_unlock(&de->pde_unload_lock); + +continue_removing: if (S_ISDIR(de->mode)) parent->nlink--; - proc_kill_inodes(de); + if (!S_ISREG(de->mode)) + proc_kill_inodes(de); de->nlink = 0; WARN_ON(de->subdir); if (!atomic_read(&de->count)) |