Re: [PATCH v4 1/3] 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 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Unvalidated `pin_id` derived from the device tree causes out-of-bounds access on `ipctl->pin_regs` and `info->pins`.
--

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

pinctrl: imx: answer OUTPUT_ENABLE/INPUT_ENABLE queries from the pad register

This patch updates the i.MX pinctrl driver to decode generic pinconfig
parameters for input and output enable. It answers these queries using the pad
register bits defined for specific SoCs. This allows chips like Vybrid to
report line directions correctly to gpio-mmio.

> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
> index 9a45b376d36f9..f08ef52bf4f85 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.c

[ ... ]

> @@ -312,16 +313,61 @@ static int imx_pinconf_get_mmio(struct pinctrl_dev *pctldev, unsigned pin_id,
>  	return 0;
>  }
>  
> +static int imx_pinconf_get_mmio(struct pinctrl_dev *pctldev,
> +				unsigned int pin_id, unsigned long *config)
> +{
> +	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +	const struct imx_pinctrl_soc_info *info = ipctl->info;
> +	const struct imx_pin_reg *pin_reg = &ipctl->pin_regs[pin_id];

[Severity: Critical]
This is a pre-existing issue, but can an unvalidated pin_id derived from the
device tree cause an out-of-bounds access on ipctl->pin_regs here?

During device probe, in drivers/pinctrl/freescale/pinctrl-imx.c in the
imx_pinctrl_parse_pin_mmio() function, the pin_id is computed directly from
register values in the fsl,pins property without bounding it to info->npins:

	*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 an improperly formatted or malicious device tree provides an excessively
large mux_reg or conf_reg, couldn't this lead to out-of-bounds memory
corruption, cascading into functions like imx_pinconf_get_mmio() which
implicitly assume pin_id is within bounds?

> +	enum pin_config_param param = pinconf_to_config_param(*config);
> +	unsigned int mask;
> +	u32 raw;

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