[PATCH 0/4] gpio: shared: fix some bugs in unregister and error paths

[PATCH 0/4] gpio: shared: fix some bugs in unregister and error paths

[Bartosz Golaszewski]

Here are a couple more fixes for shared GPIOs that I want to send before
the end of the merge window.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Bartosz Golaszewski (4):
      gpio: shared: fix NULL-pointer dereference in teardown path
      gpio: shared: check if a reference is populated before cleaning its resources
      gpio: shared: fix auxiliary device cleanup order
      gpio: shared: make locking more fine-grained

 drivers/gpio/gpiolib-shared.c | 50 ++++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 20 deletions(-)
---
base-commit: 057c46412cd90e432a73f650cdd75b56f755a082
change-id: 20251205-gpio-shared-teardown-fixes-d1e2429333c7

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

[PATCH 1/4] gpio: shared: fix NULL-pointer dereference in teardown path

[Bartosz Golaszewski]

We need to actually store the address of the GPIO lookup table in the
reference struct before we try to free it or - worse - dereference its
members.

Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/gpio/gpiolib-shared.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 5d15675d61ea52b21a8181ef9a93123774b8e7c3..17da15c1075f97f4ebed969b38197ac155141406 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -399,7 +399,8 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, unsigned long lflags)
 			lookup->table[0] = GPIO_LOOKUP(no_free_ptr(key), 0,
 						       ref->con_id, lflags);
 
-			gpiod_add_lookup_table(no_free_ptr(lookup));
+			ref->lookup = no_free_ptr(lookup);
+			gpiod_add_lookup_table(ref->lookup);
 
 			return 0;
 		}

-- 
2.51.0

[PATCH 2/4] gpio: shared: check if a reference is populated before cleaning its resources

[Bartosz Golaszewski]

It's possible that not all proxy entries will be set up when the device
gets removed so check if they are before trying to dereference members
which are still NULL. This can happen if some consumers never requested
their shared GPIOs.

Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/gpio/gpiolib-shared.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 17da15c1075f97f4ebed969b38197ac155141406..4084a28a953a7c9f4b04cbf867b05a1a74b557ca 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -491,10 +491,13 @@ void gpio_device_teardown_shared(struct gpio_device *gdev)
 			continue;
 
 		list_for_each_entry(ref, &entry->refs, list) {
-			gpiod_remove_lookup_table(ref->lookup);
-			kfree(ref->lookup->table[0].key);
-			kfree(ref->lookup);
-			ref->lookup = NULL;
+			if (ref->lookup) {
+				gpiod_remove_lookup_table(ref->lookup);
+				kfree(ref->lookup->table[0].key);
+				kfree(ref->lookup);
+				ref->lookup = NULL;
+			}
+
 			gpio_shared_remove_adev(&ref->adev);
 		}
 	}

-- 
2.51.0

[PATCH 3/4] gpio: shared: fix auxiliary device cleanup order

[Bartosz Golaszewski]

Dropping the last reference to the internal struct device should be the
last thing we do so delete the device first and then uninit it which
also involves the final put_device().

Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/gpio/gpiolib-shared.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 4084a28a953a7c9f4b04cbf867b05a1a74b557ca..2d3b0c3460e56941af8415af6989914104060bf7 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -415,8 +415,8 @@ static void gpio_shared_remove_adev(struct auxiliary_device *adev)
 {
 	lockdep_assert_held(&gpio_shared_lock);
 
-	auxiliary_device_uninit(adev);
 	auxiliary_device_delete(adev);
+	auxiliary_device_uninit(adev);
 }
 
 int gpio_device_setup_shared(struct gpio_device *gdev)

-- 
2.51.0

[PATCH 4/4] gpio: shared: make locking more fine-grained

[Bartosz Golaszewski]

The global gpio_shared_lock has caused some issues when recursively
removing GPIO shared proxies. The thing is: we don't really need it.
Once created from an init-call, the shared GPIO data structures are
never removed, there's no need to protect the linked lists of entries
and references. All we need to protect is the shared GPIO descriptor
(which we already do with a per-entry mutex) and the auxiliary device
data in struct gpio_shared_ref.

Remove the global gpio_shared_lock and use a per-reference mutex to
protect shared references when adding/removing the auxiliary devices and
their GPIO lookup entries.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/gpio/gpiolib-shared.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 2d3b0c3460e56941af8415af6989914104060bf7..ba4b718d40a087608fc06f59d242932df1d117e9 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -36,6 +36,8 @@ struct gpio_shared_ref {
 	enum gpiod_flags flags;
 	char *con_id;
 	int dev_id;
+	/* Protects the auxiliary device struct and the lookup table. */
+	struct mutex lock;
 	struct auxiliary_device adev;
 	struct gpiod_lookup_table *lookup;
 };
@@ -49,6 +51,7 @@ struct gpio_shared_entry {
 	unsigned int offset;
 	/* Index in the property value array. */
 	size_t index;
+	/* Synchronizes the modification of shared_desc. */
 	struct mutex lock;
 	struct gpio_shared_desc *shared_desc;
 	struct kref ref;
@@ -56,7 +59,6 @@ struct gpio_shared_entry {
 };
 
 static LIST_HEAD(gpio_shared_list);
-static DEFINE_MUTEX(gpio_shared_lock);
 static DEFINE_IDA(gpio_shared_ida);
 
 #if IS_ENABLED(CONFIG_OF)
@@ -187,6 +189,7 @@ static int gpio_shared_of_traverse(struct device_node *curr)
 
 			ref->fwnode = fwnode_handle_get(of_fwnode_handle(curr));
 			ref->flags = args.args[1];
+			mutex_init(&ref->lock);
 
 			if (strends(prop->name, "gpios"))
 				suffix = "-gpios";
@@ -258,7 +261,7 @@ static int gpio_shared_make_adev(struct gpio_device *gdev,
 	struct auxiliary_device *adev = &ref->adev;
 	int ret;
 
-	lockdep_assert_held(&gpio_shared_lock);
+	guard(mutex)(&ref->lock);
 
 	memset(adev, 0, sizeof(*adev));
 
@@ -373,14 +376,14 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, unsigned long lflags)
 	if (!lookup)
 		return -ENOMEM;
 
-	guard(mutex)(&gpio_shared_lock);
-
 	list_for_each_entry(entry, &gpio_shared_list, list) {
 		list_for_each_entry(ref, &entry->refs, list) {
 			if (!device_match_fwnode(consumer, ref->fwnode) &&
 			    !gpio_shared_dev_is_reset_gpio(consumer, entry, ref))
 				continue;
 
+			guard(mutex)(&ref->lock);
+
 			/* We've already done that on a previous request. */
 			if (ref->lookup)
 				return 0;
@@ -413,8 +416,6 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, unsigned long lflags)
 
 static void gpio_shared_remove_adev(struct auxiliary_device *adev)
 {
-	lockdep_assert_held(&gpio_shared_lock);
-
 	auxiliary_device_delete(adev);
 	auxiliary_device_uninit(adev);
 }
@@ -426,8 +427,6 @@ int gpio_device_setup_shared(struct gpio_device *gdev)
 	unsigned long *flags;
 	int ret;
 
-	guard(mutex)(&gpio_shared_lock);
-
 	list_for_each_entry(entry, &gpio_shared_list, list) {
 		list_for_each_entry(ref, &entry->refs, list) {
 			if (gdev->dev.parent == &ref->adev.dev) {
@@ -484,13 +483,22 @@ void gpio_device_teardown_shared(struct gpio_device *gdev)
 	struct gpio_shared_entry *entry;
 	struct gpio_shared_ref *ref;
 
-	guard(mutex)(&gpio_shared_lock);
-
 	list_for_each_entry(entry, &gpio_shared_list, list) {
 		if (!device_match_fwnode(&gdev->dev, entry->fwnode))
 			continue;
 
+		/*
+		 * For some reason if we call synchronize_srcu() in GPIO core,
+		 * descent here and take this mutex and then recursively call
+		 * synchronize_srcu() again from gpiochip_remove() (which is
+		 * totally fine) called after gpio_shared_remove_adev(),
+		 * lockdep prints a false positive deadlock splat. Disable
+		 * lockdep here.
+		 */
+		lockdep_off();
 		list_for_each_entry(ref, &entry->refs, list) {
+			guard(mutex)(&ref->lock);
+
 			if (ref->lookup) {
 				gpiod_remove_lookup_table(ref->lookup);
 				kfree(ref->lookup->table[0].key);
@@ -500,6 +508,7 @@ void gpio_device_teardown_shared(struct gpio_device *gdev)
 
 			gpio_shared_remove_adev(&ref->adev);
 		}
+		lockdep_on();
 	}
 }
 
@@ -523,8 +532,6 @@ static void gpiod_shared_put(void *data)
 {
 	struct gpio_shared_entry *entry = data;
 
-	lockdep_assert_not_held(&gpio_shared_lock);
-
 	kref_put(&entry->ref, gpio_shared_release);
 }
 
@@ -562,8 +569,6 @@ struct gpio_shared_desc *devm_gpiod_shared_get(struct device *dev)
 	struct gpio_shared_entry *entry;
 	int ret;
 
-	lockdep_assert_not_held(&gpio_shared_lock);
-
 	entry = dev_get_platdata(dev);
 	if (WARN_ON(!entry))
 		/* Programmer bug */
@@ -598,6 +603,7 @@ EXPORT_SYMBOL_GPL(devm_gpiod_shared_get);
 static void gpio_shared_drop_ref(struct gpio_shared_ref *ref)
 {
 	list_del(&ref->list);
+	mutex_destroy(&ref->lock);
 	kfree(ref->con_id);
 	ida_free(&gpio_shared_ida, ref->dev_id);
 	fwnode_handle_put(ref->fwnode);

-- 
2.51.0

Re: [PATCH 0/4] gpio: shared: fix some bugs in unregister and error paths

[Bartosz Golaszewski]

On Sat, 06 Dec 2025 12:53:52 +0100, Bartosz Golaszewski wrote:
> Here are a couple more fixes for shared GPIOs that I want to send before
> the end of the merge window.
> 
> 

Applied, thanks!

[1/4] gpio: shared: fix NULL-pointer dereference in teardown path
      https://git.kernel.org/brgl/linux/c/e2c4175b8d3b3ea65fc3801c190bd93fe8b7a7a9
[2/4] gpio: shared: check if a reference is populated before cleaning its resources
      https://git.kernel.org/brgl/linux/c/c904a0d8525d5f03529ae3176e99bd32466ece7b
[3/4] gpio: shared: fix auxiliary device cleanup order
      https://git.kernel.org/brgl/linux/c/d382c765d083ad871b4a053059351edd348a2442
[4/4] gpio: shared: make locking more fine-grained
      https://git.kernel.org/brgl/linux/c/ea513dd3c066074b12e788114b45e0f2bda382cc

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