Commit a9007553 authored by Andrew Morton's avatar Andrew Morton Committed by Dave Jones

[PATCH] register_chrdev_region() leak and race fix

- 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.
parent 49263e20
...@@ -115,7 +115,15 @@ get_chrfops(unsigned int major, unsigned int minor) ...@@ -115,7 +115,15 @@ get_chrfops(unsigned int major, unsigned int minor)
} }
/* /*
* 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 register_chrdev_region(unsigned int major, unsigned int baseminor,
int minorct, const char *name, int minorct, const char *name,
...@@ -125,23 +133,27 @@ int register_chrdev_region(unsigned int major, unsigned int baseminor, ...@@ -125,23 +133,27 @@ int register_chrdev_region(unsigned int major, unsigned int baseminor,
int ret = 0; int ret = 0;
int i; int i;
cd = kmalloc(sizeof(struct char_device_struct), GFP_KERNEL);
if (cd == NULL)
return -ENOMEM;
write_lock_irq(&chrdevs_lock);
/* temporary */ /* temporary */
if (major == 0) { 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) if (chrdevs[i] == NULL)
break; break;
read_unlock(&chrdevs_lock); }
if (i == 0) if (i == 0) {
return -EBUSY; ret = -EBUSY;
ret = major = i; goto out;
}
major = i;
ret = major;
} }
cd = kmalloc(sizeof(struct char_device_struct), GFP_KERNEL);
if (cd == NULL)
return -ENOMEM;
cd->major = major; cd->major = major;
cd->baseminor = baseminor; cd->baseminor = baseminor;
cd->minorct = minorct; cd->minorct = minorct;
...@@ -150,7 +162,6 @@ int register_chrdev_region(unsigned int major, unsigned int baseminor, ...@@ -150,7 +162,6 @@ int register_chrdev_region(unsigned int major, unsigned int baseminor,
i = major_to_index(major); i = major_to_index(major);
write_lock_irq(&chrdevs_lock);
for (cp = &chrdevs[i]; *cp; cp = &(*cp)->next) for (cp = &chrdevs[i]; *cp; cp = &(*cp)->next)
if ((*cp)->major > major || if ((*cp)->major > major ||
((*cp)->major == major && (*cp)->baseminor >= baseminor)) ((*cp)->major == major && (*cp)->baseminor >= baseminor))
...@@ -162,8 +173,10 @@ int register_chrdev_region(unsigned int major, unsigned int baseminor, ...@@ -162,8 +173,10 @@ int register_chrdev_region(unsigned int major, unsigned int baseminor,
cd->next = *cp; cd->next = *cp;
*cp = cd; *cp = cd;
} }
out:
write_unlock_irq(&chrdevs_lock); write_unlock_irq(&chrdevs_lock);
if (ret < 0)
kfree(cd);
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment