[PATCH] drivers/base: error handling fixes

From: Jeff Garzik
Date: Wed Oct 04 2006 - 09:06:15 EST


drivers/base/class:
- class_device_rename(): function basically shat itself if anything
failed, leaving things in an indeterminant state. If kmalloc() ever
failed, it would dereference ERR_PTR(-ENOMEM). Fix a bunch of bugs,
over and above sysfs_create_link() error handling.

- class_device_add(): add missing sysfs_remove_link() [fix leak] to error path
- class_device_add(): handle sysfs_create_link() failure

drivers/base/dmapool:
- kmalloc() takes a GFP_xxx argument
- handle device_create_file() failure

drivers/base/platform:
- properly handle errors (fix leak-on-err) in platform_bus_init()

drivers/base/topology:
- return sysfs error via NOTIFY_BAD

Signed-off-by: Jeff Garzik <jeff@xxxxxxxxxx>

---

drivers/base/class.c | 56 ++++++++++++++++++++++++++++++++++++++++++------
drivers/base/dmapool.c | 8 +++++-
drivers/base/platform.c | 10 ++++++--
drivers/base/topology.c | 11 ++++-----
4 files changed, 69 insertions(+), 16 deletions(-)

diff --git a/drivers/base/class.c b/drivers/base/class.c
index b32b77f..673db14 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -562,7 +562,9 @@ int class_device_add(struct class_device
goto out2;

/* add the needed attributes to this device */
- sysfs_create_link(&class_dev->kobj, &parent_class->subsys.kset.kobj, "subsystem");
+ error = sysfs_create_link(&class_dev->kobj, &parent_class->subsys.kset.kobj, "subsystem");
+ if (error)
+ goto out2_5;
class_dev->uevent_attr.attr.name = "uevent";
class_dev->uevent_attr.attr.mode = S_IWUSR;
class_dev->uevent_attr.attr.owner = parent_class->owner;
@@ -639,6 +641,8 @@ int class_device_add(struct class_device
out4:
class_device_remove_file(class_dev, &class_dev->uevent_attr);
out3:
+ sysfs_remove_link(&class_dev->kobj, "subsystem");
+ out2_5:
kobject_del(&class_dev->kobj);
out2:
if(parent_class_dev)
@@ -791,36 +795,76 @@ void class_device_destroy(struct class *

int class_device_rename(struct class_device *class_dev, char *new_name)
{
- int error = 0;
+ int error = 0, err2;
char *old_class_name = NULL, *new_class_name = NULL;
+ char *old_name;

class_dev = class_device_get(class_dev);
if (!class_dev)
return -EINVAL;

- pr_debug("CLASS: renaming '%s' to '%s'\n", class_dev->class_id,
+ old_name = kstrdup(class_dev->class_id, GFP_KERNEL);
+ if (!old_name) {
+ error = -ENOMEM;
+ goto err_out;
+ }
+
+ pr_debug("CLASS: renaming '%s' to '%s'\n", old_name,
new_name);

- if (class_dev->dev)
+ if (class_dev->dev) {
old_class_name = make_class_name(class_dev->class->name,
&class_dev->kobj);
+ if (IS_ERR(old_class_name)) {
+ error = PTR_ERR(old_class_name);
+ goto err_out_oname;
+ }
+ }

strlcpy(class_dev->class_id, new_name, KOBJ_NAME_LEN);

error = kobject_rename(&class_dev->kobj, new_name);
+ if (error)
+ goto err_out_nameset;

if (class_dev->dev) {
new_class_name = make_class_name(class_dev->class->name,
&class_dev->kobj);
- sysfs_create_link(&class_dev->dev->kobj, &class_dev->kobj,
- new_class_name);
+ if (IS_ERR(new_class_name)) {
+ error = PTR_ERR(new_class_name);
+ goto err_out_rename;
+ }
+
+ error = sysfs_create_link(&class_dev->dev->kobj,
+ &class_dev->kobj, new_class_name);
+ if (error)
+ goto err_out_new_class;
+
sysfs_remove_link(&class_dev->dev->kobj, old_class_name);
}
+
class_device_put(class_dev);

+ kfree(old_name);
kfree(old_class_name);
kfree(new_class_name);

+ return 0;
+
+err_out_new_class:
+ kfree(new_class_name);
+err_out_rename:
+ err2 = kobject_rename(&class_dev->kobj, old_name);
+ if (err2)
+ printk(KERN_ERR "Error %d while undoing kobject_rename('%s') failure... ruh roh, raggy\n",
+ err2, new_name);
+err_out_nameset:
+ strlcpy(class_dev->class_id, old_name, KOBJ_NAME_LEN);
+ kfree(old_class_name);
+err_out_oname:
+ kfree(old_name);
+err_out:
+ class_device_put(class_dev);
return error;
}

diff --git a/drivers/base/dmapool.c b/drivers/base/dmapool.c
index 33c5cce..b418b6b 100644
--- a/drivers/base/dmapool.c
+++ b/drivers/base/dmapool.c
@@ -126,7 +126,7 @@ dma_pool_create (const char *name, struc
} else if (allocation < size)
return NULL;

- if (!(retval = kmalloc (sizeof *retval, SLAB_KERNEL)))
+ if (!(retval = kmalloc (sizeof *retval, GFP_KERNEL)))
return retval;

strlcpy (retval->name, name, sizeof retval->name);
@@ -143,7 +143,11 @@ dma_pool_create (const char *name, struc
if (dev) {
down (&pools_lock);
if (list_empty (&dev->dma_pools))
- device_create_file (dev, &dev_attr_pools);
+ if (device_create_file (dev, &dev_attr_pools)) {
+ up (&pools_lock);
+ kfree(retval);
+ return NULL;
+ }
/* note: not currently insisting "name" be unique */
list_add (&retval->pools, &dev->dma_pools);
up (&pools_lock);
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 940ce41..3b6b758 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -563,8 +563,14 @@ EXPORT_SYMBOL_GPL(platform_bus_type);

int __init platform_bus_init(void)
{
- device_register(&platform_bus);
- return bus_register(&platform_bus_type);
+ int err = device_register(&platform_bus);
+ if (err)
+ return err;
+
+ err = bus_register(&platform_bus_type);
+ if (err)
+ device_unregister(&platform_bus);
+ return err;
}

#ifndef ARCH_HAS_DMA_GET_REQUIRED_MASK
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index 3ef9d51..2d0c965 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -97,14 +97,12 @@ static struct attribute_group topology_a
/* Add/Remove cpu_topology interface for CPU device */
static int __cpuinit topology_add_dev(struct sys_device * sys_dev)
{
- sysfs_create_group(&sys_dev->kobj, &topology_attr_group);
- return 0;
+ return sysfs_create_group(&sys_dev->kobj, &topology_attr_group);
}

-static int __cpuinit topology_remove_dev(struct sys_device * sys_dev)
+static void __cpuinit topology_remove_dev(struct sys_device * sys_dev)
{
sysfs_remove_group(&sys_dev->kobj, &topology_attr_group);
- return 0;
}

static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
@@ -112,17 +110,18 @@ static int __cpuinit topology_cpu_callba
{
unsigned int cpu = (unsigned long)hcpu;
struct sys_device *sys_dev;
+ int rc = 0;

sys_dev = get_cpu_sysdev(cpu);
switch (action) {
case CPU_ONLINE:
- topology_add_dev(sys_dev);
+ rc = topology_add_dev(sys_dev);
break;
case CPU_DEAD:
topology_remove_dev(sys_dev);
break;
}
- return NOTIFY_OK;
+ return rc ? NOTIFY_BAD : NOTIFY_OK;
}

static struct notifier_block __cpuinitdata topology_cpu_notifier =
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/