mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array()
@ 2026-10-21 19:02 Dmitry Torokhov
  2026-10-21 19:02 ` [PATCH 2/5] gpiolib: fix fast-path cancellation " Dmitry Torokhov
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2026-10-21 19:02 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: Andy Shevchenko, Janusz Krzysztofik, linux-gpio, linux-kernel

In gpiod_get_array(), descs->ndescs is used as the loop induction
variable and is only incremented at the end of each iteration. When
krealloc() fails on the first iteration (when index is 0), descs->desc[0]
has already been acquired via gpiod_get_index(), but descs->ndescs is
still 0. Consequently, gpiod_put_array() frees descs without releasing
descs->desc[0], leaking the descriptor.

Fix this by using a dedicated loop induction variable and incrementing
descs->ndescs immediately upon storing the retrieved descriptor.
Additionally, since the fast-path array_info setup only executes for
the first element (index 0), simplify the bitmap_set() calls to start at
bit 0 for count bits instead of calculating offsets from the loop index.

Fixes: bf9346f5d47b ("gpiolib: Identify arrays matching GPIO hardware")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpiolib.c | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index c0394c7d6279..21c9cea71c68 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -5193,7 +5193,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 	struct gpio_descs *descs;
 	struct gpio_device *gdev;
 	struct gpio_array *array_info = NULL;
-	int count, bitmap_size;
+	int i, count, bitmap_size;
 	unsigned long dflags;
 	size_t descs_size;
 
@@ -5206,21 +5206,21 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 	if (!descs)
 		return ERR_PTR(-ENOMEM);
 
-	for (descs->ndescs = 0; descs->ndescs < count; descs->ndescs++) {
-		desc = gpiod_get_index(dev, con_id, descs->ndescs, flags);
+	for (i = 0; i < count; i++) {
+		desc = gpiod_get_index(dev, con_id, i, flags);
 		if (IS_ERR(desc)) {
 			gpiod_put_array(descs);
 			return ERR_CAST(desc);
 		}
 
-		descs->desc[descs->ndescs] = desc;
+		descs->desc[descs->ndescs++] = desc;
 
 		gdev = gpiod_to_gpio_device(desc);
 		/*
 		 * If pin hardware number of array member 0 is also 0, select
 		 * its chip as a candidate for fast bitmap processing path.
 		 */
-		if (descs->ndescs == 0 && gpiod_hwgpio(desc) == 0) {
+		if (i == 0 && gpiod_hwgpio(desc) == 0) {
 			struct gpio_descs *array;
 
 			bitmap_size = BITS_TO_LONGS(gdev->ngpio > count ?
@@ -5245,10 +5245,8 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 			array_info->desc = descs->desc;
 			array_info->size = count;
 			array_info->gdev = gdev;
-			bitmap_set(array_info->get_mask, descs->ndescs,
-				   count - descs->ndescs);
-			bitmap_set(array_info->set_mask, descs->ndescs,
-				   count - descs->ndescs);
+			bitmap_set(array_info->get_mask, 0, count);
+			bitmap_set(array_info->set_mask, 0, count);
 			descs->info = array_info;
 		}
 
@@ -5258,38 +5256,34 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 
 		/* Unmark array members which don't belong to the 'fast' chip */
 		if (array_info->gdev != gdev) {
-			__clear_bit(descs->ndescs, array_info->get_mask);
-			__clear_bit(descs->ndescs, array_info->set_mask);
+			__clear_bit(i, array_info->get_mask);
+			__clear_bit(i, array_info->set_mask);
 		}
 		/*
 		 * Detect array members which belong to the 'fast' chip
 		 * but their pins are not in hardware order.
 		 */
-		else if (gpiod_hwgpio(desc) != descs->ndescs) {
+		else if (gpiod_hwgpio(desc) != i) {
 			/*
 			 * Don't use fast path if all array members processed so
 			 * far belong to the same chip as this one but its pin
 			 * hardware number is different from its array index.
 			 */
-			if (bitmap_full(array_info->get_mask, descs->ndescs)) {
+			if (bitmap_full(array_info->get_mask, i)) {
 				array_info = NULL;
 			} else {
-				__clear_bit(descs->ndescs,
-					    array_info->get_mask);
-				__clear_bit(descs->ndescs,
-					    array_info->set_mask);
+				__clear_bit(i, array_info->get_mask);
+				__clear_bit(i, array_info->set_mask);
 			}
 		} else {
 			dflags = READ_ONCE(desc->flags);
 			/* Exclude open drain or open source from fast output */
 			if (test_bit(GPIOD_FLAG_OPEN_DRAIN, &dflags) ||
 			    test_bit(GPIOD_FLAG_OPEN_SOURCE, &dflags))
-				__clear_bit(descs->ndescs,
-					    array_info->set_mask);
+				__clear_bit(i, array_info->set_mask);
 			/* Identify 'fast' pins which require invertion */
 			if (gpiod_is_active_low(desc))
-				__set_bit(descs->ndescs,
-					  array_info->invert_mask);
+				__set_bit(i, array_info->invert_mask);
 		}
 	}
 	if (array_info)
-- 
2.56.0.307.g5fe7bd680c-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/5] gpiolib: fix fast-path cancellation in gpiod_get_array()
  2026-10-21 19:02 [PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array() Dmitry Torokhov
@ 2026-10-21 19:02 ` Dmitry Torokhov
  2026-10-21 19:02 ` [PATCH 3/5] gpiolib: add support for scope-based management of GPIO descriptors Dmitry Torokhov
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2026-10-21 19:02 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: Andy Shevchenko, Janusz Krzysztofik, linux-gpio, linux-kernel

In gpiod_get_array(), when a pin on the same chip is not in hardware
order and the preceding pins form a contiguous block, the fast path is
cancelled by setting array_info to NULL. However, descs->info was
already populated with the array_info pointer during the first
iteration, leaving descs->info pointing to the aborted array_info.
Consequently, subsequent array operations still use the fast path with
remaining bits set in the masks and wrong hardware pin numbers.

Clear descs->info alongside array_info when cancelling the fast path so
callers do not erroneously use the aborted array_info.

Fixes: c4c958aa64f3 ("gpiolib: Fix array members of same chip processed separately")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpiolib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 21c9cea71c68..144a63e6692c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -5270,6 +5270,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 			 * hardware number is different from its array index.
 			 */
 			if (bitmap_full(array_info->get_mask, i)) {
+				descs->info = NULL;
 				array_info = NULL;
 			} else {
 				__clear_bit(i, array_info->get_mask);
-- 
2.56.0.307.g5fe7bd680c-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/5] gpiolib: add support for scope-based management of GPIO descriptors
  2026-10-21 19:02 [PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array() Dmitry Torokhov
  2026-10-21 19:02 ` [PATCH 2/5] gpiolib: fix fast-path cancellation " Dmitry Torokhov
