Steven Rostedt <[email protected]> wrote:
>
> Working on a custom kernel that adds and removes proc entries quite a
> bit, I discovered that remove_proc_entry is not protected against
> multiple threads removing entries belonging to the same parent. At
> first I thought that this is only a problem with my changes, but after
> inspecting the vanilla kernel, I see that there's several places that
> calls remove_proc_entry with the same parent (most noticeably
> /proc/drivers).
>
> I've added a global remove_proc_lock to protect this section of code. I
> was going to add a lock to proc_dir_entry so that the locking is only
> cut down to the same parent, but since this function is called so
> infrequently, why waste more memory then is needed. One global lock
> should not cause too much of a headache here.
>
> I'm not sure if remove_proc_entry is called from interrupt context, so I
> did a irqsave just in case.
>
> -- Steve
>
>
> Index: linux-2.6.15-rc7/fs/proc/generic.c
> ===================================================================
> --- linux-2.6.15-rc7.orig/fs/proc/generic.c 2005-12-30 14:19:39.000000000 -0500
> +++ linux-2.6.15-rc7/fs/proc/generic.c 2005-12-30 16:18:42.000000000 -0500
> @@ -19,6 +19,7 @@
> #include <linux/idr.h>
> #include <linux/namei.h>
> #include <linux/bitops.h>
> +#include <linux/spinlock.h>
> #include <asm/uaccess.h>
>
> static ssize_t proc_file_read(struct file *file, char __user *buf,
> @@ -27,6 +28,8 @@
> size_t count, loff_t *ppos);
> static loff_t proc_file_lseek(struct file *, loff_t, int);
>
> +static DEFINE_SPINLOCK(remove_proc_lock);
> +
> int proc_match(int len, const char *name, struct proc_dir_entry *de)
> {
> if (de->namelen != len)
> @@ -689,10 +692,13 @@
> struct proc_dir_entry *de;
> const char *fn = name;
> int len;
> + unsigned long flags;
>
> if (!parent && xlate_proc_name(name, &parent, &fn) != 0)
> goto out;
> len = strlen(fn);
> +
> + spin_lock_irqsave(&remove_proc_lock, flags);
> for (p = &parent->subdir; *p; p=&(*p)->next ) {
> if (!proc_match(len, fn, *p))
> continue;
> @@ -713,6 +719,7 @@
> }
> break;
> }
> + spin_unlock_irqrestore(&remove_proc_lock, flags);
> out:
> return;
> }
Aren't there other places where we need to take this lock? Code which
traverses that list, code which adds things to it?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
