Re: [PATCH 4/5] arm64: dts: apple: t8140: Add pinctrl nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pinctrl: apple: KASAN slab-out-of-bounds read during regmap initialization
- [High] pinctrl: apple: NULL pointer deref in apple_gpio_set_reg due to lazy regmap init
--

commit cd1bf71ed5d4ae5b51b2eed6ad8a3f10605beb2b
Author: Yureka Lilian <yureka@cyberchaos.dev>

arm64: dts: apple: t8140: Add pinctrl nodes

This commit adds the pin controller nodes present on the Apple Silicon T8140
(A18 Pro) SoC. It introduces the new apple,t8140-pinctrl compatible nodes
like pinctrl_nub and pinctrl_ap, which are prerequisites for enabling the
I2C and PCIe controllers.

> diff --git a/arch/arm64/boot/dts/apple/t8140.dtsi b/arch/arm64/boot/dts/apple/t8140.dtsi
> index 150d91bdf6fcc..339a6b4f283d2 100644
> --- a/arch/arm64/boot/dts/apple/t8140.dtsi
> +++ b/arch/arm64/boot/dts/apple/t8140.dtsi

[ ... ]

> @@ -162,6 +169,29 @@ aic: interrupt-controller@301000000 {
>  			power-domains = <&ps_aic>;
>  		};
>  
> +		pinctrl_nub: pinctrl@3081f0000 {
> +			compatible = "apple,t8140-pinctrl";
> +			reg = <0x3 0x081f0000 0x0 0x4000>;
> +			power-domains = <&ps_nub_gpio>;
> +
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&pinctrl_nub 0 0 30>;
> +
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			interrupt-parent = <&aic>;
> +			interrupts = <AIC_IRQ 473 IRQ_TYPE_LEVEL_HIGH>,
> +			             <AIC_IRQ 474 IRQ_TYPE_LEVEL_HIGH>,
> +			             <AIC_IRQ 475 IRQ_TYPE_LEVEL_HIGH>,
> +			             <AIC_IRQ 476 IRQ_TYPE_LEVEL_HIGH>,
> +			             <AIC_IRQ 477 IRQ_TYPE_LEVEL_HIGH>,
> +			             <AIC_IRQ 478 IRQ_TYPE_LEVEL_HIGH>,
> +			             <AIC_IRQ 479 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			apple,npins = <30>;
> +		};
> +

[Severity: High]
This problem wasn't introduced by this patch, but does adding this node with
npins < 512 trigger a KASAN slab-out-of-bounds read in the driver?

When devm_gpiochip_add_data() is called during probe, gpiolib allocates
a 1-word valid_mask for the 30 pins. When get_direction() calls
apple_gpio_get_reg(), it triggers a lazy regmap initialization via
devm_regmap_init_mmio().

Because the driver statically sets num_reg_defaults_raw = 512 and uses
REGCACHE_FLAT, regcache_hw_init() iterates 512 times, calling
apple_gpio_readable_register() for each index:

drivers/pinctrl/pinctrl-apple-gpio.c:apple_gpio_readable_register() {
	...
	return gpiochip_line_is_valid(&pctl->gpio_chip, reg / 4);
}

Will this pass an unconstrained register index up to 511 directly to
gpiochip_line_is_valid(), causing a deterministic KASAN slab-out-of-bounds
read on the 1-word valid_mask?

If the out-of-bounds read returns 1, could the driver also attempt to read
non-existent hardware registers, triggering unhandled Data Aborts or SErrors?

[ ... ]

> @@ -179,6 +209,96 @@ wdt: watchdog@3082b0000 {
>  			interrupts = <AIC_IRQ 507 IRQ_TYPE_LEVEL_HIGH>;
>  		};
>  

[ ... ]

> +		pinctrl_ap: pinctrl@31a000000 {
> +			compatible = "apple,t8140-pinctrl";
> +			reg = <0x3 0x1a000000 0x0 0x100000>;
> +			power-domains = <&ps_gpio>;
> +
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&pinctrl_ap 0 0 216>;
> +			gpio-reserved-ranges = <134 1>;
> +
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			interrupt-parent = <&aic>;
> +			interrupts = <AIC_IRQ 263 IRQ_TYPE_LEVEL_HIGH>,
> +			             <AIC_IRQ 264 IRQ_TYPE_LEVEL_HIGH>,
> +			             <AIC_IRQ 265 IRQ_TYPE_LEVEL_HIGH>,
> +			             <AIC_IRQ 266 IRQ_TYPE_LEVEL_HIGH>,
> +			             <AIC_IRQ 267 IRQ_TYPE_LEVEL_HIGH>,
> +			             <AIC_IRQ 268 IRQ_TYPE_LEVEL_HIGH>,
> +			             <AIC_IRQ 269 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			apple,npins = <216>;
> +		};

[Severity: High]
Does exposing these new pinctrl devices to consumers lead to a NULL pointer
dereference in apple_gpio_set_reg() due to lazy regmap initialization?

drivers/pinctrl/pinctrl-apple-gpio.c:apple_gpio_pinctrl_probe() {
	...
	pctl->pctldev = devm_pinctrl_register(&pdev->dev, &pctl->pinctrl_desc, pctl);
	...
}

Because the pinctrl device is exposed to consumers early before groups,
functions, and the regmap are initialized, what happens if a consumer driver
probes concurrently and requests pins during this window?

drivers/pinctrl/pinctrl-apple-gpio.c:apple_gpio_set_reg() {
	...
	regmap_update_bits(pctl->map, REG_GPIO(pin), mask, value);
	...
}

Will apple_gpio_pinmux_set() call apple_gpio_set_reg() and unconditionally
dereference the uninitialized (NULL) pctl->map, causing a kernel panic on boot?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-apple-a18-pro-dt-pinctrl-i2c-v1-0-6c757966a99e@cyberchaos.dev?part=4