Messages in this thread Patch in this message | | | Date | Fri, 21 Nov 2003 16:33:54 -0600 | From | Nathan Lynch <> | Subject | [PATCH] better locking in fs/proc/generic.c (bug 1552) |
| |
Hi-
I have been experiencing various oopses in the procfs code when rapidly adding and removing /proc entries. It seems that the main problem is lack of proper locking around code that traverses or modifies the proc_dir_entry tree. There is also the matter of proc_kill_inodes() clearing the file pointer's f_op, which caused oopses in vfs_read() in my testing.
I have followed the example of proc_lookup() and used lock_kernel/unlock_kernel around critical sections. I'm not much of a filesystem person, so I would appreciate a review of the patch from folks more knowledgeable. I have been testing the code on a couple of dual processor machines (i386 and ppc64) without incident.
Please see http://bugme.osdl.org/show_bug.cgi?id=1552 for oops traces and a testcase. Andrew posted a one-liner patch there which was included in mm4 which is also needed.
Patch is against 2.6.0-test9-mm4.
Thanks, Nathan diff -Naurp linux-2.6.0-test9-mm4.orig/fs/proc/generic.c linux-2.6.0-test9-mm4.new/fs/proc/generic.c --- linux-2.6.0-test9-mm4.orig/fs/proc/generic.c 2003-10-25 13:44:56.000000000 -0500 +++ linux-2.6.0-test9-mm4.new/fs/proc/generic.c 2003-11-21 14:47:00.000000000 -0600 @@ -260,6 +260,7 @@ static int xlate_proc_name(const char *n int len; de = &proc_root; + lock_kernel(); while (1) { next = strchr(cp, '/'); if (!next) @@ -270,10 +271,13 @@ static int xlate_proc_name(const char *n if (proc_match(len, cp, de)) break; } - if (!de) + if (!de) { + unlock_kernel(); return -ENOENT; + } cp += len + 1; } + unlock_kernel(); *residual = cp; *ret = de; return 0; @@ -462,6 +466,7 @@ static int proc_register(struct proc_dir if (i < 0) return -EAGAIN; dp->low_ino = i; + lock_kernel(); /* needed to protect against concurrent registration */ dp->next = dir->subdir; dp->parent = dir; dir->subdir = dp; @@ -480,6 +485,7 @@ static int proc_register(struct proc_dir if (dp->proc_iops == NULL) dp->proc_iops = &proc_file_inode_operations; } + unlock_kernel(); return 0; } @@ -507,7 +513,6 @@ static void proc_kill_inodes(struct proc if (PDE(inode) != de) continue; fops = filp->f_op; - filp->f_op = NULL; fops_put(fops); } file_list_unlock(); @@ -643,6 +648,7 @@ void remove_proc_entry(const char *name, if (!parent && xlate_proc_name(name, &parent, &fn) != 0) goto out; len = strlen(fn); + lock_kernel(); for (p = &parent->subdir; *p; p=&(*p)->next ) { if (!proc_match(len, fn, *p)) continue; @@ -664,6 +670,7 @@ void remove_proc_entry(const char *name, } break; } + unlock_kernel(); out: return; } | |