[PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array()

[PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array()

[Dmitry Torokhov]

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

[PATCH 2/5] gpiolib: fix fast-path cancellation in gpiod_get_array()

[Dmitry Torokhov]

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

[PATCH 3/5] gpiolib: add support for scope-based management of GPIO descriptors

[Dmitry Torokhov]

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

[PATCH 4/5] gpiolib: use __free(gpiod_put_array) in gpiod_get_array()

[Dmitry Torokhov]

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

[PATCH 5/5] gpiolib: avoid krealloc() in gpiod_get_array()

[Dmitry Torokhov]

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

Re: [PATCH 5/5] gpiolib: avoid krealloc() in gpiod_get_array()

[Andy Shevchenko]

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

Re: [PATCH 5/5] gpiolib: avoid krealloc() in gpiod_get_array()

[Dmitry Torokhov]

Hi Andy,

On Wed, Oct 21, 2026 at 10:34:01PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 21, 2026 at 12:02:10PM -0700, Dmitry Torokhov wrote:
> > +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.

struct gpio_array is defined with a flexible array member (invert_mask[])
specifically so that descs, array_info, and all three bitmaps live in a
single allocation. Switching to bitmap_zalloc() would turn one allocation
into four or five separate allocations and require freeing the bitmaps
both in gpiod_put_array() and when cancelling the fast path in
gpio_descs_check_fastpath(), so I would prefer to keep the single
contiguous allocation here.

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

Looking closer at gpiod_count(), all of its real backends
(of_gpio_count(), acpi_gpio_count(), swnode_gpio_count(), and
platform_gpio_count()) return either a positive count or -ENOENT, and
the !CONFIG_GPIO_ACPI stub for acpi_gpio_count() returns -ENODEV. The
only outlier is the !CONFIG_OF_GPIO stub for of_gpio_count() in
drivers/gpio/gpiolib-of.h, which returns 0 (and also causes
gpiod_count() to skip the if (count < 0) fallback to
platform_gpio_count()).

Do we actually need to check for count == 0 in gpiod_get_array(), or can
we fix the !CONFIG_OF_GPIO stub of of_gpio_count() to return -ENOENT and
leave the count < 0 check in gpiod_get_array() untouched?

> >  	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);
> >  
> > +		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);
> >  		}
> >  
> > +		gpio_descs_check_fastpath(descs, desc);
> >  
> > +		descs->desc[descs->ndescs++] = no_free_ptr(desc);
> >  	}

gpio_descs_alloc() needs the first descriptor (desc at index 0, acquired
by gpiod_get_index() inside the loop) to check gpiod_hwgpio(desc) and
gdev->ngpio so it knows whether to allocate space for array_info and the
bitmaps up front instead of reallocating with krealloc(). Before the
loop we have not looked up the first descriptor yet.

Thanks.

-- 
Dmitry

Re: [PATCH 5/5] gpiolib: avoid krealloc() in gpiod_get_array()

[Andy Shevchenko]

On Wed, Oct 21, 2026 at 01:39:49PM -0700, Dmitry Torokhov wrote:
> On Wed, Oct 21, 2026 at 10:34:01PM +0300, Andy Shevchenko wrote:
> > On Wed, Oct 21, 2026 at 12:02:10PM -0700, Dmitry Torokhov wrote:

...

> > > +	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.
> 
> struct gpio_array is defined with a flexible array member (invert_mask[])
> specifically so that descs, array_info, and all three bitmaps live in a
> single allocation. Switching to bitmap_zalloc() would turn one allocation
> into four or five separate allocations and require freeing the bitmaps
> both in gpiod_put_array() and when cancelling the fast path in
> gpio_descs_check_fastpath(), so I would prefer to keep the single
> contiguous allocation here.

Why? You can also allocate 3x bitmaps with bitmap_zalloc(). So I see only
a single additional allocation.

...

> > >  	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.
> 
> Looking closer at gpiod_count(), all of its real backends
> (of_gpio_count(), acpi_gpio_count(), swnode_gpio_count(), and
> platform_gpio_count()) return either a positive count or -ENOENT, and
> the !CONFIG_GPIO_ACPI stub for acpi_gpio_count() returns -ENODEV. The
> only outlier is the !CONFIG_OF_GPIO stub for of_gpio_count() in
> drivers/gpio/gpiolib-of.h, which returns 0 (and also causes
> gpiod_count() to skip the if (count < 0) fallback to
> platform_gpio_count()).
> 
> Do we actually need to check for count == 0 in gpiod_get_array(), or can
> we fix the !CONFIG_OF_GPIO stub of of_gpio_count() to return -ENOENT and
> leave the count < 0 check in gpiod_get_array() untouched?

