Re: Two identical entries for "rtc" in /proc/devices

[David Brownell <david-b@pacbell.net>]

On Saturday 15 September 2007, Andrew Morton wrote:
> On Sat, 15 Sep 2007 11:50:21 -0700 David Brownell <david-b@pacbell.net> wrote:
> 
> > > On Thu, 06 Sep 2007 18:23:22 -0400 Chuck Ebbert <cebbert@redhat.com> wrote:
> > >
> > > > # ls -li
> > > > total 0
> > > > 4026532007 -r--r--r-- 1 root root 0 Sep  6 18:18 nvram
> > > > 4026532067 -r--r--r-- 1 root root 0 Sep  6 18:18 rtc
> > > > 4026532067 -r--r--r-- 1 root root 0 Sep  6 18:18 rtc
> > > > 4026532056 -rw-r--r-- 1 root root 0 Sep  6 18:18 snd-page-alloc
> > >
> > > ...
> > 
> > Semes pretty clear that this must be procfs itself...
> > when a filesystem sees a name in a directory, it should
> > refuse to make another file with the same name.  And it
> > should *never* reuse inode numbers...
>
> ...
>
> procfs can reject the attempt to create the file, but the bottom line
> is that two different callsites are trying to create the same file.  One
> of those callsites needs fixing?

Both of those call sites have code to handle procfs rejecting
the file creation; nothing to fix.  And anyway, there's no way
this is a *caller* bug!

The missing step seems to be that proc_register() doesn't bother
to check whether there's already an entry for that file.  Which
is what the appended *UNTESTED* patch does (it compiles though).

- Dave

--- g26.orig/fs/proc/generic.c	2007-09-18 22:08:44.000000000 -0700
+++ g26/fs/proc/generic.c	2007-09-18 22:14:07.000000000 -0700
@@ -521,10 +521,11 @@ static const struct inode_operations pro
 	.setattr	= proc_notify_change,
 };
 
-static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp)
+static int proc_register(struct proc_dir_entry *dir, struct proc_dir_entry *dp)
 {
 	unsigned int i;
-	
+	struct proc_dir_entry *de;
+
 	i = get_inode_number();
 	if (i == 0)
 		return -EAGAIN;
@@ -547,6 +548,16 @@ static int proc_register(struct proc_dir
 	}
 
 	spin_lock(&proc_subdir_lock);
+
+	for (de = dir->subdir; de ; de = de->next) {
+		if (de->namelen != dp->namelen)
+			continue;
+		if (!memcmp(de->name, dp->name, de->namelen)) {
+			spin_unlock(&proc_subdir_lock);
+			return -EEXIST;
+		}
+	}
+
 	dp->next = dir->subdir;
 	dp->parent = dir;
 	dir->subdir = dp;