Re: [PATCH v2 04/11] pinctrl: sunxi: add Allwinner B288 pin controller driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pinctrl: out-of-bounds access and corrupt IRQ mapping due to shifted bank index
- [High] pinctrl: NULL pointer dereference on probe deferral due to global state leak
--

commit c211ae9972d97b1239a013dc4594d43764f7806c
Author: Pablo Mazzini <pmazzini@gmail.com>

    pinctrl: sunxi: add Allwinner B288 pin controller driver

This commit adds pin controller and GPIO support for the Allwinner B288
SoC. It defines 80 pins across ports B, C, D, F, and G, omitting port A
completely.

> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun8i-b288.c b/drivers/pinctrl/sunxi/pinctrl-sun8i-b288.c
> new file mode 100644
> index 0000000000000..98dd4ead4c328
> --- /dev/null
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun8i-b288.c
> @@ -0,0 +1,53 @@

[ ... ]

> +/* PH0 exists only on the BSP's FPGA platform and is omitted. */
> +static const u8 b288_nr_bank_pins[SUNXI_PINCTRL_MAX_BANKS] =
> +/*	  PA  PB  PC  PD  PE  PF  PG */
> +	{  0, 17, 15, 29,  0,  7, 12 };
> +
> +/* Port C has no external interrupts. */
> +static const unsigned int b288_irq_bank_map[] = { 1, 3, 5, 6 };
> +
> +static const u8 b288_irq_bank_muxes[SUNXI_PINCTRL_MAX_BANKS] =
> +/*	  PA  PB  PC  PD  PE  PF  PG */
> +	{  0,  6,  0,  6,  0,  6,  6 };

[Severity: High]
Does this pin layout trigger an out-of-bounds access and corrupt the IRQ
mapping?

Because bank PA has 0 pins, PB (bank 1) is the first bank processed. In
drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c:prepare_function_table(),
last_bank is initialized to 0. When PB is processed, bank > last_bank
(1 > 0) evaluates to true, which erroneously increments irq_bank to 1 for
the first bank instead of 0:

			if (bank > last_bank)
				irq_bank++;

This shifts all logical IRQ banks so PB gets irqbank=1, PD=2, PF=3, and
PG=4. Since b288_irq_bank_map only has 4 elements, processing PG (irqbank=4)
causes sunxi_irq_hw_bank_num() to read out of bounds.

Further, sunxi_pinctrl_build_state() will write out of bounds past the
end of pctl->irq_array:

	int irqnum = func->irqnum + func->irqbank * IRQ_PER_BANK;
	pctl->irq_array[irqnum] = pin->pin.number;

Since func->irqbank evaluates to 4 (which is >= desc->irq_banks), this
causes boot-time memory corruption and breaks IRQ routing for all pins.

> +static struct sunxi_pinctrl_desc b288_pinctrl_data = {
> +	.irq_banks	= ARRAY_SIZE(b288_irq_bank_map),
> +	.irq_bank_map	= b288_irq_bank_map,
> +};
> +
> +static int b288_pinctrl_probe(struct platform_device *pdev)
> +{
> +	return sunxi_pinctrl_dt_table_init(pdev, b288_nr_bank_pins,
> +					   b288_irq_bank_muxes,
> +					   &b288_pinctrl_data, 0);
> +}

[Severity: High]
Does passing this static global struct to the framework leak state on
probe deferral, leading to a NULL pointer dereference?

The b288_pinctrl_data struct is a static global passed to
sunxi_pinctrl_dt_table_init(). Inside init_pins_table(), desc->npins is
unconditionally incremented into the global state:

	desc->npins += pins_per_bank[i];

If sunxi_pinctrl_init_with_flags() subsequently defers probe (e.g., waiting
for a clock or PMIC), desc->npins permanently retains the 80 count. On the
next probe attempt, desc->npins becomes 160.

This causes devm_kcalloc() to allocate 160 descriptors, but only the first 80
are properly initialized with names. The trailing 80 elements retain NULL
pin.name pointers, which causes a crash during re-probe in
prepare_function_table() when iterating over the uninitialized elements:

				if (strcmp(pins[i].pin.name, name))
					continue;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927151016.186493-1-pmazzini@gmail.com?part=4