[PATCH v2 1/1] gpiolib: acpi: Fix potential out-of-boundary left shift

[PATCH v2 1/1] gpiolib: acpi: Fix potential out-of-boundary left shift

[Andy Shevchenko]

GPIO Address Space handler gets a pointer to the in or out value.
This value is supposed to be at least 64-bit, but it's not limited
to be exactly 64-bit. When ACPI tables are being parsed, for
the bigger Connection():s ACPICA creates a Buffer instead of regular
Integer object. The Buffer exists as long as Namespace holds
the certain Connection(). Hence we can access the necessary bits
without worrying. On the other hand, the left shift, used in
the code, is limited by 31 (on 32-bit platforms) and otherwise
considered to be Undefined Behaviour. Also the code uses only
the first 64-bit word for the value, and anything bigger than 63
will be also subject to UB. Fix all this by modifying the code
to correctly set or clear the respective bit in the bitmap constructed
of 64-bit words.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: added a comment, named variables differently (Mika), rebased on top of accepted patch
 drivers/gpio/gpiolib-acpi-core.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
index d42f769eeb11..9627b3a9c7f3 100644
--- a/drivers/gpio/gpiolib-acpi-core.c
+++ b/drivers/gpio/gpiolib-acpi-core.c
@@ -1104,6 +1104,7 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 		unsigned int pin = agpio->pin_table[i];
 		struct acpi_gpio_connection *conn;
 		struct gpio_desc *desc;
+		u16 word, shift;
 		bool found;
 
 		mutex_lock(&achip->conn_lock);
@@ -1158,10 +1159,22 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 
 		mutex_unlock(&achip->conn_lock);
 
-		if (function == ACPI_WRITE)
-			gpiod_set_raw_value_cansleep(desc, !!(*value & BIT_ULL(i)));
-		else
-			*value |= (u64)gpiod_get_raw_value_cansleep(desc) << i;
+		/*
+		 * For the cases when OperationRegion() consists of more than
+		 * 64 bits calculate the word and bit shift to use that one to
+		 * access the value.
+		 */
+		word = i / 64;
+		shift = i % 64;
+
+		if (function == ACPI_WRITE) {
+			gpiod_set_raw_value_cansleep(desc, value[word] & BIT_ULL(shift));
+		} else {
+			if (gpiod_get_raw_value_cansleep(desc))
+				value[word] |= BIT_ULL(shift);
+			else
+				value[word] &= ~BIT_ULL(shift);
+		}
 	}
 
 out:
-- 
2.50.1

Re: [PATCH v2 1/1] gpiolib: acpi: Fix potential out-of-boundary left shift

[Mika Westerberg]

On Tue, Jan 27, 2026 at 12:06:30PM +0100, Andy Shevchenko wrote:
> GPIO Address Space handler gets a pointer to the in or out value.
> This value is supposed to be at least 64-bit, but it's not limited
> to be exactly 64-bit. When ACPI tables are being parsed, for
> the bigger Connection():s ACPICA creates a Buffer instead of regular
> Integer object. The Buffer exists as long as Namespace holds
> the certain Connection(). Hence we can access the necessary bits
> without worrying. On the other hand, the left shift, used in
> the code, is limited by 31 (on 32-bit platforms) and otherwise
> considered to be Undefined Behaviour. Also the code uses only
> the first 64-bit word for the value, and anything bigger than 63
> will be also subject to UB. Fix all this by modifying the code
> to correctly set or clear the respective bit in the bitmap constructed
> of 64-bit words.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Perfect, thanks!

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH v2 1/1] gpiolib: acpi: Fix potential out-of-boundary left shift

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 02:43:00PM +0100, Mika Westerberg wrote:
> On Tue, Jan 27, 2026 at 12:06:30PM +0100, Andy Shevchenko wrote:
> > GPIO Address Space handler gets a pointer to the in or out value.
> > This value is supposed to be at least 64-bit, but it's not limited
> > to be exactly 64-bit. When ACPI tables are being parsed, for
> > the bigger Connection():s ACPICA creates a Buffer instead of regular
> > Integer object. The Buffer exists as long as Namespace holds
> > the certain Connection(). Hence we can access the necessary bits
> > without worrying. On the other hand, the left shift, used in
> > the code, is limited by 31 (on 32-bit platforms) and otherwise
> > considered to be Undefined Behaviour. Also the code uses only
> > the first 64-bit word for the value, and anything bigger than 63
> > will be also subject to UB. Fix all this by modifying the code
> > to correctly set or clear the respective bit in the bitmap constructed
> > of 64-bit words.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Perfect, thanks!

> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thank you!

Bart, since you picked up the patch in the same lines of code, feel free to
take this one on top (I think it's your gpio/for-current branch).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/1] gpiolib: acpi: Fix potential out-of-boundary left shift

[Bartosz Golaszewski]

On Tue, 27 Jan 2026 12:06:30 +0100, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> said:
> GPIO Address Space handler gets a pointer to the in or out value.
> This value is supposed to be at least 64-bit, but it's not limited
> to be exactly 64-bit. When ACPI tables are being parsed, for
> the bigger Connection():s ACPICA creates a Buffer instead of regular
> Integer object. The Buffer exists as long as Namespace holds
> the certain Connection(). Hence we can access the necessary bits
> without worrying. On the other hand, the left shift, used in
> the code, is limited by 31 (on 32-bit platforms) and otherwise
> considered to be Undefined Behaviour. Also the code uses only
> the first 64-bit word for the value, and anything bigger than 63
> will be also subject to UB. Fix all this by modifying the code
> to correctly set or clear the respective bit in the bitmap constructed
> of 64-bit words.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: added a comment, named variables differently (Mika), rebased on top of accepted patch

Should I take it for v6.19? Fixes:? Cc: stable?

Bart

Re: [PATCH v2 1/1] gpiolib: acpi: Fix potential out-of-boundary left shift

[Bartosz Golaszewski]

On Tue, 27 Jan 2026 14:49:51 +0100, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> said:
> On Tue, Jan 27, 2026 at 02:43:00PM +0100, Mika Westerberg wrote:
>> On Tue, Jan 27, 2026 at 12:06:30PM +0100, Andy Shevchenko wrote:
>> > GPIO Address Space handler gets a pointer to the in or out value.
>> > This value is supposed to be at least 64-bit, but it's not limited
>> > to be exactly 64-bit. When ACPI tables are being parsed, for
>> > the bigger Connection():s ACPICA creates a Buffer instead of regular
>> > Integer object. The Buffer exists as long as Namespace holds
>> > the certain Connection(). Hence we can access the necessary bits
>> > without worrying. On the other hand, the left shift, used in
>> > the code, is limited by 31 (on 32-bit platforms) and otherwise
>> > considered to be Undefined Behaviour. Also the code uses only
>> > the first 64-bit word for the value, and anything bigger than 63
>> > will be also subject to UB. Fix all this by modifying the code
>> > to correctly set or clear the respective bit in the bitmap constructed
>> > of 64-bit words.
>> >
>> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>
>> Perfect, thanks!
>
>> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> Thank you!
>
> Bart, since you picked up the patch in the same lines of code, feel free to
> take this one on top (I think it's your gpio/for-current branch).
>

Don't you want a Fixes: and Cc: stable tags?

Bart

Re: [PATCH v2 1/1] gpiolib: acpi: Fix potential out-of-boundary left shift

[Andy Shevchenko]

On Wed, Jan 28, 2026 at 01:02:11AM -0800, Bartosz Golaszewski wrote:
> On Tue, 27 Jan 2026 14:49:51 +0100, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> said:
> > On Tue, Jan 27, 2026 at 02:43:00PM +0100, Mika Westerberg wrote:
> >> On Tue, Jan 27, 2026 at 12:06:30PM +0100, Andy Shevchenko wrote:
> >> > GPIO Address Space handler gets a pointer to the in or out value.
> >> > This value is supposed to be at least 64-bit, but it's not limited
> >> > to be exactly 64-bit. When ACPI tables are being parsed, for
> >> > the bigger Connection():s ACPICA creates a Buffer instead of regular
> >> > Integer object. The Buffer exists as long as Namespace holds
> >> > the certain Connection(). Hence we can access the necessary bits
> >> > without worrying. On the other hand, the left shift, used in
> >> > the code, is limited by 31 (on 32-bit platforms) and otherwise
> >> > considered to be Undefined Behaviour. Also the code uses only
> >> > the first 64-bit word for the value, and anything bigger than 63
> >> > will be also subject to UB. Fix all this by modifying the code
> >> > to correctly set or clear the respective bit in the bitmap constructed
> >> > of 64-bit words.
> >> >
> >> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >>
> >> Perfect, thanks!
> >
> >> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >
> > Thank you!
> >
> > Bart, since you picked up the patch in the same lines of code, feel free to
> > take this one on top (I think it's your gpio/for-current branch).
> 
> Don't you want a Fixes: and Cc: stable tags?

Okay, I will add them in v3, thanks!

-- 
With Best Regards,
Andy Shevchenko