[PATCH] gpio: shared: fix a race in shared desc release path

[PATCH] gpio: shared: fix a race in shared desc release path

[Bartosz Golaszewski]

gpiod_shared_put() races with concurrent calls to
devm_gpiod_shared_get() like so:

CPU 0 (unbind / probe failure)      CPU 1 (concurrent proxy probe)
==============================      ==============================
gpiod_shared_put()
  kref_put(&entry->ref, ...)
    refcount_dec_and_test() // -> 0
                                    devm_gpiod_shared_get()
                                      mutex_lock(&entry->lock);
                                      if (entry->shared_desc) {
                                        kref_get(&entry->ref); // on 0!
                                        shared_desc = entry->shared_desc;
                                      }
                                      mutex_unlock(&entry->lock);
    gpio_shared_release()
      mutex_lock(&entry->lock);
      kfree(shared_desc);
      entry->shared_desc = NULL;
      mutex_unlock(&entry->lock);
                                      // uses freed shared_desc

Because gpiod_shared_put() decrements entry->ref without holding
entry->lock and gpio_shared_release() only acquires entry->lock after the
reference count has already reached zero, a concurrent call to
devm_gpiod_shared_get() can acquire entry->lock in the intermediate window
and observe a non-NULL entry->shared_desc. This can result in a
use-after-free and a double-free when the second consumer releases its
reference.

Use kref_put_mutex() to ensure the entry lock is taken without a window
in which a concurrent user can see a non-NULL entry->shared_desc while
it's being released.

Cc: stable@vger.kernel.org
Closes: https://sashiko.dev/#/bug/linux-30e071e4-4e4a-4b63-bd83-df1147055d35
Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/gpio/gpiolib-shared.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 8c9ac9d9330490b17fb3936b8758beb97a308a15..f6cf298e28b082d7719004b92314eaed1184f2be 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -635,8 +635,6 @@ static void gpio_shared_release(struct kref *kref)
 		container_of(kref, struct gpio_shared_entry, ref);
 	struct gpio_shared_desc *shared_desc;
 
-	guard(mutex)(&entry->lock);
-
 	shared_desc = entry->shared_desc;
 	gpio_device_put(shared_desc->desc->gdev);
 	mutex_destroy(&shared_desc->mutex);
@@ -648,7 +646,7 @@ static void gpiod_shared_put(void *data)
 {
 	struct gpio_shared_entry *entry = data;
 
-	kref_put(&entry->ref, gpio_shared_release);
+	kref_put_mutex(&entry->ref, gpio_shared_release, &entry->lock);
 }
 
 static struct gpio_shared_desc *

---
base-commit: ddfe51b0a0f5dc9e415cc2c8ad8db17532dd426b
change-id: 20261106-gpio-shared-refcnt-race-2b1eddecadbd

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

Re: [PATCH] gpio: shared: fix a race in shared desc release path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] gpio: shared: missing mutex_unlock() in gpio_shared_release()
--

commit 5cfdf2552fd680712c24f5438a69c1cd0f6f788d
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

gpio: shared: fix a race in shared desc release path

This commit switches gpiod_shared_put() from kref_put() to
kref_put_mutex() so that entry->lock is acquired atomically when the last
reference is dropped, and removes guard(mutex)(&entry->lock) from
gpio_shared_release().

> diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
> index 8c9ac9d933049..f6cf298e28b08 100644
> --- a/drivers/gpio/gpiolib-shared.c
> +++ b/drivers/gpio/gpiolib-shared.c
> @@ -635,8 +635,6 @@ static void gpio_shared_release(struct kref *kref)
>  		container_of(kref, struct gpio_shared_entry, ref);
>  	struct gpio_shared_desc *shared_desc;
>  
> -	guard(mutex)(&entry->lock);
> -
>  	shared_desc = entry->shared_desc;
>  	gpio_device_put(shared_desc->desc->gdev);
>  	mutex_destroy(&shared_desc->mutex);
> @@ -648,7 +646,7 @@ static void gpiod_shared_put(void *data)
>  {
>  	struct gpio_shared_entry *entry = data;
>  
> -	kref_put(&entry->ref, gpio_shared_release);
> +	kref_put_mutex(&entry->ref, gpio_shared_release, &entry->lock);
>  }

[Severity: High]
Does gpio_shared_release() leave entry->lock locked when the last
reference is dropped?  kref_put_mutex() acquires entry->lock via
refcount_dec_and_mutex_lock() and requires the release callback to call
mutex_unlock(&entry->lock), so removing guard(mutex)(&entry->lock) without
adding mutex_unlock(&entry->lock) causes any later caller acquiring
entry->lock, such as devm_gpiod_shared_get(), to deadlock.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-gpio-shared-refcnt-race-v1-1-79c9167505ab@oss.qualcomm.com?part=1