gpio: sysfs: fix chip removal with GPIOs exported over sysfs

Currently if we export a GPIO over sysfs and unbind the parent GPIO
controller, the exported attribute will remain under /sys/class/gpio
because once we remove the parent device, we can no longer associate the
descriptor with it in gpiod_unexport() and never drop the final
reference.

Rework the teardown code: provide an unlocked variant of
gpiod_unexport() and remove all exported GPIOs with the sysfs_lock taken
before unregistering the parent device itself. This is done to prevent
any new exports happening before we unregister the device completely.

Cc: stable@vger.kernel.org
Fixes: 1cd53df733c2 ("gpio: sysfs: don't look up exported lines as class devices")
Link: https://patch.msgid.link/20260212133505.81516-1-bartosz.golaszewski@oss.qualcomm.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index cd553ac..d4a46a0 100644 (file)
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -919,63 +919,68 @@ int gpiod_export_link(struct device *dev, const char *name,
 }
 EXPORT_SYMBOL_GPL(gpiod_export_link);
 
-/**
- * gpiod_unexport - reverse effect of gpiod_export()
- * @desc: GPIO to make unavailable
- *
- * This is implicit on gpiod_free().
- */
-void gpiod_unexport(struct gpio_desc *desc)
+static void gpiod_unexport_unlocked(struct gpio_desc *desc)
 {
        struct gpiod_data *tmp, *desc_data = NULL;
        struct gpiodev_data *gdev_data;
        struct gpio_device *gdev;
 
-       if (!desc) {
-               pr_warn("%s: invalid GPIO\n", __func__);
+       if (!test_bit(GPIOD_FLAG_EXPORT, &desc->flags))
                return;
-       }
 
-       scoped_guard(mutex, &sysfs_lock) {
-               if (!test_bit(GPIOD_FLAG_EXPORT, &desc->flags))
-                       return;
-
-               gdev = gpiod_to_gpio_device(desc);
-               gdev_data = gdev_get_data(gdev);
-               if (!gdev_data)
-                       return;
+       gdev = gpiod_to_gpio_device(desc);
+       gdev_data = gdev_get_data(gdev);
+       if (!gdev_data)
+               return;
 
-               list_for_each_entry(tmp, &gdev_data->exported_lines, list) {
-                       if (gpiod_is_equal(desc, tmp->desc)) {
-                               desc_data = tmp;
-                               break;
-                       }
+       list_for_each_entry(tmp, &gdev_data->exported_lines, list) {
+               if (gpiod_is_equal(desc, tmp->desc)) {
+                       desc_data = tmp;
+                       break;
                }
+       }
 
-               if (!desc_data)
-                       return;
+       if (!desc_data)
+               return;
 
-               list_del(&desc_data->list);
-               clear_bit(GPIOD_FLAG_EXPORT, &desc->flags);
+       list_del(&desc_data->list);
+       clear_bit(GPIOD_FLAG_EXPORT, &desc->flags);
 #if IS_ENABLED(CONFIG_GPIO_SYSFS_LEGACY)
-               sysfs_put(desc_data->value_kn);
-               device_unregister(desc_data->dev);
-
-               /*
-                * Release irq after deregistration to prevent race with
-                * edge_store.
-                */
-               if (desc_data->irq_flags)
-                       gpio_sysfs_free_irq(desc_data);
+       sysfs_put(desc_data->value_kn);
+       device_unregister(desc_data->dev);
+
+       /*
+        * Release irq after deregistration to prevent race with
+        * edge_store.
+        */
+       if (desc_data->irq_flags)
+               gpio_sysfs_free_irq(desc_data);
 #endif /* CONFIG_GPIO_SYSFS_LEGACY */
 
-               sysfs_remove_groups(desc_data->parent,
-                                   desc_data->chip_attr_groups);
-       }
+       sysfs_remove_groups(desc_data->parent,
+                           desc_data->chip_attr_groups);
 
        mutex_destroy(&desc_data->mutex);
        kfree(desc_data);
 }
+
+/**
+ * gpiod_unexport - reverse effect of gpiod_export()
+ * @desc: GPIO to make unavailable
+ *
+ * This is implicit on gpiod_free().
+ */
+void gpiod_unexport(struct gpio_desc *desc)
+{
+       if (!desc) {
+               pr_warn("%s: invalid GPIO\n", __func__);
+               return;
+       }
+
+       guard(mutex)(&sysfs_lock);
+
+       gpiod_unexport_unlocked(desc);
+}
 EXPORT_SYMBOL_GPL(gpiod_unexport);
 
 int gpiochip_sysfs_register(struct gpio_device *gdev)
@@ -1054,29 +1059,28 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev)
        struct gpio_desc *desc;
        struct gpio_chip *chip;
 
-       scoped_guard(mutex, &sysfs_lock) {
-               data = gdev_get_data(gdev);
-               if (!data)
-                       return;
+       guard(mutex)(&sysfs_lock);
 
-#if IS_ENABLED(CONFIG_GPIO_SYSFS_LEGACY)
-               device_unregister(data->cdev_base);
-#endif /* CONFIG_GPIO_SYSFS_LEGACY */
-               device_unregister(data->cdev_id);
-               kfree(data);
-       }
+       data = gdev_get_data(gdev);
+       if (!data)
+               return;
 
        guard(srcu)(&gdev->srcu);
-
        chip = srcu_dereference(gdev->chip, &gdev->srcu);
        if (!chip)
                return;
 
        /* unregister gpiod class devices owned by sysfs */
        for_each_gpio_desc_with_flag(chip, desc, GPIOD_FLAG_SYSFS) {
-               gpiod_unexport(desc);
+               gpiod_unexport_unlocked(desc);
                gpiod_free(desc);
        }
+
+#if IS_ENABLED(CONFIG_GPIO_SYSFS_LEGACY)
+       device_unregister(data->cdev_base);
+#endif /* CONFIG_GPIO_SYSFS_LEGACY */
+       device_unregister(data->cdev_id);
+       kfree(data);
 }
 
 /*