[PATCH] gpio: sysfs: fix chip removal with GPIOs exported over sysfs

[PATCH] gpio: sysfs: fix chip removal with GPIOs exported over sysfs

[Bartosz Golaszewski]

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
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/gpio/gpiolib-sysfs.c | 106 ++++++++++++++++++-----------------
 1 file changed, 55 insertions(+), 51 deletions(-)

diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index cd553acf3055e..d4a46a0a37d8f 100644
--- 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);
 }
 
 /*
-- 
2.47.3

Re: [PATCH] gpio: sysfs: fix chip removal with GPIOs exported over sysfs

[Johan Hovold]

On Thu, Feb 12, 2026 at 02:35:05PM +0100, Bartosz Golaszewski wrote:
> 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.

Is this a recent regression? I'm quite sure fixed this once back in
2015.

> 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.

So please add a Fixes tag here to document this.

> Cc: stable@vger.kernel.org
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Johan

Re: [PATCH] gpio: sysfs: fix chip removal with GPIOs exported over sysfs

[Bartosz Golaszewski]

On Fri, 13 Feb 2026 09:32:47 +0100, Johan Hovold <johan@kernel.org> said:
> On Thu, Feb 12, 2026 at 02:35:05PM +0100, Bartosz Golaszewski wrote:
>> 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.
>
> Is this a recent regression? I'm quite sure fixed this once back in
> 2015.
>

Yes, that sometimes happens. There have been close to 100 commits between your
fix and now, including two major reworks.

>> 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.
>
> So please add a Fixes tag here to document this.
>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
>
> Johan
>

It bisects to this commit, though it's possible it's a mix of several changes.

Fixes: 1cd53df733c2 ("gpio: sysfs: don't look up exported lines as
class devices")

Bartosz

Re: [PATCH] gpio: sysfs: fix chip removal with GPIOs exported over sysfs

[Johan Hovold]

On Fri, Feb 13, 2026 at 10:11:47AM -0800, Bartosz Golaszewski wrote:
> On Fri, 13 Feb 2026 09:32:47 +0100, Johan Hovold <johan@kernel.org> said:
> > On Thu, Feb 12, 2026 at 02:35:05PM +0100, Bartosz Golaszewski wrote:
> >> 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.
> >
> > Is this a recent regression? I'm quite sure fixed this once back in
> > 2015.
> 
> Yes, that sometimes happens. There have been close to 100 commits between your
> fix and now, including two major reworks.

It sure does. I was just pointing out that this is a regression, which
wasn't apparent from your commit message.

> >> 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.
> >
> > So please add a Fixes tag here to document this.
> >
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

> It bisects to this commit, though it's possible it's a mix of several changes.
> 
> Fixes: 1cd53df733c2 ("gpio: sysfs: don't look up exported lines as
> class devices")

That's the commit from last summer that I suspected as well after a
quick glance at the code (hence the word "recent" above).

We don't want anyone trying to backport this further back than needed.

Johan

Re: [PATCH] gpio: sysfs: fix chip removal with GPIOs exported over sysfs

[Bartosz Golaszewski]

On Thu, 12 Feb 2026 14:35:05 +0100, Bartosz Golaszewski wrote:
> 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.
> 
> [...]

Applied, thanks!

[1/1] gpio: sysfs: fix chip removal with GPIOs exported over sysfs
      https://git.kernel.org/brgl/c/6766f59012301f1bf3f46c6e7149caca45d92309

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>