- If two CPUs run register_chrdev_region(major == 0) at the same time they
  can get the same major.

  Fix that by extending the lock coverage.

- local variable `cd' was leaky on an error path.

- Add some API commentary.


 fs/char_dev.c |   39 ++++++++++++++++++++++++++-------------
 1 files changed, 26 insertions(+), 13 deletions(-)

diff -puN fs/char_dev.c~register_chrdev_region-leak-fix fs/char_dev.c
--- 25/fs/char_dev.c~register_chrdev_region-leak-fix	2003-03-23 00:51:35.000000000 -0800
+++ 25-akpm/fs/char_dev.c	2003-03-23 01:02:14.000000000 -0800
@@ -115,7 +115,15 @@ get_chrfops(unsigned int major, unsigned
 }
 
 /*
- * Register a single major with a specified minor range
+ * Register a single major with a specified minor range.
+ *
+ * If major == 0 this functions will dynamically allocate a major and return
+ * its number.
+ *
+ * If major > 0 this function will attempt to reserve the passed range of
+ * minors and will return zero on success.
+ *
+ * Returns a -ve errno on failure.
  */
 int register_chrdev_region(unsigned int major, unsigned int baseminor,
 			   int minorct, const char *name,
@@ -125,23 +133,27 @@ int register_chrdev_region(unsigned int 
 	int ret = 0;
 	int i;
 
+	cd = kmalloc(sizeof(struct char_device_struct), GFP_KERNEL);
+	if (cd == NULL)
+		return -ENOMEM;
+
+	write_lock_irq(&chrdevs_lock);
+
 	/* temporary */
 	if (major == 0) {
-		read_lock(&chrdevs_lock);
-		for (i = ARRAY_SIZE(chrdevs)-1; i > 0; i--)
+		for (i = ARRAY_SIZE(chrdevs)-1; i > 0; i--) {
 			if (chrdevs[i] == NULL)
 				break;
-		read_unlock(&chrdevs_lock);
+		}
 
-		if (i == 0)
-			return -EBUSY;
-		ret = major = i;
+		if (i == 0) {
+			ret = -EBUSY;
+			goto out;
+		}
+		major = i;
+		ret = major;
 	}
 
-	cd = kmalloc(sizeof(struct char_device_struct), GFP_KERNEL);
-	if (cd == NULL)
-		return -ENOMEM;
-
 	cd->major = major;
 	cd->baseminor = baseminor;
 	cd->minorct = minorct;
@@ -150,7 +162,6 @@ int register_chrdev_region(unsigned int 
 
 	i = major_to_index(major);
 
-	write_lock_irq(&chrdevs_lock);
 	for (cp = &chrdevs[i]; *cp; cp = &(*cp)->next)
 		if ((*cp)->major > major ||
 		    ((*cp)->major == major && (*cp)->baseminor >= baseminor))
@@ -162,8 +173,10 @@ int register_chrdev_region(unsigned int 
 		cd->next = *cp;
 		*cp = cd;
 	}
+out:
 	write_unlock_irq(&chrdevs_lock);
-
+	if (ret < 0)
+		kfree(cd);
 	return ret;
 }
 

_