aboutsummaryrefslogtreecommitdiffstats
path: root/fs/proc/generic.c
diff options
context:
space:
mode:
authorAlexey Dobriyan <adobriyan@sw.ru>2007-07-16 02:39:00 -0400
committerLinus Torvalds <torvalds@woody.linux-foundation.org>2007-07-16 12:05:39 -0400
commit786d7e1612f0b0adb6046f19b906609e4fe8b1ba (patch)
tree9d5f1623c19c9d3f84606ea160d57cd3c8c97ea9 /fs/proc/generic.c
parent5568b0e8028d966ddb16f0be44a9df1fcbd1dc8d (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.c32
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
765continue_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))