aboutsummaryrefslogtreecommitdiffstats
path: root/fs
diff options
context:
space:
mode:
authorAlexey Dobriyan <adobriyan@sw.ru>2007-12-05 02:45:28 -0500
committerLinus Torvalds <torvalds@woody.linux-foundation.org>2007-12-05 12:21:20 -0500
commit5a622f2d0f86b316b07b55a4866ecb5518dd1cf7 (patch)
treef89bb875b5b440dced320336db6cac24b9e4d8b7 /fs
parentd4beaf4ab5f89496f2bcf67db62ad95d99bfeff6 (diff)
proc: fix proc_dir_entry refcounting
Creating PDEs with refcount 0 and "deleted" flag has problems (see below). Switch to usual scheme: * PDE is created with refcount 1 * every de_get does +1 * every de_put() and remove_proc_entry() do -1 * once refcount reaches 0, PDE is freed. This elegantly fixes at least two following races (both observed) without introducing new locks, without abusing old locks, without spreading lock_kernel(): 1) PDE leak remove_proc_entry de_put ----------------- ------ [refcnt = 1] if (atomic_read(&de->count) == 0) if (atomic_dec_and_test(&de->count)) if (de->deleted) /* also not taken! */ free_proc_entry(de); else de->deleted = 1; [refcount=0, deleted=1] 2) use after free remove_proc_entry de_put ----------------- ------ [refcnt = 1] if (atomic_dec_and_test(&de->count)) if (atomic_read(&de->count) == 0) free_proc_entry(de); /* boom! */ if (de->deleted) free_proc_entry(de); BUG: unable to handle kernel paging request at virtual address 6b6b6b6b printing eip: c10acdda *pdpt = 00000000338f8001 *pde = 0000000000000000 Oops: 0000 [#1] PREEMPT SMP Modules linked in: af_packet ipv6 cpufreq_ondemand loop serio_raw psmouse k8temp hwmon sr_mod cdrom Pid: 23161, comm: cat Not tainted (2.6.24-rc2-8c0863403f109a43d7000b4646da4818220d501f #4) EIP: 0060:[<c10acdda>] EFLAGS: 00210097 CPU: 1 EIP is at strnlen+0x6/0x18 EAX: 6b6b6b6b EBX: 6b6b6b6b ECX: 6b6b6b6b EDX: fffffffe ESI: c128fa3b EDI: f380bf34 EBP: ffffffff ESP: f380be44 DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 Process cat (pid: 23161, ti=f380b000 task=f38f2570 task.ti=f380b000) Stack: c10ac4f0 00000278 c12ce000 f43cd2a8 00000163 00000000 7da86067 00000400 c128fa20 00896b18 f38325a8 c128fe20 ffffffff 00000000 c11f291e 00000400 f75be300 c128fa20 f769c9a0 c10ac779 f380bf34 f7bfee70 c1018e6b f380bf34 Call Trace: [<c10ac4f0>] vsnprintf+0x2ad/0x49b [<c10ac779>] vscnprintf+0x14/0x1f [<c1018e6b>] vprintk+0xc5/0x2f9 [<c10379f1>] handle_fasteoi_irq+0x0/0xab [<c1004f44>] do_IRQ+0x9f/0xb7 [<c117db3b>] preempt_schedule_irq+0x3f/0x5b [<c100264e>] need_resched+0x1f/0x21 [<c10190ba>] printk+0x1b/0x1f [<c107c8ad>] de_put+0x3d/0x50 [<c107c8f8>] proc_delete_inode+0x38/0x41 [<c107c8c0>] proc_delete_inode+0x0/0x41 [<c1066298>] generic_delete_inode+0x5e/0xc6 [<c1065aa9>] iput+0x60/0x62 [<c1063c8e>] d_kill+0x2d/0x46 [<c1063fa9>] dput+0xdc/0xe4 [<c10571a1>] __fput+0xb0/0xcd [<c1054e49>] filp_close+0x48/0x4f [<c1055ee9>] sys_close+0x67/0xa5 [<c10026b6>] sysenter_past_esp+0x5f/0x85 ======================= Code: c9 74 0c f2 ae 74 05 bf 01 00 00 00 4f 89 fa 5f 89 d0 c3 85 c9 57 89 c7 89 d0 74 05 f2 ae 75 01 4f 89 f8 5f c3 89 c1 89 c8 eb 06 <80> 38 00 74 07 40 4a 83 fa ff 75 f4 29 c8 c3 90 90 90 57 83 c9 EIP: [<c10acdda>] strnlen+0x6/0x18 SS:ESP 0068:f380be44 Also, remove broken usage of ->deleted from reiserfs: if sget() succeeds, module is already pinned and remove_proc_entry() can't happen => nobody can mark PDE deleted. Dummy proc root in netns code is not marked with refcount 1. AFAICS, we never get it, it's just for proper /proc/net removal. I double checked CLONE_NETNS continues to work. Patch survives many hours of modprobe/rmmod/cat loops without new bugs which can be attributed to refcounting. Signed-off-by: Alexey Dobriyan <adobriyan@sw.ru> Cc: "Eric W. Biederman" <ebiederm@xmission.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'fs')
-rw-r--r--fs/proc/generic.c9
-rw-r--r--fs/proc/inode.c9
-rw-r--r--fs/proc/root.c1
-rw-r--r--fs/reiserfs/procfs.c6
4 files changed, 5 insertions, 20 deletions
diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 5fccfe222a63..8d49838e5554 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -595,6 +595,7 @@ static struct proc_dir_entry *proc_create(struct proc_dir_entry **parent,
595 ent->namelen = len; 595 ent->namelen = len;
596 ent->mode = mode; 596 ent->mode = mode;
597 ent->nlink = nlink; 597 ent->nlink = nlink;
598 atomic_set(&ent->count, 1);
598 ent->pde_users = 0; 599 ent->pde_users = 0;
599 spin_lock_init(&ent->pde_unload_lock); 600 spin_lock_init(&ent->pde_unload_lock);
600 ent->pde_unload_completion = NULL; 601 ent->pde_unload_completion = NULL;
@@ -692,7 +693,6 @@ void free_proc_entry(struct proc_dir_entry *de)
692 693
693/* 694/*
694 * Remove a /proc entry and free it if it's not currently in use. 695 * Remove a /proc entry and free it if it's not currently in use.
695 * If it is in use, we set the 'deleted' flag.
696 */ 696 */
697void remove_proc_entry(const char *name, struct proc_dir_entry *parent) 697void remove_proc_entry(const char *name, struct proc_dir_entry *parent)
698{ 698{
@@ -741,13 +741,8 @@ continue_removing:
741 parent->nlink--; 741 parent->nlink--;
742 de->nlink = 0; 742 de->nlink = 0;
743 WARN_ON(de->subdir); 743 WARN_ON(de->subdir);
744 if (!atomic_read(&de->count)) 744 if (atomic_dec_and_test(&de->count))
745 free_proc_entry(de); 745 free_proc_entry(de);
746 else {
747 de->deleted = 1;
748 printk("remove_proc_entry: %s/%s busy, count=%d\n",
749 parent->name, de->name, atomic_read(&de->count));
750 }
751 break; 746 break;
752 } 747 }
753 spin_unlock(&proc_subdir_lock); 748 spin_unlock(&proc_subdir_lock);
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index abe6a3f04368..1a551d92e1d8 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -43,13 +43,8 @@ void de_put(struct proc_dir_entry *de)
43 return; 43 return;
44 } 44 }
45 45
46 if (atomic_dec_and_test(&de->count)) { 46 if (atomic_dec_and_test(&de->count))
47 if (de->deleted) { 47 free_proc_entry(de);
48 printk("de_put: deferred delete of %s\n",
49 de->name);
50 free_proc_entry(de);
51 }
52 }
53 unlock_kernel(); 48 unlock_kernel();
54 } 49 }
55} 50}
diff --git a/fs/proc/root.c b/fs/proc/root.c
index ec9cb3b6c93b..81f99e691f99 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -207,6 +207,7 @@ struct proc_dir_entry proc_root = {
207 .name = "/proc", 207 .name = "/proc",
208 .mode = S_IFDIR | S_IRUGO | S_IXUGO, 208 .mode = S_IFDIR | S_IRUGO | S_IXUGO,
209 .nlink = 2, 209 .nlink = 2,
210 .count = ATOMIC_INIT(1),
210 .proc_iops = &proc_root_inode_operations, 211 .proc_iops = &proc_root_inode_operations,
211 .proc_fops = &proc_root_operations, 212 .proc_fops = &proc_root_operations,
212 .parent = &proc_root, 213 .parent = &proc_root,
diff --git a/fs/reiserfs/procfs.c b/fs/reiserfs/procfs.c
index 9aa7a06e093f..001144621672 100644
--- a/fs/reiserfs/procfs.c
+++ b/fs/reiserfs/procfs.c
@@ -420,12 +420,6 @@ static void *r_start(struct seq_file *m, loff_t * pos)
420 return NULL; 420 return NULL;
421 421
422 up_write(&s->s_umount); 422 up_write(&s->s_umount);
423
424 if (de->deleted) {
425 deactivate_super(s);
426 return NULL;
427 }
428
429 return s; 423 return s;
430} 424}
431 425