I would fix of_gpio_count() stub. But I haven't check its users for correctness
of such a change. I think it would be weird for them to expect 0 when OF_GPIO=n.

...

> > >  	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);
> > >  
> > > +		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);
> > >  		}
> > >  
> > > +		gpio_descs_check_fastpath(descs, desc);
> > >  
> > > +		descs->desc[descs->ndescs++] = no_free_ptr(desc);
> > >  	}
> 
> gpio_descs_alloc() needs the first descriptor (desc at index 0, acquired
> by gpiod_get_index() inside the loop) to check gpiod_hwgpio(desc) and
> gdev->ngpio so it knows whether to allocate space for array_info and the
> bitmaps up front instead of reallocating with krealloc(). Before the
> loop we have not looked up the first descriptor yet.

That's why I put 'almost' which means we might need to duplicate a few calls
outside as well.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 5/5] gpiolib: avoid krealloc() in gpiod_get_array()

[Dmitry Torokhov]

On Wed, Oct 21, 2026 at 11:54:39PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 21, 2026 at 01:39:49PM -0700, Dmitry Torokhov wrote:
> > On Wed, Oct 21, 2026 at 10:34:01PM +0300, Andy Shevchenko wrote:
> > > On Wed, Oct 21, 2026 at 12:02:10PM -0700, Dmitry Torokhov wrote:
> 
> ...
> 
> > > > +	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.
> > 
> > struct gpio_array is defined with a flexible array member (invert_mask[])
> > specifically so that descs, array_info, and all three bitmaps live in a
> > single allocation. Switching to bitmap_zalloc() would turn one allocation
> > into four or five separate allocations and require freeing the bitmaps
> > both in gpiod_put_array() and when cancelling the fast path in
> > gpio_descs_check_fastpath(), so I would prefer to keep the single
> > contiguous allocation here.
> 
> Why? You can also allocate 3x bitmaps with bitmap_zalloc(). So I see only
> a single additional allocation.

So the proposal is to continue combining gpio_descs and gpio_array
allocation but split out bitmap allocations (still combining them into
one, so needing to round up nbits).

This looks like worst combination of all. Why do you prefer
bitmap_zalloc()?

> 
> ...
> 
> > > >  	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.
> > 
> > Looking closer at gpiod_count(), all of its real backends
> > (of_gpio_count(), acpi_gpio_count(), swnode_gpio_count(), and
> > platform_gpio_count()) return either a positive count or -ENOENT, and
> > the !CONFIG_GPIO_ACPI stub for acpi_gpio_count() returns -ENODEV. The
> > only outlier is the !CONFIG_OF_GPIO stub for of_gpio_count() in
> > drivers/gpio/gpiolib-of.h, which returns 0 (and also causes
> > gpiod_count() to skip the if (count < 0) fallback to
> > platform_gpio_count()).
> > 
> > Do we actually need to check for count == 0 in gpiod_get_array(), or can
> > we fix the !CONFIG_OF_GPIO stub of of_gpio_count() to return -ENOENT and
> > leave the count < 0 check in gpiod_get_array() untouched?
> 
> I would fix of_gpio_count() stub. But I haven't check its users for correctness
> of such a change. I think it would be weird for them to expect 0 when OF_GPIO=n.

OK, I can fix up the stub. The only user of of_gpio_count() is
gpiod_count().

> 
> ...
> 
> > > >  	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);
> > > >  
> > > > +		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);
> > > >  		}
> > > >  
> > > > +		gpio_descs_check_fastpath(descs, desc);
> > > >  
> > > > +		descs->desc[descs->ndescs++] = no_free_ptr(desc);
> > > >  	}
> > 
> > gpio_descs_alloc() needs the first descriptor (desc at index 0, acquired
> > by gpiod_get_index() inside the loop) to check gpiod_hwgpio(desc) and
> > gdev->ngpio so it knows whether to allocate space for array_info and the
> > bitmaps up front instead of reallocating with krealloc(). Before the
> > loop we have not looked up the first descriptor yet.
> 
> That's why I put 'almost' which means we might need to duplicate a few calls
> outside as well.

Yeah, I think doing it inside the loop rather than duplication
gpiod_get_index() outside makes it cleaner.

Thanks.

-- 
Dmitry

Re: [PATCH 5/5] gpiolib: avoid krealloc() in gpiod_get_array()

[Andy Shevchenko]

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

Re: [PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array()

[Andy Shevchenko]

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

Re: [PATCH 1/5] gpiolib: fix GPIO descriptor leak in gpiod_get_array()

[Dmitry Torokhov]

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