software node: Free resources explicitly when swnode_register() fails
authorAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Mon, 29 Mar 2021 15:12:02 +0000 (18:12 +0300)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Mon, 5 Apr 2021 11:18:21 +0000 (13:18 +0200)
Currently we have a slightly twisted logic in swnode_register().
It frees resources that it doesn't allocate on error path and
in once case it relies on the ->release() implementation.

Untwist the logic by freeing resources explicitly when swnode_register()
fails. Currently it happens only in fwnode_create_software_node().

Tested-by: Daniel Scally <djrscally@gmail.com>
Reviewed-by: Daniel Scally <djrscally@gmail.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20210329151207.36619-1-andriy.shevchenko@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/base/swnode.c

index 579ef8e..0acb908 100644 (file)
@@ -767,22 +767,19 @@ swnode_register(const struct software_node *node, struct swnode *parent,
        int ret;
 
        swnode = kzalloc(sizeof(*swnode), GFP_KERNEL);
-       if (!swnode) {
-               ret = -ENOMEM;
-               goto out_err;
-       }
+       if (!swnode)
+               return ERR_PTR(-ENOMEM);
 
        ret = ida_simple_get(parent ? &parent->child_ids : &swnode_root_ids,
                             0, 0, GFP_KERNEL);
        if (ret < 0) {
                kfree(swnode);
-               goto out_err;
+               return ERR_PTR(ret);
        }
 
        swnode->id = ret;
        swnode->node = node;
        swnode->parent = parent;
-       swnode->allocated = allocated;
        swnode->kobj.kset = swnode_kset;
        fwnode_init(&swnode->fwnode, &software_node_ops);
 
@@ -803,16 +800,17 @@ swnode_register(const struct software_node *node, struct swnode *parent,
                return ERR_PTR(ret);
        }
 
+       /*
+        * Assign the flag only in the successful case, so
+        * the above kobject_put() won't mess up with properties.
+        */
+       swnode->allocated = allocated;
+
        if (parent)
                list_add_tail(&swnode->entry, &parent->children);
 
        kobject_uevent(&swnode->kobj, KOBJ_ADD);
        return &swnode->fwnode;
-
-out_err:
-       if (allocated)
-               property_entries_free(node->properties);
-       return ERR_PTR(ret);
 }
 
 /**
@@ -971,6 +969,7 @@ struct fwnode_handle *
 fwnode_create_software_node(const struct property_entry *properties,
                            const struct fwnode_handle *parent)
 {
+       struct fwnode_handle *fwnode;
        struct software_node *node;
        struct swnode *p = NULL;
        int ret;
@@ -995,7 +994,13 @@ fwnode_create_software_node(const struct property_entry *properties,
 
        node->parent = p ? p->node : NULL;
 
-       return swnode_register(node, p, 1);
+       fwnode = swnode_register(node, p, 1);
+       if (IS_ERR(fwnode)) {
+               property_entries_free(node->properties);
+               kfree(node);
+       }
+
+       return fwnode;
 }
 EXPORT_SYMBOL_GPL(fwnode_create_software_node);