Re: [PATCH 2/2] gpiolib: acpi: fix out-of-bounds pointer arithmetic in acpi_gpio_package_count

[Marco Scardovi <scardracs@disroot.org>]

In data lunedì 1 giugno 2026 07:17:35 Ora legale dell’Europa centrale, Mika 
Westerberg ha scritto:
> On Sat, May 30, 2026 at 11:40:12AM +0200, Marco Scardovi wrote:
> > When counting GPIOs in an ACPI package, encountering a reference or
> > string causes the element pointer to be advanced by 3 (element += 3)
> > and then by 1 (element++).
> > 
> > If a malformed ACPI package contains fewer than 4 remaining elements
> > when a reference or string is processed, this pointer arithmetic
> > advances the element pointer past the end of the package elements
> > array. This results in undefined behavior and can cause out-of-bounds
> > reads.
> 
> How can it cause out-of-bounds reads? We increase "element" but the next
> iteration checks that it is still inside "end" and it's never dereferenced.
> Maybe I'm missing something?
>
Hi Mika,

I agree that `element` is not dereferenced after the loop exits.

My main concern is the parser logic rather than the pointer arithmetic
itself.

A GPIO connection is defined to consist of 4 package elements
(a reference/string followed by 3 integers), but the loop condition only
checks whether at least one element remains:

```
element < end
```

As a result, a malformed package containing fewer than 4 remaining elements
can still be processed as if it were a complete GPIO entry. This can lead
to a GPIO connection being accounted for even though the descriptor is
structurally incomplete.

Such truncated descriptors should be rejected with `-EPROTO` rather than
being accepted as valid input.

Ensuring sufficient remaining elements before entering the loop also
guarantees that pointer arithmetic stays within the defined bounds of the
package, but the primary issue is the acceptance of incomplete GPIO entries.
>
> > Fix this by ensuring at least 4 elements remain in the package before
> > advancing the element pointer, returning -EPROTO if the package
> > structure is invalid.
> > 
> > Signed-off-by: Marco Scardovi <scardracs@disroot.org>
> > ---
> > 
> >  drivers/gpio/gpiolib-acpi-core.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpio/gpiolib-acpi-core.c
> > b/drivers/gpio/gpiolib-acpi-core.c index 049e4cbc14ed..494dcd166aef
> > 100644
> > --- a/drivers/gpio/gpiolib-acpi-core.c
> > +++ b/drivers/gpio/gpiolib-acpi-core.c
> > @@ -1310,6 +1310,8 @@ static int acpi_gpio_package_count(const union
> > acpi_object *obj)> 
> >  		switch (element->type) {
> >  		case ACPI_TYPE_LOCAL_REFERENCE:
> > 
> >  		case ACPI_TYPE_STRING:
> > +			if (end - element < 4)
> > +				return -EPROTO;
> > 
> >  			element += 3;
> >  			fallthrough;
> >  		
> >  		case ACPI_TYPE_INTEGER: