diff mbox series

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

Message ID 20260927151016.186493-5-pmazzini@gmail.com (mailing list archive)
State New
Headers
Series ARM: sunxi: add Allwinner B288 and the PocketBook Verse |

Commit Message

Pablo Mazzini Sept. 27, 2026, 3:10 p.m. UTC 
80 pins across ports B, C, D, F and G; the mux values come from the DT.
Port C has no external interrupts. PH0 is BSP FPGA-only and omitted.

Signed-off-by: Pablo Mazzini <pmazzini@gmail.com>
---
 drivers/pinctrl/sunxi/Kconfig              | 10 ++++
 drivers/pinctrl/sunxi/Makefile             |  1 +
 drivers/pinctrl/sunxi/pinctrl-sun8i-b288.c | 53 ++++++++++++++++++++++
 3 files changed, 64 insertions(+)
 create mode 100644 drivers/pinctrl/sunxi/pinctrl-sun8i-b288.c

Comments

sashiko-bot@kernel.org Sept. 27, 2026, 3:26 p.m. UTC | #1 
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;
diff mbox series

Patch

diff --git a/drivers/pinctrl/sunxi/Kconfig b/drivers/pinctrl/sunxi/Kconfig
index dc62eba96348..a08186133090 100644
--- a/drivers/pinctrl/sunxi/Kconfig
+++ b/drivers/pinctrl/sunxi/Kconfig
@@ -56,6 +56,16 @@  config PINCTRL_SUN8I_A23_R
 	default MACH_SUN8I
 	select PINCTRL_SUNXI
 
+config PINCTRL_SUN8I_B288
+	bool "Support for the Allwinner B288 PIO"
+	default MACH_SUN8I
+	select PINCTRL_SUNXI
+	help
+	  Say Y here to enable pin controller and GPIO support for the
+	  Allwinner B288 (sun8iw10p1) SoC, as used in e-book readers such
+	  as the PocketBook Verse. This covers ports B, C, D, F and G,
+	  including the four banks of external interrupt capable pins.
+
 config PINCTRL_SUN8I_H3
 	bool "Support for the Allwinner H3 PIO"
 	default MACH_SUN8I
diff --git a/drivers/pinctrl/sunxi/Makefile b/drivers/pinctrl/sunxi/Makefile
index 951b3f1e4b4f..5129c0f19f3e 100644
--- a/drivers/pinctrl/sunxi/Makefile
+++ b/drivers/pinctrl/sunxi/Makefile
@@ -18,6 +18,7 @@  obj-$(CONFIG_PINCTRL_SUN50I_A100)	+= pinctrl-sun50i-a100.o
 obj-$(CONFIG_PINCTRL_SUN50I_A100_R)	+= pinctrl-sun50i-a100-r.o
 obj-$(CONFIG_PINCTRL_SUN8I_A83T)	+= pinctrl-sun8i-a83t.o
 obj-$(CONFIG_PINCTRL_SUN8I_A83T_R)	+= pinctrl-sun8i-a83t-r.o
+obj-$(CONFIG_PINCTRL_SUN8I_B288)	+= pinctrl-sun8i-b288.o
 obj-$(CONFIG_PINCTRL_SUN8I_H3)		+= pinctrl-sun8i-h3.o
 obj-$(CONFIG_PINCTRL_SUN8I_H3_R)	+= pinctrl-sun8i-h3-r.o
 obj-$(CONFIG_PINCTRL_SUN8I_V3S)		+= pinctrl-sun8i-v3s.o
diff --git a/drivers/pinctrl/sunxi/pinctrl-sun8i-b288.c b/drivers/pinctrl/sunxi/pinctrl-sun8i-b288.c
new file mode 100644
index 000000000000..98dd4ead4c32
--- /dev/null
+++ b/drivers/pinctrl/sunxi/pinctrl-sun8i-b288.c
@@ -0,0 +1,53 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Allwinner B288 (sun8iw10p1) SoC pinctrl driver.
+ *
+ * Bank sizes and the interrupt mux derived from Allwinner's BSP driver
+ * pinctrl-sun8iw10p1.c, which is:
+ * Copyright (C) 2014 Jackie Hwang <huangshr@allwinnertech.com>
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/pinctrl/pinctrl.h>
+
+#include "pinctrl-sunxi.h"
+
+/* 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 };
+
+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);
+}
+
+static const struct of_device_id b288_pinctrl_match[] = {
+	{ .compatible = "allwinner,sun8i-b288-pinctrl", },
+	{}
+};
+
+static struct platform_driver b288_pinctrl_driver = {
+	.probe	= b288_pinctrl_probe,
+	.driver	= {
+		.name		= "sun8i-b288-pinctrl",
+		.of_match_table	= b288_pinctrl_match,
+	},
+};
+builtin_platform_driver(b288_pinctrl_driver);