Re: [PATCH 1/2] gpiolib: acpi: prevent address truncation in OperationRegion handler

[Marco Scardovi]

In data lunedì 1 giugno 2026 07:02:38 Ora legale dell’Europa centrale, Mika 
Westerberg ha scritto:
> Hi,
> 
> On Sat, May 30, 2026 at 11:40:11AM +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.
> 
> If you look at the ACPI spec:
> 
> https://uefi.org/specs/ACPI/6.5/06_Device_Configuration.html#connection-desc
> riptors
> 
> the pin number is 2 bytes and 0xffff is defined as no connection. So the
> firmware cannot really think that it can access GPIO outside of that range.
> 
Hi Mika,

You are right that GPIO connection descriptors define pin indices as 16-bit
values and that 0xffff is reserved as "no connection".

My concern is that `address` is provided by ACPICA as a 64-bit
`acpi_physical_address`, while the GPIO driver immediately narrows it:

```
u16 pin_index = address;
```

At this point, the value is converted to the target type before any
validation against the GPIO resource is performed in the driver.

The intention of the change is to ensure that the GPIO layer validates the
value against its 16-bit domain before narrowing it, so that validation is
performed on the original value received from ACPICA rather than on a
truncated representation.

This keeps the type boundary explicit at the GPIO driver level, where the
semantic meaning of the value (GPIO pin index) is actually enforced.
>
> > 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.
> > 
> > Signed-off-by: Marco Scardovi <scardracs@xxxxxxxxxxx>
> > ---
> > 
> >  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 eb8a40cfb7a9..049e4cbc14ed
> > 100644
> > --- a/drivers/gpio/gpiolib-acpi-core.c
> > +++ b/drivers/gpio/gpiolib-acpi-core.c
> > @@ -1087,10 +1087,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);
> > 
> > @@ -1110,7 +1110,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;