Re: [PATCH v7 7/9] pinctrl: mediatek: mt6397: Add MediaTek MT6392

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing `.remove` callback in `mtk_pinctrl_driver` causes a Use-After-Free of devres-managed memory upon driver unbind because the GPIO chip is registered with an unmanaged API.
--

commit a353f51e9f14248ddf84f2093bfe6ddad0c83ce8
Author: Luca Leonardo Scorcia <l.scorcia@gmail.com>

pinctrl: mediatek: mt6397: Add MediaTek MT6392

This commit adds support for the MT6392 pinctrl device by introducing
its pin definitions and match data. It also updates the MT6397 driver
to retrieve and use this device data from the match table during probe.

> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt6397.c b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> index 03d0f65d7bcc1..8ba02e70595ce 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt6397.c

[ ... ]

> @@ -40,13 +62,24 @@ static const struct mtk_pinctrl_devdata mt6397_pinctrl_data = {
>  static int mt6397_pinctrl_probe(struct platform_device *pdev)
>  {
>  	struct mt6397_chip *mt6397;
> +	const struct mtk_pinctrl_devdata *data;
> +
> +	data = device_get_match_data(&pdev->dev);
> +	if (!data)
> +		return -ENOENT;
>  
>  	mt6397 = dev_get_drvdata(pdev->dev.parent);
> -	return mtk_pctrl_init(pdev, &mt6397_pinctrl_data, mt6397->regmap);
> +	return mtk_pctrl_init(pdev, data, mt6397->regmap);
>  }

[Severity: High]
This is a pre-existing issue, but by sharing the same initialization path,
the new MT6392 variant inherits a potential use-after-free on driver unbind.

Looking at mtk_pctrl_init() in pinctrl-mtk-common.c, it allocates pctl->chip
using devm_kzalloc() and registers it with the unmanaged gpiochip_add_data()
API:

mtk_pctrl_init() {
    ...
    pctl->chip = devm_kzalloc(&pdev->dev, sizeof(*pctl->chip), GFP_KERNEL);
    ...
    ret = gpiochip_add_data(pctl->chip, pctl);
    ...
}

However, the mtk_pinctrl_driver struct does not have a .remove callback and
does not set suppress_bind_attrs = true.

If a user manually unbinds the driver via sysfs, devres will automatically
free pctl->chip, but since gpiochip_remove() is never called, the GPIO
framework retains pointers to the freed memory.

Would it make sense to fix this by switching to devm_gpiochip_add_data() in
mtk_pctrl_init(), or by adding suppress_bind_attrs = true to the driver
struct?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615071836.362883-1-l.scorcia@gmail.com?part=7