Re: [PATCH v6 15/17] pinctrl: airoha: add support of en7523 SoC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Missing and incorrect electrical configuration (pull-up, pull-down, drive strength, open-drain) mappings for dedicated IO pins.
- [Low] The device tree binding example violates the schema's `const` constraint by using an incorrect compatible string.

Pre-existing issues:
- [High] Use-After-Free (UAF) of irq_domain in the interrupt handler on driver unbind due to incorrect devm registration order.
--

commit 11bb118efd0b8683f86390269dd15e4752a654e8
Author: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
Subject: pinctrl: airoha: add support of en7523 SoC

This patch adds support for the Airoha EN7523 SoC pin controller and its
corresponding device tree binding schema.

> diff --git a/Documentation/devicetree/bindings/pinctrl/airoha,en7523-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/airoha,en7523-pinctrl.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/airoha,en7523-pinctrl.yaml
[ ... ]
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    pinctrl {
> +      compatible = "airoha,en7521-pinctrl";

[Severity: Low]
Does this compatible string match the schema's const constraint? The binding
definition specifies "airoha,en7523-pinctrl", so it seems this will cause a
dt_binding_check validation failure on the example block.

> diff --git a/drivers/pinctrl/airoha/pinctrl-en7523.c b/drivers/pinctrl/airoha/pinctrl-en7523.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/pinctrl/airoha/pinctrl-en7523.c
[ ... ]
> +static const struct airoha_pinctrl_conf pinctrl_pullup_conf[] = {
> +	PINCTRL_CONF_DESC(12, REG_GPIO_L_PU, BIT(0)),
> +	PINCTRL_CONF_DESC(13, REG_GPIO_L_PU, BIT(1)),
[ ... ]
> +	PINCTRL_CONF_DESC(40, REG_GPIO_L_PU, BIT(28)),
> +	PINCTRL_CONF_DESC(41, REG_GPIO_L_PU, BIT(29)),
> +};

[Severity: High]
Are we missing electrical configuration mappings for pins 2-9 here?

Also, are pins 40-41 being mapped to the wrong hardware registers? They appear
to be mapped to REG_GPIO_L_PU instead of their dedicated hardware registers
(e.g., REG_I2C_SDA_PU).

[ ... ]
> +static const struct airoha_pinctrl_match_data pinctrl_match_data = {
> +	.pinctrl_name = KBUILD_MODNAME,
> +	.pinctrl_owner = THIS_MODULE,
> +	.pins = pinctrl_pins,
[ ... ]
> +	.confs_info = {
> +		[AIROHA_PINCTRL_CONFS_PULLUP] = {
> +			.confs = pinctrl_pullup_conf,
> +			.num_confs = ARRAY_SIZE(pinctrl_pullup_conf),
> +		},
[ ... ]
> +		[AIROHA_PINCTRL_CONFS_DRIVE_E4] = {
> +			.confs = pinctrl_drive_e4_conf,
> +			.num_confs = ARRAY_SIZE(pinctrl_drive_e4_conf),
> +		},
> +	},
> +};

[Severity: High]
Should AIROHA_PINCTRL_CONFS_PCIE_RST_OD be included in this initialization?

Without it, applying standard device tree pin configurations (such as
drive-open-drain on PCIe resets) will fail with -EINVAL, causing driver
probe failures for dependent hardware like PCIe.

[ ... ]
> +static struct platform_driver airoha_pinctrl_driver = {
> +	.probe = airoha_pinctrl_probe,
> +	.driver = {
> +		.name = "pinctrl-airoha-en7523",
> +		.of_match_table = airoha_pinctrl_of_match,
> +	},
> +};

[Severity: High]
This is a pre-existing issue, but looking at how airoha_pinctrl_probe()
operates in the shared drivers/pinctrl/airoha/pinctrl-airoha.c:

drivers/pinctrl/airoha/pinctrl-airoha.c:airoha_pinctrl_add_gpiochip() {
    err = devm_request_irq(dev, irq, airoha_irq_handler, IRQF_SHARED,
                           dev_name(dev), pinctrl);
    ...
    return devm_gpiochip_add_data(dev, gc, pinctrl);
}

Does this lifecycle teardown sequence create a use-after-free on module unbind?

Because devres frees in reverse order, devm_gpiochip_add_data() is undone
first (freeing the irq_domain), and the irq handler is freed second. If an
interrupt fires in this window, airoha_irq_handler() will dereference the
freed girq->domain via irq_find_mapping(), leading to a kernel crash.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628143733.273651-1-mikhail.kshevetskiy@iopsys.eu?part=15