Re: [PATCH v3 2/2] mux: gpio: Add optional enable gpio

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds stack read in `mux_gpio_set` when the number of GPIOs specified in the device tree exceeds the bit width of `unsigned long`.
--

commit e5d22f785ccbb077940b83f814cc7d4237449e05
Author: Tapio Reijonen <tapio.reijonen@vaisala.com>

mux: gpio: Add optional enable gpio

Some analog multiplexers have an enable input that disconnects all
channels when deasserted, independent of the address inputs. This patch
adds support for an optional enable gpio to handle this. It also updates
the mux idle state logic to require this enable gpio when disconnecting.

> diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
> index f9c7863e51b82..2fd8c83dc22e1 100644
> --- a/drivers/mux/gpio.c
> +++ b/drivers/mux/gpio.c

[ ... ]

> @@ -25,12 +26,26 @@ static int mux_gpio_set(struct mux_control *mux, int state)
>  	struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
>  	DECLARE_BITMAP(values, BITS_PER_TYPE(state));
>  	u32 value = state;
> +	int ret;
> +
> +	/*
> +	 * The gpios might not be updated atomically, disable the mux
> +	 * meanwhile.
> +	 */
> +	ret = gpiod_set_value_cansleep(mux_gpio->enable, 0);
> +	if (ret)
> +		return ret;
> +
> +	if (state == MUX_IDLE_DISCONNECT)
> +		return 0;
>  
>  	bitmap_from_arr32(values, &value, BITS_PER_TYPE(value));
>  
> -	gpiod_multi_set_value_cansleep(mux_gpio->gpios, values);
> +	ret = gpiod_multi_set_value_cansleep(mux_gpio->gpios, values);

[Severity: High]
This is a pre-existing issue, but is it possible for an out-of-bounds stack
read to occur here?

In mux_gpio_probe(), the number of pins is parsed from the device tree
using gpiod_count() which sets ndescs. There doesn't appear to be an upper
limit check on this value.

The values bitmap allocated earlier in mux_gpio_set():

    DECLARE_BITMAP(values, BITS_PER_TYPE(state));

limits the capacity to a single unsigned long. If the number of GPIOs
specified in the device tree exceeds the bit width of state, it looks like
gpiod_multi_set_value_cansleep() will iterate past the bounds of the
allocated stack array when reading from values.

Could this lead to leaking adjacent kernel stack memory or triggering a
KASAN panic if an oversized number of pins is provided?

> +	if (ret)
> +		return ret;
>  
> -	return 0;
> +	return gpiod_set_value_cansleep(mux_gpio->enable, 1);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-add-external-mux-enable-gpio-v3-0-1b868e669e43@vaisala.com?part=2