diff options
author | Alexey Dobriyan <adobriyan@sw.ru> | 2007-07-16 02:39:00 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2007-07-16 12:05:39 -0400 |
commit | 786d7e1612f0b0adb6046f19b906609e4fe8b1ba (patch) | |
tree | 9d5f1623c19c9d3f84606ea160d57cd3c8c97ea9 /fs/proc/generic.c | |
parent | 5568b0e8028d966ddb16f0be44a9df1fcbd1dc8d (diff) |
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 @@ | |||
20 | #include <linux/namei.h> | 20 | #include <linux/namei.h> |
21 | #include <linux/bitops.h> | 21 | #include <linux/bitops.h> |
22 | #include <linux/spinlock.h> | 22 | #include <linux/spinlock.h> |
23 | #include <linux/completion.h> | ||
23 | #include <asm/uaccess.h> | 24 | #include <asm/uaccess.h> |
24 | 25 | ||
25 | #include "internal.h" | 26 | #include "internal.h" |
@@ -613,6 +614,9 @@ static struct proc_dir_entry *proc_create(struct proc_dir_entry **parent, | |||
613 | ent->namelen = len; | 614 | ent->namelen = len; |
614 | ent->mode = mode; | 615 | ent->mode = mode; |
615 | ent->nlink = nlink; | 616 | ent->nlink = nlink; |
617 | ent->pde_users = 0; | ||
618 | spin_lock_init(&ent->pde_unload_lock); | ||
619 | ent->pde_unload_completion = NULL; | ||
616 | out: | 620 | out: |
617 | return ent; | 621 | return ent; |
618 | } | 622 | } |
@@ -734,9 +738,35 @@ void remove_proc_entry(const char *name, struct proc_dir_entry *parent) | |||
734 | de = *p; | 738 | de = *p; |
735 | *p = de->next; | 739 | *p = de->next; |
736 | de->next = NULL; | 740 | de->next = NULL; |
741 | |||
742 | spin_lock(&de->pde_unload_lock); | ||
743 | /* | ||
744 | * Stop accepting new callers into module. If you're | ||
745 | * dynamically allocating ->proc_fops, save a pointer somewhere. | ||
746 | */ | ||
747 | de->proc_fops = NULL; | ||
748 | /* Wait until all existing callers into module are done. */ | ||
749 | if (de->pde_users > 0) { | ||
750 | DECLARE_COMPLETION_ONSTACK(c); | ||
751 | |||
752 | if (!de->pde_unload_completion) | ||
753 | de->pde_unload_completion = &c; | ||
754 | |||
755 | spin_unlock(&de->pde_unload_lock); | ||
756 | spin_unlock(&proc_subdir_lock); | ||
757 | |||
758 | wait_for_completion(de->pde_unload_completion); | ||
759 | |||
760 | spin_lock(&proc_subdir_lock); | ||
761 | goto continue_removing; | ||
762 | } | ||
763 | spin_unlock(&de->pde_unload_lock); | ||
764 | |||
765 | continue_removing: | ||
737 | if (S_ISDIR(de->mode)) | 766 | if (S_ISDIR(de->mode)) |
738 | parent->nlink--; | 767 | parent->nlink--; |
739 | proc_kill_inodes(de); | 768 | if (!S_ISREG(de->mode)) |
769 | proc_kill_inodes(de); | ||
740 | de->nlink = 0; | 770 | de->nlink = 0; |
741 | WARN_ON(de->subdir); | 771 | WARN_ON(de->subdir); |
742 | if (!atomic_read(&de->count)) | 772 | if (!atomic_read(&de->count)) |