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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] pinctrl: sunxi: out-of-bounds write in irq_array when Bank 0 has 0 pins
- [High] pinctrl: sunxi: Missing irq_bank_map breaks B288 IRQ offsets and domain size
- [High] pinctrl: sunxi: probe deferral crash due to npins accumulation
--

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

pinctrl: sunxi: add Allwinner B288 pin controller driver

This commit adds a new pin controller driver for the Allwinner B288 SoC,
supporting 80 pins across ports B, C, D, F, and G. It uses DT-provided
mux values and specifies 4 banks of external interrupts.

> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun8i-b288.c b/drivers/pinctrl/sunxi/pinctrl-sun8i-b288.c
> new file mode 100644
> index 0000000000000..f973c2b3f2eba
> --- /dev/null
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun8i-b288.c
> @@ -0,0 +1,49 @@
[ ... ]
> +/* 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 };

[Severity: High]
Since Bank 0 (PA) has 0 pins, does this cause an out-of-bounds write in
the core sunxi pinctrl driver?

In drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c:prepare_function_table(),
last_bank and irq_bank are initialized to 0. When processing PB (bank 1),
the condition (bank > last_bank) evaluates to true (1 > 0), causing irq_bank
to erroneously increment to 1.

This shifts the logical IRQ bank indices for PB, PD, PF, and PG to
1, 2, 3, and 4 instead of 0, 1, 2, and 3.

Later, sunxi_pinctrl_build_state() configures the interrupts:

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

For PG (irqbank 4), irqnum will be between 128 and 139. However, irq_array
is allocated for exactly 4 banks (128 elements) in
sunxi_pinctrl_init_with_flags(), which would result in an out-of-bounds
write past the end of the array.

> +
> +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 };
> +
> +static struct sunxi_pinctrl_desc b288_pinctrl_data = {
> +	.irq_banks	= 4,
> +};

[Severity: High]
Does omitting the irq_bank_map array break external interrupts?

Without an irq_bank_map, drivers/pinctrl/sunxi/pinctrl-sunxi.h:
sunxi_irq_hw_bank_num() returns the sequential logical irqbank indices.

Because the irq_bank offset is already shifted to 1, 2, 3, and 4, these
logical indices will be used as physical hardware bank offsets.

This incorrectly maps PD, PF, and PG to hardware banks 2, 3, and 4 instead
of their actual hardware indices 3, 5, and 6.

Additionally, because PG receives irqbank 4, its out_hwirq is calculated
as 4 * 32 + irqnum (>= 128), which exceeds the bounds of the 128-entry
IRQ domain created during probe.

> +
> +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]
Could this lead to a kernel panic during a probe deferral?

The driver passes the static b288_pinctrl_data descriptor to
sunxi_pinctrl_dt_table_init(), which calls init_pins_table() and
accumulates the total pin count:

    desc->npins += pins_per_bank[i];

If the driver probe defers, desc->npins will double on the next attempt
(from 80 to 160) because the static descriptor is reused.

The pins array is then allocated for 160 elements, but only the first 80
will have their .pin.name populated.

During DT parsing in fill_pin_function(), if an unrecognized pin name is
encountered, the search loops over the oversized array and evaluates:

    if (!strcmp(pins[pin].pin.name, name))
        break;

Iterating past index 79 would call strcmp() with a NULL pointer for
.pin.name, causing a panic.

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