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

[PATCH v1 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
address 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 values.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi-core.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
index 83dd227dbbec..da0ab749b4dc 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 vala, valo;
 		bool found;
 
 		mutex_lock(&achip->conn_lock);
@@ -1158,10 +1159,17 @@ 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(i)));
-		else
-			*value |= (u64)gpiod_get_raw_value_cansleep(desc) << i;
+		vala = i / 64;
+		valo = i % 64;
+
+		if (function == ACPI_WRITE) {
+			gpiod_set_raw_value_cansleep(desc, value[vala] & BIT_ULL(valo));
+		} else {
+			if (gpiod_get_raw_value_cansleep(desc))
+				value[vala] |= BIT_ULL(valo);
+			else
+				value[vala] &= ~BIT_ULL(valo);
+		}
 	}
 
 out:
-- 
2.50.1

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

[Mika Westerberg]

On Mon, Jan 26, 2026 at 02:42:26PM +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
> address for the value, and anything bigger than	63 will be also
                                                 ^^^^^^^
There is some extra whitespace above.

> 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 values.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/gpiolib-acpi-core.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
> index 83dd227dbbec..da0ab749b4dc 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 vala, valo;

Perhaps bit more descriptive names.

And while at it, add comment why this is needed.

>  		bool found;
>  
>  		mutex_lock(&achip->conn_lock);
> @@ -1158,10 +1159,17 @@ 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(i)));
> -		else
> -			*value |= (u64)gpiod_get_raw_value_cansleep(desc) << i;
> +		vala = i / 64;
> +		valo = i % 64;
> +
> +		if (function == ACPI_WRITE) {
> +			gpiod_set_raw_value_cansleep(desc, value[vala] & BIT_ULL(valo));
> +		} else {
> +			if (gpiod_get_raw_value_cansleep(desc))
> +				value[vala] |= BIT_ULL(valo);
> +			else
> +				value[vala] &= ~BIT_ULL(valo);
> +		}
>  	}
>  
>  out:
> -- 
> 2.50.1