[PATCH] gpio: shared: shorten the critical section in gpiochip_setup_shared()

Bartosz Golaszewski posted 1 patch 1 week, 1 day ago
drivers/gpio/gpiolib-shared.c | 56 +++++++++++++++++++++----------------------
1 file changed, 28 insertions(+), 28 deletions(-)
[PATCH] gpio: shared: shorten the critical section in gpiochip_setup_shared()
Posted by Bartosz Golaszewski 1 week, 1 day ago
Commit 710abda58055 ("gpio: shared: call gpio_chip::of_xlate() if set")
introduced a critical section around the adjustmenet of entry->offset.
However this may cause a deadlock if we create the auxiliary shared
proxy devices with this lock taken. We only need to protect
entry->offset while it's read/written so shorten the critical section
and release the lock before creating the proxy device as the field in
question is no longer accessed at this point.

Fixes: 710abda58055 ("gpio: shared: call gpio_chip::of_xlate() if set")
Reported-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/gpio/gpiolib-shared.c | 56 +++++++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index e257212fa5e3df249de0d06eebdb2165ae734ebc..e02d6b93a4ab42b197f0fd64e4854a303f54f140 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -533,48 +533,48 @@ int gpiochip_setup_shared(struct gpio_chip *gc)
 	 * exposing shared pins. Find them and create the proxy devices.
 	 */
 	list_for_each_entry(entry, &gpio_shared_list, list) {
-		guard(mutex)(&entry->lock);
-
 		if (!device_match_fwnode(&gdev->dev, entry->fwnode))
 			continue;
 
 		if (list_count_nodes(&entry->refs) <= 1)
 			continue;
 
+		scoped_guard(mutex, &entry->lock) {
 #if IS_ENABLED(CONFIG_OF)
-		if (is_of_node(entry->fwnode) && gc->of_xlate) {
-			/*
-			 * This is the earliest that we can tranlate the
-			 * devicetree offset to the chip offset.
-			 */
-			struct of_phandle_args gpiospec = { };
+			if (is_of_node(entry->fwnode) && gc->of_xlate) {
+				/*
+				 * This is the earliest that we can tranlate the
+				 * devicetree offset to the chip offset.
+				 */
+				struct of_phandle_args gpiospec = { };
 
-			gpiospec.np = to_of_node(entry->fwnode);
-			gpiospec.args_count = 2;
-			gpiospec.args[0] = entry->offset;
+				gpiospec.np = to_of_node(entry->fwnode);
+				gpiospec.args_count = 2;
+				gpiospec.args[0] = entry->offset;
 
-			ret = gc->of_xlate(gc, &gpiospec, NULL);
-			if (ret < 0)
-				return ret;
+				ret = gc->of_xlate(gc, &gpiospec, NULL);
+				if (ret < 0)
+					return ret;
 
-			entry->offset = ret;
-		}
+				entry->offset = ret;
+			}
 #endif /* CONFIG_OF */
 
-		desc = &gdev->descs[entry->offset];
+			desc = &gdev->descs[entry->offset];
 
-		__set_bit(GPIOD_FLAG_SHARED, &desc->flags);
-		/*
-		 * Shared GPIOs are not requested via the normal path. Make
-		 * them inaccessible to anyone even before we register the
-		 * chip.
-		 */
-		ret = gpiod_request_commit(desc, "shared");
-		if (ret)
-			return ret;
+			__set_bit(GPIOD_FLAG_SHARED, &desc->flags);
+			/*
+			 * Shared GPIOs are not requested via the normal path. Make
+			 * them inaccessible to anyone even before we register the
+			 * chip.
+			 */
+			ret = gpiod_request_commit(desc, "shared");
+			if (ret)
+				return ret;
 
-		pr_debug("GPIO %u owned by %s is shared by multiple consumers\n",
-			 entry->offset, gpio_device_get_label(gdev));
+			pr_debug("GPIO %u owned by %s is shared by multiple consumers\n",
+				 entry->offset, gpio_device_get_label(gdev));
+		}
 
 		list_for_each_entry(ref, &entry->refs, list) {
 			pr_debug("Setting up a shared GPIO entry for %s (con_id: '%s')\n",

---
base-commit: 85964cdcad0fac9a0eb7b87a0f9d88cc074b854c
change-id: 20260325-gpio-shared-deadlock-b63622a86ce1

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Re: [PATCH] gpio: shared: shorten the critical section in gpiochip_setup_shared()
Posted by Bartosz Golaszewski 3 days, 16 hours ago
On Wed, 25 Mar 2026 12:06:38 +0100, Bartosz Golaszewski wrote:
> Commit 710abda58055 ("gpio: shared: call gpio_chip::of_xlate() if set")
> introduced a critical section around the adjustmenet of entry->offset.
> However this may cause a deadlock if we create the auxiliary shared
> proxy devices with this lock taken. We only need to protect
> entry->offset while it's read/written so shorten the critical section
> and release the lock before creating the proxy device as the field in
> question is no longer accessed at this point.
> 
> [...]

Applied, thanks!

[1/1] gpio: shared: shorten the critical section in gpiochip_setup_shared()
      https://git.kernel.org/brgl/c/310a4a9cbb17037668ea440f6a3964d00705b400

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