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

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

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

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

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