[PATCH v5 0/2] gpiolib: acpi: Add bounds-checking and address validation

[PATCH v5 0/2] gpiolib: acpi: Add bounds-checking and address validation

[Marco Scardovi]

Hi all,

after some thinking and following Mika and Andy's reviews I've come up with
a new version of the series.

This series adds defensive hardening in the ACPI GPIO path, focusing on
bounds checking and safe handling of GPIO resources and ACPI addresses.

Drop the previously proposed GPIO connection handling changes in the ACPI
OperationRegion path, following review feedback.

This also closes the old threads completely and drop my @gmail account
as it is not related to my linux stuffs.

Changes in v5:

* Add bounds checking for GPIO pin resource accesses in ACPI GPIO helpers
  to prevent out-of-bounds access on malformed or empty pin tables.
* Validate ACPI address in acpi_gpio_adr_space_handler() before converting
  it to a GPIO index, preventing truncation-induced wraparound.
* Improve type safety and overflow handling in length calculation and loop
  bounds in the OperationRegion handler.

Changes in v4:

* Dropped connection resource leak fix to keep focus on validation.
* See v4 series at:
  https://lore.kernel.org/linux-gpio/20260531120816.17255-1-scardracs@disroot.org/

Changes in v3:

* Fixed global over-cleanup bug in OpRegion error path.
* See v3 series at:
  https://lore.kernel.org/linux-gpio/20260520074955.55443-1-mscardovi95@gmail.com/

Changes in v2:

* Dropped gpiolib-acpi-core.c modularization (out of scope).
* Split series into bounds checking and address truncation fixes.
* See v2 series at:
  https://lore.kernel.org/linux-gpio/20260519070039.9280-1-mscardovi95@gmail.com/

Changes in v1:

* Initial ACPI GPIO hardening series.

Marco Scardovi (2):
gpiolib: acpi: Add robust bounds-checking for GPIO pin resources
gpiolib: acpi: prevent address truncation in OperationRegion handler

drivers/gpio/gpiolib-acpi-core.c | 40 +++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 8 deletions(-)

--
2.54.0

[PATCH v5 1/2] gpiolib: acpi: Add robust bounds-checking for GPIO pin resources

[Marco Scardovi]

Ensure that the GPIO pin resource arrays are safely bounded before
accessing indices. Add bounds checking in acpi_request_own_gpiod(),
acpi_gpio_irq_is_wake(), and acpi_gpiochip_alloc_event() to prevent
out-of-bounds array reads if the ACPI namespace provides malformed or
empty pin tables.

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
 drivers/gpio/gpiolib-acpi-core.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