@ 2026-10-21 19:02 ` Dmitry Torokhov
  2026-10-21 19:02 ` [PATCH 4/5] gpiolib: use __free(gpiod_put_array) in gpiod_get_array() Dmitry Torokhov
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2026-10-21 19:02 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: Andy Shevchenko, Janusz Krzysztofik, linux-gpio, linux-kernel

Add DEFINE_FREE() definitions for gpiod_put and gpiod_put_array to
allow using __free(gpiod_put) and __free(gpiod_put_array) for automatic,
scope-based cleanup of individual GPIO descriptors and descriptor
arrays. Place these definitions after the CONFIG_GPIOLIB check so that
they are available regardless of whether CONFIG_GPIOLIB is enabled.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 include/linux/gpio/consumer.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index fceeefd5f893..ee183a97575c 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -3,6 +3,7 @@
 #define __LINUX_GPIO_CONSUMER_H
 
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/err.h>
 #include <linux/types.h>
 
@@ -578,6 +579,9 @@ gpiod_is_equal(const struct gpio_desc *desc, const struct gpio_desc *other)
 
 #endif /* CONFIG_GPIOLIB */
 
+DEFINE_FREE(gpiod_put, struct gpio_desc *, if (!IS_ERR_OR_NULL(_T)) gpiod_put(_T))
+DEFINE_FREE(gpiod_put_array, struct gpio_descs *, if (!IS_ERR_OR_NULL(_T)) gpiod_put_array(_T))
+
 #if IS_ENABLED(CONFIG_GPIOLIB) && IS_ENABLED(CONFIG_HTE)
 int gpiod_enable_hw_timestamp_ns(struct gpio_desc *desc, unsigned long flags);
 int gpiod_disable_hw_timestamp_ns(struct gpio_desc *desc, unsigned long flags);
