Re: [PATCH v3 1/2] pinctrl: imx: answer OUTPUT_ENABLE/INPUT_ENABLE queries from the pad register

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing bounds check for `pin_id` in `imx_pinconf_set()`, `imx_pinconf_set_mmio()`, and GPIO direction callbacks allows out-of-bounds array access via malformed device tree inputs.
- [High] Missing bounds check during device tree parsing in `imx_pinctrl_parse_pin_mmio()` and `imx_pinctrl_parse_pin_scu()`.
- [High] Unprotected Read-Modify-Write (RMW) on shared general-purpose IOMUXC registers causes a race condition.
--

commit fdadfc4d7f207e1b65da9199ace383a6f7c5d92a
Author: Mehmet Fide <mehmet.fide@screeningeagle.com>

    pinctrl: imx: answer OUTPUT_ENABLE/INPUT_ENABLE queries from the pad register
    
This patch updates the imx pinctrl driver to decode requested parameters and
answer PIN_CONFIG_OUTPUT_ENABLE and PIN_CONFIG_INPUT_ENABLE on SoCs that
declare where those bits live in the pad register. It also adds bounds checking
to ensure that pin indices coming from gpio ranges do not read past the pin_regs
array during pin configuration gets.

> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
> index 9a45b376d36f9..1bcb2f772d380 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
[ ... ]
> @@ -312,16 +313,63 @@ static int imx_pinconf_get_mmio(struct pinctrl_dev *pctldev, unsigned pin_id,
[ ... ]
>  static int imx_pinconf_get(struct pinctrl_dev *pctldev,
>  			   unsigned pin_id, unsigned long *config)
>  {
>  	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
>  	const struct imx_pinctrl_soc_info *info = ipctl->info;
>  
> +	if (info->flags & IMX_USE_SCU)
> +		return -ENOTSUPP;
> +	if (pin_id >= info->npins)
> +		return -EINVAL;
> +

[Severity: High]
This isn't a bug introduced by this patch, but since you are adding bounds
checking here, does imx_pinconf_set() also need this verification?

Without validation, a malformed gpio-ranges property in the device tree could
translate to an invalid pin_id. Since imx_pinconf_set() lacks the
if (pin_id >= info->npins) check, this unvalidated pin_id could cause an
out-of-bounds array access in imx_pinconf_set_mmio():

	const struct imx_pin_reg *pin_reg = &ipctl->pin_regs[pin_id];

Could the same missing bounds check also cause out-of-bounds array accesses to
ipctl->pin_regs in the GPIO direction callbacks like imx_pmx_gpio_set_direction()
and vf610_pmx_gpio_set_direction()?

[Severity: High]
This is another pre-existing issue, but does imx_pinctrl_parse_pin_mmio() also
lack bounds checking for *pin_id during device tree parsing?

The pin_id is derived algebraically from raw device tree properties:

	*pin_id = (mux_reg != -1) ? mux_reg / 4 : conf_reg / 4;
	pin_reg = &ipctl->pin_regs[*pin_id];
	pin->pin = *pin_id;
	pin_reg->mux_reg = mux_reg;
	pin_reg->conf_reg = conf_reg;

If the parsed *pin_id exceeds info->npins, this uses the unverified index
directly on the ipctl->pin_regs array, leading to an immediate out-of-bounds
memory write.

Does imx_pinctrl_parse_pin_scu() have a similar vulnerability when indexing the
info->pins array with an unverified pin->pin?

[Severity: High]
This is a pre-existing issue, but can concurrent driver probes cause
hardware state corruption of the shared input_reg in imx_pmx_set_one_pin_mmio()?

There is an unprotected read-modify-write on shared general-purpose IOMUXC
registers:

	val = readl(ipctl->base + pin_mmio->input_reg);
	val &= ~mask;
	val |= select << shift;
	writel(val, ipctl->base + pin_mmio->input_reg);

If two drivers concurrently request pinctrl states that share the same
general-purpose register, and the pinctrl core calls pinmux_enable_setting()
locklessly, will these RMW operations race and overwrite each other's
configurations?

> +	return imx_pinconf_get_mmio(pctldev, pin_id, config);
> +}
> +
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902154525.4090276-1-mehmet.fide@gmail.com?part=1