gpio: Fix resource leaks on errors in gpiochip_add_data_with_key()

Since commit aab5c6f20023 ("gpio: set device type for GPIO chips"),
`gdev->dev.release` is unset.  As a result, the reference count to
`gdev->dev` isn't dropped on the error handling paths.

Drop the reference on errors.

Also reorder the instructions to make the error handling simpler.
Now gpiochip_add_data_with_key() roughly looks like:

   >>> Some memory allocation.  Go to ERR ZONE 1 on errors.
   >>> device_initialize().

   gpiodev_release() takes over the responsibility for freeing the
   resources of `gdev->dev`.  The subsequent error handling paths
   shouldn't go through ERR ZONE 1 again which leads to double free.

   >>> Some initialization mainly on `gdev`.
   >>> The rest of initialization.  Go to ERR ZONE 2 on errors.
   >>> Chip registration success and exit.

   >>> ERR ZONE 2.  gpio_device_put() and exit.
   >>> ERR ZONE 1.

Cc: stable@vger.kernel.org
Fixes: aab5c6f20023 ("gpio: set device type for GPIO chips")
Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
Link: https://patch.msgid.link/20260205092840.2574840-1-tzungbi@kernel.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 86a171e96b0ebc9dd77acce8de11266dd870002d..f77d5121a8a87de9844cf9f324a352eb4cb43a50 100644 (file)
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -892,13 +892,15 @@ static const struct device_type gpio_dev_type = {
 #define gcdev_unregister(gdev)         device_del(&(gdev)->dev)
 #endif
 
+/*
+ * An initial reference count has been held in gpiochip_add_data_with_key().
+ * The caller should drop the reference via gpio_device_put() on errors.
+ */
 static int gpiochip_setup_dev(struct gpio_device *gdev)
 {
        struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
        int ret;
 
-       device_initialize(&gdev->dev);
-
        /*
         * If fwnode doesn't belong to another device, it's safe to clear its
         * initialized flag.
@@ -964,9 +966,11 @@ static void gpiochip_setup_devs(void)
        list_for_each_entry_srcu(gdev, &gpio_devices, list,
                                 srcu_read_lock_held(&gpio_devices_srcu)) {
                ret = gpiochip_setup_dev(gdev);
-               if (ret)
+               if (ret) {
+                       gpio_device_put(gdev);
                        dev_err(&gdev->dev,
                                "Failed to initialize gpio device (%d)\n", ret);
+               }
        }
 }
 
@@ -1047,71 +1051,72 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
        int base = 0;
        int ret;
 
-       /*
-        * First: allocate and populate the internal stat container, and
-        * set up the struct device.
-        */
        gdev = kzalloc(sizeof(*gdev), GFP_KERNEL);
        if (!gdev)
                return -ENOMEM;
-
-       gdev->dev.type = &gpio_dev_type;
-       gdev->dev.bus = &gpio_bus_type;
-       gdev->dev.parent = gc->parent;
-       rcu_assign_pointer(gdev->chip, gc);
-
        gc->gpiodev = gdev;
        gpiochip_set_data(gc, data);
 
-       device_set_node(&gdev->dev, gpiochip_choose_fwnode(gc));
-
        ret = ida_alloc(&gpio_ida, GFP_KERNEL);
        if (ret < 0)
                goto err_free_gdev;
        gdev->id = ret;
 
-       ret = dev_set_name(&gdev->dev, GPIOCHIP_NAME "%d", gdev->id);
+       ret = init_srcu_struct(&gdev->srcu);
        if (ret)
                goto err_free_ida;
+       rcu_assign_pointer(gdev->chip, gc);
 
-       if (gc->parent && gc->parent->driver)
-               gdev->owner = gc->parent->driver->owner;
-       else if (gc->owner)
-               /* TODO: remove chip->owner */
-               gdev->owner = gc->owner;
-       else
-               gdev->owner = THIS_MODULE;
+       ret = init_srcu_struct(&gdev->desc_srcu);
+       if (ret)
+               goto err_cleanup_gdev_srcu;
+
+       ret = dev_set_name(&gdev->dev, GPIOCHIP_NAME "%d", gdev->id);
+       if (ret)
+               goto err_cleanup_desc_srcu;
+
+       device_initialize(&gdev->dev);
+       /*
+        * After this point any allocated resources to `gdev` will be
+        * free():ed by gpiodev_release().  If you add new resources
+        * then make sure they get free():ed there.
+        */
+       gdev->dev.type = &gpio_dev_type;
+       gdev->dev.bus = &gpio_bus_type;
+       gdev->dev.parent = gc->parent;
+       device_set_node(&gdev->dev, gpiochip_choose_fwnode(gc));
 
        ret = gpiochip_get_ngpios(gc, &gdev->dev);
        if (ret)
-               goto err_free_dev_name;
+               goto err_put_device;
+       gdev->ngpio = gc->ngpio;
 
        gdev->descs = kcalloc(gc->ngpio, sizeof(*gdev->descs), GFP_KERNEL);
        if (!gdev->descs) {
                ret = -ENOMEM;
-               goto err_free_dev_name;
+               goto err_put_device;
        }
 
        gdev->label = kstrdup_const(gc->label ?: "unknown", GFP_KERNEL);
        if (!gdev->label) {
                ret = -ENOMEM;
-               goto err_free_descs;
+               goto err_put_device;
        }
 
-       gdev->ngpio = gc->ngpio;
        gdev->can_sleep = gc->can_sleep;
-
        rwlock_init(&gdev->line_state_lock);
        RAW_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
        BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);
-
-       ret = init_srcu_struct(&gdev->srcu);
-       if (ret)
-               goto err_free_label;
-
-       ret = init_srcu_struct(&gdev->desc_srcu);
-       if (ret)
-               goto err_cleanup_gdev_srcu;
+#ifdef CONFIG_PINCTRL
+       INIT_LIST_HEAD(&gdev->pin_ranges);
+#endif
+       if (gc->parent && gc->parent->driver)
+               gdev->owner = gc->parent->driver->owner;
+       else if (gc->owner)
+               /* TODO: remove chip->owner */
+               gdev->owner = gc->owner;
+       else
+               gdev->owner = THIS_MODULE;
 
        scoped_guard(mutex, &gpio_devices_lock) {
                /*
@@ -1127,7 +1132,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
                        if (base < 0) {
                                ret = base;
                                base = 0;
-                               goto err_cleanup_desc_srcu;
+                               goto err_put_device;
                        }
 
                        /*
@@ -1147,14 +1152,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
                ret = gpiodev_add_to_list_unlocked(gdev);
                if (ret) {
                        gpiochip_err(gc, "GPIO integer space overlap, cannot add chip\n");
-                       goto err_cleanup_desc_srcu;
+                       goto err_put_device;
                }
        }
 
-#ifdef CONFIG_PINCTRL
-       INIT_LIST_HEAD(&gdev->pin_ranges);
-#endif
-
        if (gc->names)
                gpiochip_set_desc_names(gc);
 
@@ -1248,25 +1249,19 @@ err_remove_from_list:
        scoped_guard(mutex, &gpio_devices_lock)
                list_del_rcu(&gdev->list);
        synchronize_srcu(&gpio_devices_srcu);
-       if (gdev->dev.release) {
-               /* release() has been registered by gpiochip_setup_dev() */
-               gpio_device_put(gdev);
-               goto err_print_message;
-       }
+err_put_device:
+       gpio_device_put(gdev);
+       goto err_print_message;
+
 err_cleanup_desc_srcu:
        cleanup_srcu_struct(&gdev->desc_srcu);
 err_cleanup_gdev_srcu:
        cleanup_srcu_struct(&gdev->srcu);
-err_free_label:
-       kfree_const(gdev->label);
-err_free_descs:
-       kfree(gdev->descs);
-err_free_dev_name:
-       kfree(dev_name(&gdev->dev));
 err_free_ida:
        ida_free(&gpio_ida, gdev->id);
 err_free_gdev:
        kfree(gdev);
+
 err_print_message:
        /* failures here can mean systems won't boot... */
        if (ret != -EPROBE_DEFER) {