-- 
2.56.0.307.g5fe7bd680c-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/5] gpiolib: use __free(gpiod_put_array) in gpiod_get_array()
  2026-10-21 19:02 [PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array() Dmitry Torokhov
  2026-10-21 19:02 ` [PATCH 2/5] gpiolib: fix fast-path cancellation " Dmitry Torokhov
  2026-10-21 19:02 ` [PATCH 3/5] gpiolib: add support for scope-based management of GPIO descriptors Dmitry Torokhov
@ 2026-10-21 19:02 ` Dmitry Torokhov
  2026-10-21 19:02 ` [PATCH 5/5] gpiolib: avoid krealloc() " Dmitry Torokhov
  2026-10-21 19:36 ` [PATCH 1/5] gpiolib: fix GPIO descriptor leak " Andy Shevchenko
  4 siblings, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2026-10-21 19:02 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: Andy Shevchenko, Janusz Krzysztofik, linux-gpio, linux-kernel

Switch to using __free(gpiod_put_array) scope-based cleanup facility
in gpiod_get_array(), simplifying error unwinding and eliminating manual
gpiod_put_array() calls.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpiolib.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 144a63e6692c..79f3be30ace1 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -5190,28 +5190,24 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 						enum gpiod_flags flags)
 {
 	struct gpio_desc *desc;
-	struct gpio_descs *descs;
 	struct gpio_device *gdev;
 	struct gpio_array *array_info = NULL;
 	int i, count, bitmap_size;
 	unsigned long dflags;
-	size_t descs_size;
 
 	count = gpiod_count(dev, con_id);
 	if (count < 0)
 		return ERR_PTR(count);
 
-	descs_size = struct_size(descs, desc, count);
-	descs = kzalloc(descs_size, GFP_KERNEL);
+	struct gpio_descs *descs __free(gpiod_put_array) =
+		kzalloc_flex(*descs, desc, count);
 	if (!descs)
 		return ERR_PTR(-ENOMEM);
 
 	for (i = 0; i < count; i++) {
 		desc = gpiod_get_index(dev, con_id, i, flags);
-		if (IS_ERR(desc)) {
-			gpiod_put_array(descs);
+		if (IS_ERR(desc))
 			return ERR_CAST(desc);
-		}
 
 		descs->desc[descs->ndescs++] = desc;
 
@@ -5222,6 +5218,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 		 */
 		if (i == 0 && gpiod_hwgpio(desc) == 0) {
 			struct gpio_descs *array;
+			size_t descs_size = struct_size(descs, desc, count);
 
 			bitmap_size = BITS_TO_LONGS(gdev->ngpio > count ?
 						    gdev->ngpio : count);
@@ -5229,10 +5226,8 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 			array = krealloc(descs, descs_size +
 					 struct_size(array_info, invert_mask, 3 * bitmap_size),
 					 GFP_KERNEL | __GFP_ZERO);
-			if (!array) {
-				gpiod_put_array(descs);
+			if (!array)
 				return ERR_PTR(-ENOMEM);
-			}
 
 			descs = array;
 
@@ -5293,7 +5288,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 			array_info->gdev->label, array_info->size,
 			*array_info->get_mask, *array_info->set_mask,
 			*array_info->invert_mask);
-	return descs;
+	return_ptr(descs);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_array);
 
-- 
2.56.0.307.g5fe7bd680c-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5/5] gpiolib: avoid krealloc() in gpiod_get_array()
  2026-10-21 19:02 [PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array() Dmitry Torokhov
                   ` (2 preceding siblings ...)
  2026-10-21 19:02 ` [PATCH 4/5] gpiolib: use __free(gpiod_put_array) in gpiod_get_array() Dmitry Torokhov
@ 2026-10-21 19:02 ` Dmitry Torokhov
  2026-10-21 19:34   ` Andy Shevchenko
  2026-10-21 19:39   ` Andy Shevchenko
  2026-10-21 19:36 ` [PATCH 1/5] gpiolib: fix GPIO descriptor leak " Andy Shevchenko
  4 siblings, 2 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2026-10-21 19:02 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: Andy Shevchenko, Janusz Krzysztofik, linux-gpio, linux-kernel

Instead of allocating descs before the loop and reallocating it via
krealloc() when candidate hardware array matching is detected on the
first iteration, defer allocation of descs until the first descriptor
is retrieved.

Factor descriptor allocation and fast-path validation into helper
functions gpio_descs_alloc() and gpio_descs_check_fastpath(). Inside
the loop, acquire the descriptor using __free(gpiod_put), allocate
descs once on the first iteration, check fast-path candidate
suitability, and transfer descriptor ownership using no_free_ptr().

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpiolib.c | 184 ++++++++++++++++++++++-------------------
 1 file changed, 99 insertions(+), 85 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 79f3be30ace1..177ca7748dae 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -5171,6 +5171,90 @@ static void gpiochip_free_hogs(struct gpio_chip *gc)
 		gpiochip_free_own_desc(desc);
 }
 
+static struct gpio_descs *gpio_descs_alloc(unsigned int count, struct gpio_desc *first_desc)
+{
+	struct gpio_device *gdev = gpiod_to_gpio_device(first_desc);
+	struct gpio_array *array_info;
+	struct gpio_descs *descs;
+	size_t descs_size, alloc_size;
+	unsigned int bitmap_size;
+
+	/*
+	 * If pin hardware number of array member 0 is also 0, select
+	 * its chip as a candidate for fast bitmap processing path.
+	 */
+	if (gpiod_hwgpio(first_desc) != 0)
+		return kzalloc_flex(*descs, desc, count);
+
+	descs_size = struct_size(descs, desc, count);
+	bitmap_size = BITS_TO_LONGS(max(gdev->ngpio, count));
+	alloc_size = descs_size + struct_size(array_info, invert_mask, 3 * bitmap_size);
+
+	descs = kzalloc(alloc_size, GFP_KERNEL);
+	if (!descs)
+		return NULL;
+
+	array_info = (void *)descs + descs_size;
+	array_info->get_mask = array_info->invert_mask + bitmap_size;
+	array_info->set_mask = array_info->get_mask + bitmap_size;
+
+	array_info->desc = descs->desc;
+	array_info->size = count;
+	array_info->gdev = gdev;
+	bitmap_set(array_info->get_mask, 0, count);
+	bitmap_set(array_info->set_mask, 0, count);
+	descs->info = array_info;
+
+	return descs;
+}
+
+static void gpio_descs_check_fastpath(struct gpio_descs *descs,
+				      struct gpio_desc *desc)
+{
+	struct gpio_array *array_info = descs->info;
+	unsigned int idx = descs->ndescs;
+	unsigned long dflags;
+
+	if (!array_info)
+		return;
+
+	/* Unmark array members which don't belong to the 'fast' chip */
+	if (array_info->gdev != gpiod_to_gpio_device(desc)) {
+		__clear_bit(idx, array_info->get_mask);
+		__clear_bit(idx, array_info->set_mask);
+		return;
+	}
+
+	/*
+	 * Detect array members which belong to the 'fast' chip
+	 * but their pins are not in hardware order.
+	 */
+	if (gpiod_hwgpio(desc) != idx) {
+		/*
+		 * Don't use fast path if all array members processed so
+		 * far belong to the same chip as this one but its pin
+		 * hardware number is different from its array index.
+		 */
+		if (bitmap_full(array_info->get_mask, idx)) {
+			descs->info = NULL;
+		} else {
+			__clear_bit(idx, array_info->get_mask);
+			__clear_bit(idx, array_info->set_mask);
+		}
+		return;
+	}
+
+	dflags = READ_ONCE(desc->flags);
+	/* Exclude open drain or open source from fast output */
+	if (test_bit(GPIOD_FLAG_OPEN_DRAIN, &dflags) ||
+	    test_bit(GPIOD_FLAG_OPEN_SOURCE, &dflags))
+		__clear_bit(idx, array_info->set_mask);
+
+	/* Identify 'fast' pins which require invertion */
+	if (gpiod_is_active_low(desc))
+		__set_bit(idx, array_info->invert_mask);
+}
+
 /**
  * gpiod_get_array - obtain multiple GPIOs from a multi-index GPIO function
  * @dev:	GPIO consumer, can be NULL for system-global GPIOs
@@ -5189,105 +5273,35 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 						const char *con_id,
 						enum gpiod_flags flags)
 {
-	struct gpio_desc *desc;
-	struct gpio_device *gdev;
-	struct gpio_array *array_info = NULL;
-	int i, count, bitmap_size;
-	unsigned long dflags;
+	struct gpio_descs *descs __free(gpiod_put_array) = NULL;
+	int i, count;
 
 	count = gpiod_count(dev, con_id);
-	if (count < 0)
-		return ERR_PTR(count);
-
-	struct gpio_descs *descs __free(gpiod_put_array) =
-		kzalloc_flex(*descs, desc, count);
-	if (!descs)
-		return ERR_PTR(-ENOMEM);
+	if (count <= 0)
+		return ERR_PTR(count ?: -ENOENT);
 
 	for (i = 0; i < count; i++) {
-		desc = gpiod_get_index(dev, con_id, i, flags);
+		struct gpio_desc *desc __free(gpiod_put) =
+			gpiod_get_index(dev, con_id, i, flags);
 		if (IS_ERR(desc))
 			return ERR_CAST(desc);
 
-		descs->desc[descs->ndescs++] = desc;
-
-		gdev = gpiod_to_gpio_device(desc);
-		/*
-		 * If pin hardware number of array member 0 is also 0, select
-		 * its chip as a candidate for fast bitmap processing path.
-		 */
-		if (i == 0 && gpiod_hwgpio(desc) == 0) {
-			struct gpio_descs *array;
-			size_t descs_size = struct_size(descs, desc, count);
-
-			bitmap_size = BITS_TO_LONGS(gdev->ngpio > count ?
-						    gdev->ngpio : count);
-
-			array = krealloc(descs, descs_size +
-					 struct_size(array_info, invert_mask, 3 * bitmap_size),
-					 GFP_KERNEL | __GFP_ZERO);
-			if (!array)
+		if (i == 0) {
+			descs = gpio_descs_alloc(count, desc);
+			if (!descs)
 				return ERR_PTR(-ENOMEM);
-
-			descs = array;
-
-			array_info = (void *)descs + descs_size;
-			array_info->get_mask = array_info->invert_mask +
-						  bitmap_size;
-			array_info->set_mask = array_info->get_mask +
-						  bitmap_size;
-
-			array_info->desc = descs->desc;
-			array_info->size = count;
-			array_info->gdev = gdev;
-			bitmap_set(array_info->get_mask, 0, count);
-			bitmap_set(array_info->set_mask, 0, count);
-			descs->info = array_info;
 		}
 
-		/* If there is no cache for fast bitmap processing path, continue */
-		if (!array_info)
-			continue;
+		gpio_descs_check_fastpath(descs, desc);
 
-		/* Unmark array members which don't belong to the 'fast' chip */
-		if (array_info->gdev != gdev) {
-			__clear_bit(i, array_info->get_mask);
-			__clear_bit(i, array_info->set_mask);
-		}
-		/*
-		 * Detect array members which belong to the 'fast' chip
-		 * but their pins are not in hardware order.
-		 */
-		else if (gpiod_hwgpio(desc) != i) {
-			/*
-			 * Don't use fast path if all array members processed so
-			 * far belong to the same chip as this one but its pin
-			 * hardware number is different from its array index.
-			 */
-			if (bitmap_full(array_info->get_mask, i)) {
-				descs->info = NULL;
-				array_info = NULL;
-			} else {
-				__clear_bit(i, array_info->get_mask);
-				__clear_bit(i, array_info->set_mask);
-			}
-		} else {
-			dflags = READ_ONCE(desc->flags);
-			/* Exclude open drain or open source from fast output */
-			if (test_bit(GPIOD_FLAG_OPEN_DRAIN, &dflags) ||
-			    test_bit(GPIOD_FLAG_OPEN_SOURCE, &dflags))
-				__clear_bit(i, array_info->set_mask);
-			/* Identify 'fast' pins which require invertion */
-			if (gpiod_is_active_low(desc))
-				__set_bit(i, array_info->invert_mask);
-		}
+		descs->desc[descs->ndescs++] = no_free_ptr(desc);
 	}
-	if (array_info)
+	if (descs->info)
 		dev_dbg(dev,
 			"GPIO array info: chip=%s, size=%d, get_mask=%lx, set_mask=%lx, invert_mask=%lx\n",
-			array_info->gdev->label, array_info->size,
-			*array_info->get_mask, *array_info->set_mask,
-			*array_info->invert_mask);
+			descs->info->gdev->label, descs->info->size,
+			*descs->info->get_mask, *descs->info->set_mask,
+			*descs->info->invert_mask);
 	return_ptr(descs);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_array);
-- 
2.56.0.307.g5fe7bd680c-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 5/5] gpiolib: avoid krealloc() in gpiod_get_array()
  2026-10-21 19:02 ` [PATCH 5/5] gpiolib: avoid krealloc() " Dmitry Torokhov
@ 2026-10-21 19:34   ` Andy Shevchenko
  2026-10-21 19:39   ` Andy Shevchenko
  1 sibling, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2026-10-21 19:34 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Janusz Krzysztofik, linux-gpio, linux-kernel

On Wed, Oct 21, 2026 at 12:02:10PM -0700, Dmitry Torokhov wrote:
> Instead of allocating descs before the loop and reallocating it via
> krealloc() when candidate hardware array matching is detected on the
> first iteration, defer allocation of descs until the first descriptor
> is retrieved.
> 
> Factor descriptor allocation and fast-path validation into helper
> functions gpio_descs_alloc() and gpio_descs_check_fastpath(). Inside
> the loop, acquire the descriptor using __free(gpiod_put), allocate
> descs once on the first iteration, check fast-path candidate
> suitability, and transfer descriptor ownership using no_free_ptr().

...

> +static struct gpio_descs *gpio_descs_alloc(unsigned int count, struct gpio_desc *first_desc)
> +{
> +	struct gpio_device *gdev = gpiod_to_gpio_device(first_desc);
> +	struct gpio_array *array_info;
> +	struct gpio_descs *descs;
> +	size_t descs_size, alloc_size;
> +	unsigned int bitmap_size;
> +
> +	/*
> +	 * If pin hardware number of array member 0 is also 0, select
> +	 * its chip as a candidate for fast bitmap processing path.
> +	 */
> +	if (gpiod_hwgpio(first_desc) != 0)
> +		return kzalloc_flex(*descs, desc, count);
> +
> +	descs_size = struct_size(descs, desc, count);
> +	bitmap_size = BITS_TO_LONGS(max(gdev->ngpio, count));
> +	alloc_size = descs_size + struct_size(array_info, invert_mask, 3 * bitmap_size);
> +
> +	descs = kzalloc(alloc_size, GFP_KERNEL);
> +	if (!descs)
> +		return NULL;
> +
> +	array_info = (void *)descs + descs_size;

> +	array_info->get_mask = array_info->invert_mask + bitmap_size;
> +	array_info->set_mask = array_info->get_mask + bitmap_size;

> +	array_info->desc = descs->desc;
> +	array_info->size = count;
> +	array_info->gdev = gdev;
> +	bitmap_set(array_info->get_mask, 0, count);
> +	bitmap_set(array_info->set_mask, 0, count);

With the above being done, I would rather see bitmap_zalloc() for bitmaps.

> +	descs->info = array_info;
> +
> +	return descs;
> +}

...

>  	count = gpiod_count(dev, con_id);
> -	if (count < 0)
> -		return ERR_PTR(count);

Leave this and simply add

> -	struct gpio_descs *descs __free(gpiod_put_array) =
> -		kzalloc_flex(*descs, desc, count);
> -	if (!descs)
> -		return ERR_PTR(-ENOMEM);
> +	if (count <= 0)
> +		return ERR_PTR(count ?: -ENOENT);

	if (count == 0)
		return -ENOENT;

The currently proposed code is less readable.

...

>  	for (i = 0; i < count; i++) {
> -		desc = gpiod_get_index(dev, con_id, i, flags);
> +		struct gpio_desc *desc __free(gpiod_put) =
> +			gpiod_get_index(dev, con_id, i, flags);
>  		if (IS_ERR(desc))
>  			return ERR_CAST(desc);
>  
> -		descs->desc[descs->ndescs++] = desc;
> -
> -		gdev = gpiod_to_gpio_device(desc);
> -		/*
> -		 * If pin hardware number of array member 0 is also 0, select
> -		 * its chip as a candidate for fast bitmap processing path.
> -		 */
> -		if (i == 0 && gpiod_hwgpio(desc) == 0) {
> -			struct gpio_descs *array;
> -			size_t descs_size = struct_size(descs, desc, count);
> -
> -			bitmap_size = BITS_TO_LONGS(gdev->ngpio > count ?
> -						    gdev->ngpio : count);
> -
> -			array = krealloc(descs, descs_size +
> -					 struct_size(array_info, invert_mask, 3 * bitmap_size),
> -					 GFP_KERNEL | __GFP_ZERO);
> -			if (!array)
> +>  		if (i == 0) {

This is almost invariant to the loop. Can we move it out?

> +			descs = gpio_descs_alloc(count, desc);
> +			if (!descs)
>  				return ERR_PTR(-ENOMEM);
> -
> -			descs = array;
> -
> -			array_info = (void *)descs + descs_size;
> -			array_info->get_mask = array_info->invert_mask +
> -						  bitmap_size;
> -			array_info->set_mask = array_info->get_mask +
> -						  bitmap_size;
> -
> -			array_info->desc = descs->desc;
> -			array_info->size = count;
> -			array_info->gdev = gdev;
> -			bitmap_set(array_info->get_mask, 0, count);
> -			bitmap_set(array_info->set_mask, 0, count);
> -			descs->info = array_info;
>  		}
>  
> -		/* If there is no cache for fast bitmap processing path, continue */
> -		if (!array_info)
> -			continue;
> +		gpio_descs_check_fastpath(descs, desc);
>  
> -		/* Unmark array members which don't belong to the 'fast' chip */
> -		if (array_info->gdev != gdev) {
> -			__clear_bit(i, array_info->get_mask);
> -			__clear_bit(i, array_info->set_mask);
> -		}
> -		/*
> -		 * Detect array members which belong to the 'fast' chip
> -		 * but their pins are not in hardware order.
> -		 */
> -		else if (gpiod_hwgpio(desc) != i) {
> -			/*
> -			 * Don't use fast path if all array members processed so
> -			 * far belong to the same chip as this one but its pin
> -			 * hardware number is different from its array index.
> -			 */
> -			if (bitmap_full(array_info->get_mask, i)) {
> -				descs->info = NULL;
> -				array_info = NULL;
> -			} else {
> -				__clear_bit(i, array_info->get_mask);
> -				__clear_bit(i, array_info->set_mask);
> -			}
> -		} else {
> -			dflags = READ_ONCE(desc->flags);
> -			/* Exclude open drain or open source from fast output */
> -			if (test_bit(GPIOD_FLAG_OPEN_DRAIN, &dflags) ||
> -			    test_bit(GPIOD_FLAG_OPEN_SOURCE, &dflags))
> -				__clear_bit(i, array_info->set_mask);
> -			/* Identify 'fast' pins which require invertion */
> -			if (gpiod_is_active_low(desc))
> -				__set_bit(i, array_info->invert_mask);
> -		}
> +		descs->desc[descs->ndescs++] = no_free_ptr(desc);
>  	}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array()
  2026-10-21 19:02 [PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array() Dmitry Torokhov
                   ` (3 preceding siblings ...)
  2026-10-21 19:02 ` [PATCH 5/5] gpiolib: avoid krealloc() " Dmitry Torokhov
@ 2026-10-21 19:36 ` Andy Shevchenko
  2026-10-21 19:43   ` Dmitry Torokhov
  4 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2026-10-21 19:36 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Janusz Krzysztofik, linux-gpio, linux-kernel

On Wed, Oct 21, 2026 at 12:02:06PM -0700, Dmitry Torokhov wrote:
> In gpiod_get_array(), descs->ndescs is used as the loop induction
> variable and is only incremented at the end of each iteration. When
> krealloc() fails on the first iteration (when index is 0), descs->desc[0]
> has already been acquired via gpiod_get_index(), but descs->ndescs is
> still 0. Consequently, gpiod_put_array() frees descs without releasing
> descs->desc[0], leaking the descriptor.
> 
> Fix this by using a dedicated loop induction variable and incrementing
> descs->ndescs immediately upon storing the retrieved descriptor.
> Additionally, since the fast-path array_info setup only executes for
> the first element (index 0), simplify the bitmap_set() calls to start at
> bit 0 for count bits instead of calculating offsets from the loop index.

Can you start doing cover letters, please? The problem is if I want to give a
tag for the entire series I will need to iterate over all patches, while `b4`
has facility to do it for me. Also with --base it would be easier to see which
base you have used for the series.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 5/5] gpiolib: avoid krealloc() in gpiod_get_array()
  2026-10-21 19:02 ` [PATCH 5/5] gpiolib: avoid krealloc() " Dmitry Torokhov
  2026-10-21 19:34   ` Andy Shevchenko
@ 2026-10-21 19:39   ` Andy Shevchenko
  1 sibling, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2026-10-21 19:39 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Janusz Krzysztofik, linux-gpio, linux-kernel

On Wed, Oct 21, 2026 at 12:02:10PM -0700, Dmitry Torokhov wrote:
> Instead of allocating descs before the loop and reallocating it via
> krealloc() when candidate hardware array matching is detected on the
> first iteration, defer allocation of descs until the first descriptor
> is retrieved.
> 
> Factor descriptor allocation and fast-path validation into helper
> functions gpio_descs_alloc() and gpio_descs_check_fastpath(). Inside
> the loop, acquire the descriptor using __free(gpiod_put), allocate
> descs once on the first iteration, check fast-path candidate
> suitability, and transfer descriptor ownership using no_free_ptr().

...

> +static void gpio_descs_check_fastpath(struct gpio_descs *descs,
> +				      struct gpio_desc *desc)
> +{
> +	struct gpio_array *array_info = descs->info;

Split the assignment. It's robust for long-term maintenance to have assignment
coupled with a validation check.

> +	unsigned int idx = descs->ndescs;
> +	unsigned long dflags;


	array_info = desc->info;

> +	if (!array_info)
> +		return;
> +
> +	/* Unmark array members which don't belong to the 'fast' chip */
> +	if (array_info->gdev != gpiod_to_gpio_device(desc)) {
> +		__clear_bit(idx, array_info->get_mask);
> +		__clear_bit(idx, array_info->set_mask);
> +		return;
> +	}
> +
> +	/*
> +	 * Detect array members which belong to the 'fast' chip
> +	 * but their pins are not in hardware order.
> +	 */
> +	if (gpiod_hwgpio(desc) != idx) {
> +		/*
> +		 * Don't use fast path if all array members processed so
> +		 * far belong to the same chip as this one but its pin
> +		 * hardware number is different from its array index.
> +		 */
> +		if (bitmap_full(array_info->get_mask, idx)) {
> +			descs->info = NULL;
> +		} else {
> +			__clear_bit(idx, array_info->get_mask);
> +			__clear_bit(idx, array_info->set_mask);
> +		}
> +		return;
> +	}
> +
> +	dflags = READ_ONCE(desc->flags);
> +	/* Exclude open drain or open source from fast output */
> +	if (test_bit(GPIOD_FLAG_OPEN_DRAIN, &dflags) ||
> +	    test_bit(GPIOD_FLAG_OPEN_SOURCE, &dflags))
> +		__clear_bit(idx, array_info->set_mask);
> +
> +	/* Identify 'fast' pins which require invertion */
> +	if (gpiod_is_active_low(desc))
> +		__set_bit(idx, array_info->invert_mask);
> +}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array()
  2026-10-21 19:36 ` [PATCH 1/5] gpiolib: fix GPIO descriptor leak " Andy Shevchenko
@ 2026-10-21 19:43   ` Dmitry Torokhov
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2026-10-21 19:43 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Janusz Krzysztofik, linux-gpio, linux-kernel

On Wed, Oct 21, 2026 at 10:36:13PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 21, 2026 at 12:02:06PM -0700, Dmitry Torokhov wrote:
> > In gpiod_get_array(), descs->ndescs is used as the loop induction
> > variable and is only incremented at the end of each iteration. When
> > krealloc() fails on the first iteration (when index is 0), descs->desc[0]
> > has already been acquired via gpiod_get_index(), but descs->ndescs is
> > still 0. Consequently, gpiod_put_array() frees descs without releasing
> > descs->desc[0], leaking the descriptor.
> > 
> > Fix this by using a dedicated loop induction variable and incrementing
> > descs->ndescs immediately upon storing the retrieved descriptor.
> > Additionally, since the fast-path array_info setup only executes for
> > the first element (index 0), simplify the bitmap_set() calls to start at
> > bit 0 for count bits instead of calculating offsets from the loop index.
> 
> Can you start doing cover letters, please? The problem is if I want to give a
> tag for the entire series I will need to iterate over all patches, while `b4`
> has facility to do it for me. Also with --base it would be easier to see which
> base you have used for the series.

Alright. It was supposed to be a small patch so I did it in my patch
queue, but then it grew to series if 5...

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-10-21 19:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-21 19:02 [PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array() Dmitry Torokhov
2026-10-21 19:02 ` [PATCH 2/5] gpiolib: fix fast-path cancellation " Dmitry Torokhov
2026-10-21 19:02 ` [PATCH 3/5] gpiolib: add support for scope-based management of GPIO descriptors Dmitry Torokhov
2026-10-21 19:02 ` [PATCH 4/5] gpiolib: use __free(gpiod_put_array) in gpiod_get_array() Dmitry Torokhov
2026-10-21 19:02 ` [PATCH 5/5] gpiolib: avoid krealloc() " Dmitry Torokhov
2026-10-21 19:34   ` Andy Shevchenko
2026-10-21 19:39   ` Andy Shevchenko
2026-10-21 19:36 ` [PATCH 1/5] gpiolib: fix GPIO descriptor leak " Andy Shevchenko
2026-10-21 19:43   ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®