[PATCH] gpiolib: fix the error path of gpiochip_setup_dev()

[Joe Hattori <joe@xxxxxxxxxxxxxxxxxxxxx>]

gpiochip_setup_dev() calls gpio_device_put() on error when
gdev->dev.release is set, but this field is always empty as the cleanup
function moved to gpio_dev_type in commit aab5c6f20023 ("gpio: set
device type for GPIO chips"), resulting in a memory leak. Call
gpio_device_put() at the end of the gpiochip_setup_dev() to trigger the
resource cleanup, and remove the GPIO device from gpio_devices. Because
of this operation, gpiochip_setup_devs() may modify gpio_devices, so
acquire a mutex instead of a read lock.

This bug was found by an experimental verification tool that I am
developing. Tested the behavior with gpio-sim and confirmed basic
operations on GPIO devices worked as intended without any error logs.

Fixes: aab5c6f20023 ("gpio: set device type for GPIO chips")
Signed-off-by: Joe Hattori <joe@xxxxxxxxxxxxxxxxxxxxx>
---
 drivers/gpio/gpiolib.c | 42 ++++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 679ed764cb14..1f612a54a475 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -784,7 +784,7 @@ static const struct device_type gpio_dev_type = {
 #define gcdev_unregister(gdev)		device_del(&(gdev)->dev)
 #endif
 
-static int gpiochip_setup_dev(struct gpio_device *gdev)
+static int gpiochip_setup_dev(struct gpio_device *gdev, bool locked)
 {
 	struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
 	int ret;
@@ -800,7 +800,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
 
 	ret = gcdev_register(gdev, gpio_devt);
 	if (ret)
-		return ret;
+		goto err_put_device;
 
 	ret = gpiochip_sysfs_register(gdev);
 	if (ret)
@@ -813,6 +813,14 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
 
 err_remove_device:
 	gcdev_unregister(gdev);
+err_put_device:
+	if (locked)
+		list_del_rcu(&gdev->list);
+	else
+		scoped_guard(mutex, &gpio_devices_lock)
+			list_del_rcu(&gdev->list);
+	synchronize_srcu(&gpio_devices_srcu);
+	gpio_device_put(gdev);
 	return ret;
 }
 
@@ -850,17 +858,15 @@ static void machine_gpiochip_add(struct gpio_chip *gc)
 
 static void gpiochip_setup_devs(void)
 {
-	struct gpio_device *gdev;
+	struct gpio_device *gdev, *tmp;
 	int ret;
 
-	guard(srcu)(&gpio_devices_srcu);
-
-	list_for_each_entry_srcu(gdev, &gpio_devices, list,
-				 srcu_read_lock_held(&gpio_devices_srcu)) {
-		ret = gpiochip_setup_dev(gdev);
-		if (ret)
-			dev_err(&gdev->dev,
-				"Failed to initialize gpio device (%d)\n", ret);
+	scoped_guard(mutex, &gpio_devices_lock) {
+		list_for_each_entry_safe(gdev, tmp, &gpio_devices, list) {
+			ret = gpiochip_setup_dev(gdev, true);
+			if (ret)
+				pr_err("Failed to initialize gpio device (%d)\n", ret);
+		}
 	}
 }
 
@@ -921,6 +927,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 			       struct lock_class_key *request_key)
 {
 	struct gpio_device *gdev;
+	bool already_put = false;
 	unsigned int desc_index;
 	int base = 0;
 	int ret = 0;
@@ -1098,9 +1105,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	 * Otherwise, defer until later.
 	 */
 	if (gpiolib_initialized) {
-		ret = gpiochip_setup_dev(gdev);
-		if (ret)
+		ret = gpiochip_setup_dev(gdev, false);
+		if (ret) {
+			already_put = true;
 			goto err_remove_irqchip;
+		}
 	}
 	return 0;
 
@@ -1116,6 +1125,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	of_gpiochip_remove(gc);
 err_free_valid_mask:
 	gpiochip_free_valid_mask(gc);
+	if (already_put)
+		goto err_print_message;
 err_cleanup_desc_srcu:
 	cleanup_srcu_struct(&gdev->desc_srcu);
 err_cleanup_gdev_srcu:
@@ -1124,11 +1135,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	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_free_label:
 	kfree_const(gdev->label);
 err_free_descs:
-- 
2.34.1