Re: [PATCH 01/10] pinctrl: sunxi: Fix out-of-bounds irq_bank index when first bank has no IRQs

[Andre Przywara <andre.przywara@arm.com>]

Hi Lee,

On 11/3/26 22:29, Lee Jones wrote:
> In prepare_function_table(), irq_bank and last_bank are initialised to 0
> and last_bank is updated on every bank regardless of whether it supports
> interrupts.  On SoCs where bank 0 (PA) has no pins or does not support
> interrupts (such as Allwinner A523 and A733), the first IRQ-capable bank
> has bank > 0, which causes irq_bank to be incremented to 1 instead of
> starting at 0.  As a result, the final IRQ bank receives irq_bank equal
> to desc->irq_banks and writes out-of-bounds past pctl->irq_array during
> sunxi_pinctrl_build_state().
> 
> Initialise irq_bank and last_bank to -1 and only update last_bank when
> processing an IRQ-capable bank so that irq_bank always starts at 0 and
> increments sequentially across IRQ-capable banks.

Can we please hold back this patch for now? There are more issues around 
the IRQ bank usage on recent Allwinner SoCs, patches for which [1] are 
on the list. They should solve the out-of-bounds access, I think, though 
we might want to add explicit checks, maybe.

But taking just this patch here would torpedo the efforts in fixing the 
correctness issues that we see.

Cheers,
Andre

[1]
https://lore.kernel.org/linux-gpio/20261025142412.112216-1-pmazzini@gmail.com/

> 
> Fixes: f5e2cd34b12f ("pinctrl: sunxi: allow reading mux values from DT")
> Closes: https://sashiko.dev/#/bug/linux-9c4dd832-02d4-4096-9e91-a03c1d4d85b3
> Cc: stable@vger.kernel.org
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
>   drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c b/drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c
> index 8bcd566593ae..79ca1a7c4837 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c
> @@ -208,8 +208,8 @@ static int prepare_function_table(struct device *dev, struct device_node *pnode,
>   	 * Assign the function's memory and fill in GPIOs, IRQ and a sentinel.
>   	 * The extra functions will be filled in later.
>   	 */
> -	irq_bank = 0;
> -	last_bank = 0;
> +	irq_bank = -1;
> +	last_bank = -1;
>   	for (i = 0; i < npins; i++) {
>   		struct sunxi_desc_pin *pin = &pins[i];
>   		int bank = (pin->pin.number - pin_base) / PINS_PER_BANK;
> @@ -222,17 +222,16 @@ static int prepare_function_table(struct device *dev, struct device_node *pnode,
>   		func[1].muxval = 1;
>   
>   		if (irq_mux) {
> -			if (bank > last_bank)
> +			if (bank > last_bank) {
>   				irq_bank++;
> +				last_bank = bank;
> +			}
>   			func[lastfunc].muxval = irq_mux;
>   			func[lastfunc].irqbank = irq_bank;
>   			func[lastfunc].irqnum = pin->pin.number % PINS_PER_BANK;
>   			func[lastfunc].name = "irq";
>   		}
>   
> -		if (bank > last_bank)
> -			last_bank = bank;
> -
>   		pin->functions = func;
>   
>   		/* Skip over the other needed functions and the sentinel. */