index eb8a40cfb7a9..1cb5f5884ff0 100644
--- a/drivers/gpio/gpiolib-acpi-core.c
+++ b/drivers/gpio/gpiolib-acpi-core.c
@@ -320,10 +320,17 @@ static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
 						unsigned int index,
 						const char *label)
 {
-	int polarity = GPIO_ACTIVE_HIGH;
-	enum gpiod_flags flags = acpi_gpio_to_gpiod_flags(agpio, polarity);
-	unsigned int pin = agpio->pin_table[index];
+	enum gpiod_flags flags;
 	struct gpio_desc *desc;
+	unsigned int pin;
+	int polarity;
+
+	if (index >= agpio->pin_table_length)
+		return ERR_PTR(-EINVAL);
+
+	pin = agpio->pin_table[index];
+	polarity = GPIO_ACTIVE_HIGH;
+	flags = acpi_gpio_to_gpiod_flags(agpio, polarity);
 
 	desc = gpiochip_request_own_desc(chip, pin, label, polarity, flags);
 	if (IS_ERR(desc))
@@ -337,7 +344,12 @@ static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
 static bool acpi_gpio_irq_is_wake(struct device *parent,
 				  const struct acpi_resource_gpio *agpio)
 {
-	unsigned int pin = agpio->pin_table[0];
+	unsigned int pin;
+
+	if (agpio->pin_table_length == 0)
+		return false;
+
+	pin = agpio->pin_table[0];
 
 	if (agpio->wake_capable != ACPI_WAKE_CAPABLE)
 		return false;
@@ -367,6 +379,9 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
 	if (!acpi_gpio_get_irq_resource(ares, &agpio))
 		return AE_OK;
 
+	if (agpio->pin_table_length == 0)
+		return AE_OK;
+
 	handle = ACPI_HANDLE(chip->parent);
 	pin = agpio->pin_table[0];
 
-- 
2.54.0

[PATCH v5] gpiolib: acpi: prevent address truncation in OperationRegion handler

[Marco Scardovi]

The ACPI address space handler for GPIO OperationRegions receives the
pin offset as a 64-bit acpi_physical_address. However, the handler
truncates this address to a u16 pin_index before validating it.

If an ACPI table attempts to access a pin offset greater than 65535,
the truncation wraps the index around. This may result in accesses to
unintended GPIO pins.

Fix this by adding an explicit check to verify that the 64-bit address
is less than agpio->pin_table_length before assigning it to the u16
pin_index, returning AE_BAD_PARAMETER if it is out of bounds.
Additionally, make the length calculation overflow-safe and change the types
of length and loop counter to unsigned.

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
 drivers/gpio/gpiolib-acpi-core.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
index 1cb5f5884ff0..fc157ee9ac61 100644
--- a/drivers/gpio/gpiolib-acpi-core.c
+++ b/drivers/gpio/gpiolib-acpi-core.c
@@ -1102,10 +1102,10 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 	struct gpio_chip *chip = achip->chip;
 	struct acpi_resource_gpio *agpio;
 	struct acpi_resource *ares;
-	u16 pin_index = address;
+	unsigned int length;
 	acpi_status status;
-	int length;
-	int i;
+	unsigned int i;
+	u16 pin_index;
 
 	status = acpi_buffer_to_resource(achip->conn_info.connection,
 					 achip->conn_info.length, &ares);
@@ -1125,7 +1125,16 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 		return AE_BAD_PARAMETER;
 	}
 
-	length = min(agpio->pin_table_length, pin_index + bits);
+	if (address >= agpio->pin_table_length) {
+		ACPI_FREE(ares);
+		return AE_BAD_PARAMETER;
+	}
+
+	pin_index = address;
+	if (bits > agpio->pin_table_length - pin_index)
+		length = agpio->pin_table_length;
+	else
+		length = pin_index + bits;
 	for (i = pin_index; i < length; ++i) {
 		unsigned int pin = agpio->pin_table[i];
 		struct acpi_gpio_connection *conn;
-- 
2.54.0

Re: [PATCH v5] gpiolib: acpi: prevent address truncation in OperationRegion handler

[Mika Westerberg]

On Tue, Jun 02, 2026 at 01:32:20PM +0200, Marco Scardovi wrote:
> The ACPI address space handler for GPIO OperationRegions receives the
> pin offset as a 64-bit acpi_physical_address. However, the handler
> truncates this address to a u16 pin_index before validating it.
> 
> If an ACPI table attempts to access a pin offset greater than 65535,
> the truncation wraps the index around. This may result in accesses to
> unintended GPIO pins.

How in practice this can be done given that the GPIO resource has only 2
bytes for the index?

> Fix this by adding an explicit check to verify that the 64-bit address
> is less than agpio->pin_table_length before assigning it to the u16
> pin_index, returning AE_BAD_PARAMETER if it is out of bounds.
> Additionally, make the length calculation overflow-safe and change the types
> of length and loop counter to unsigned.
> 
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Marco Scardovi <scardracs@disroot.org>
> ---
>  drivers/gpio/gpiolib-acpi-core.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
> index 1cb5f5884ff0..fc157ee9ac61 100644
> --- a/drivers/gpio/gpiolib-acpi-core.c
> +++ b/drivers/gpio/gpiolib-acpi-core.c
> @@ -1102,10 +1102,10 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
>  	struct gpio_chip *chip = achip->chip;
>  	struct acpi_resource_gpio *agpio;
>  	struct acpi_resource *ares;
> -	u16 pin_index = address;
> +	unsigned int length;
>  	acpi_status status;
> -	int length;
> -	int i;
> +	unsigned int i;
> +	u16 pin_index;
>  
>  	status = acpi_buffer_to_resource(achip->conn_info.connection,
>  					 achip->conn_info.length, &ares);
> @@ -1125,7 +1125,16 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
>  		return AE_BAD_PARAMETER;
>  	}
>  
> -	length = min(agpio->pin_table_length, pin_index + bits);
> +	if (address >= agpio->pin_table_length) {
> +		ACPI_FREE(ares);
> +		return AE_BAD_PARAMETER;
> +	}
> +
> +	pin_index = address;
> +	if (bits > agpio->pin_table_length - pin_index)
> +		length = agpio->pin_table_length;
> +	else
> +		length = pin_index + bits;
>  	for (i = pin_index; i < length; ++i) {
>  		unsigned int pin = agpio->pin_table[i];
>  		struct acpi_gpio_connection *conn;
> -- 
> 2.54.0

Re: [PATCH v5] gpiolib: acpi: prevent address truncation in OperationRegion handler

[Marco Scardovi]

Hi Mika,

On Tue, Jun 02, 2026 at 01:45:40PM +0200, Mika Westerberg wrote:
> How in practice this can be done given that the GPIO resource has only 2
> bytes for the index?

The 2-byte limitation is in the GPIO resource descriptor representation of the 
pin table, not in the ACPI address space handler interface itself.

The acpi_gpio_adr_space_handler() receives the access offset as a 64-bit 
acpi_physical_address from ACPICA. This value is generated when AML
accesses a Field within the GPIO OperationRegion, and it is not constrained
by the GPIO resource descriptor's pin_table_length.

This is not GPIO-specific in ACPICA terms: all address space handlers
receive a raw 64-bit address, and any semantic interpretation (such as
treating it as a GPIO pin index) is done by the individual handler.

In the GPIO case, the driver maps this address directly to an index into
agpio->pin_table[]. Without validating the full 64-bit value against
pin_table_length before truncating to u16, an out-of-bounds access can
occur due to wraparound.

The fix ensures the 64-bit address is validated against the table size
before any narrowing conversion, avoiding the wraparound and
rejecting invalid AML accesses.

Marco

Re: [PATCH v5] gpiolib: acpi: prevent address truncation in OperationRegion handler

[Mika Westerberg]

Hi,

On Tue, Jun 02, 2026 at 01:59:36PM +0200, Marco Scardovi wrote:
> Hi Mika,
> 
> On Tue, Jun 02, 2026 at 01:45:40PM +0200, Mika Westerberg wrote:
> > How in practice this can be done given that the GPIO resource has only 2
> > bytes for the index?
> 
> The 2-byte limitation is in the GPIO resource descriptor representation of the 
> pin table, not in the ACPI address space handler interface itself.
> 
> The acpi_gpio_adr_space_handler() receives the access offset as a 64-bit 
> acpi_physical_address from ACPICA. This value is generated when AML
> accesses a Field within the GPIO OperationRegion, and it is not constrained
> by the GPIO resource descriptor's pin_table_length.

Yes, but you access it from AML like this:

Field(\_SB.GPI2.GPO2, ByteAcc, NoLock, Preserve)
{
	Connection (GpioIo(Exclusive, PullUp, , , , "\\_SB.GPI2") {7}),
	STAT, 1,           // e.g. Status signal from the device
	Connection (GpioIo (Exclusive, PullUp, , , , "\\_SB.GPI2") {9}),
	RSET, 1            // e.g. Reset signal to the device
}

In other words it still uses GPIO resource descriptor (so 2 bytes per pin),
nothing else is accepted as far as I understand.

The example is here: 

https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#declaring-generalpurposeio-fields

Re: [PATCH v5] gpiolib: acpi: prevent address truncation in OperationRegion handler

[Marco Scardovi]

Hi Mika,

Thanks for the clarification.

After looking again at the ACPI specification, I agree that AML accesses
GPIO fields through Connection() resources, so the offset used by the
handler is derived from the GPIO resource descriptor itself.

Therefore, describing this as a truncation issue is not accurate. The
actual issue is that the address argument is used as an index into
agpio->pin_table without first validating that it is within
agpio->pin_table_length.

I'll update the description accordingly and focus on the missing bounds
check rather than truncation or wrap-around.

If You or anyone else haven't any more suggestions/reviews I'll post it
later today/tomorrow as a v6.

Thanks,
Marco