Re: [PATCH v5 1/4] pinctrl: mediatek: use devm_gpiochip_add_data() for GPIO chip

[Chen-Yu Tsai <wenst@chromium.org>]

On Thu, Jul 9, 2026 at 2:35 PM Justin Yeh <justin.yeh@mediatek.com> wrote:
>
> The gpio_chip is allocated with device-managed memory but registered with
> the non-managed gpiochip_add_data(). This was harmless while the drivers
> were built-in, but once they can be built as modules and unbound/rmmod'd,
> devm frees the gpio_chip's memory while it is still registered, causing a
> use-after-free.
>
> Register it with devm_gpiochip_add_data() so it shares the same
> device-managed lifecycle, which also lets the manual gpiochip_remove()
> error paths go away.
>
> Fixes: a6df410d420a ("pinctrl: mediatek: Add Pinctrl/GPIO driver for mt8135.")
> Fixes: 805250982bb5 ("pinctrl: mediatek: add pinctrl-paris that implements the vendor dt-bindings")
> Fixes: e78d57b2f87c ("pinctrl: mediatek: add pinctrl-moore that implements the generic pinctrl dt-bindings")
> Signed-off-by: Justin Yeh <justin.yeh@mediatek.com>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

But see below.

> ---
>  drivers/pinctrl/mediatek/pinctrl-moore.c      |  6 ++----
>  drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 14 ++++----------
>  drivers/pinctrl/mediatek/pinctrl-paris.c      |  2 +-
>  3 files changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pinctrl/mediatek/pinctrl-moore.c b/drivers/pinctrl/mediatek/pinctrl-moore.c
> index 17e30f83dc19..38f15dbe9a28 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-moore.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-moore.c
> @@ -594,7 +594,7 @@ static int mtk_build_gpiochip(struct mtk_pinctrl *hw)
>         chip->base              = -1;
>         chip->ngpio             = hw->soc->npins;
>
> -       ret = gpiochip_add_data(chip, hw);
> +       ret = devm_gpiochip_add_data(hw->dev, chip, hw);
>         if (ret < 0)
>                 return ret;
>
> @@ -608,10 +608,8 @@ static int mtk_build_gpiochip(struct mtk_pinctrl *hw)
>         if (!of_property_present(hw->dev->of_node, "gpio-ranges")) {
>                 ret = gpiochip_add_pin_range(chip, dev_name(hw->dev), 0, 0,
>                                              chip->ngpio);
> -               if (ret < 0) {
> -                       gpiochip_remove(chip);
> +               if (ret < 0)
>                         return ret;
> -               }
>         }
>
>         return 0;
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> index dd2c8aa03938..791eddd7a2c6 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> @@ -1130,30 +1130,24 @@ int mtk_pctrl_init(struct platform_device *pdev,
>         pctl->chip->parent = &pdev->dev;
>         pctl->chip->base = -1;
>
> -       ret = gpiochip_add_data(pctl->chip, pctl);
> +       ret = devm_gpiochip_add_data(&pdev->dev, pctl->chip, pctl);
>         if (ret)
>                 return -EINVAL;
>
>         /* Register the GPIO to pin mappings. */
>         ret = gpiochip_add_pin_range(pctl->chip, dev_name(&pdev->dev),
>                         0, 0, pctl->devdata->npins);
> -       if (ret) {
> -               ret = -EINVAL;
> -               goto chip_error;
> -       }
> +       if (ret)
> +               return -EINVAL;
>
>         /* Only initialize EINT if we have EINT pins */
>         if (data->eint_hw.ap_num > 0) {
>                 ret = mtk_eint_init(pctl, pdev);

I think you would also need some matching cleanup function for mtk_eint_init()
or you end up with a dangling IRQ domain and mappings that are active but
have invalid chain handlers.

This would probably be a separate patch.

>                 if (ret)
> -                       goto chip_error;
> +                       return ret;
>         }
>
>         return 0;
> -
> -chip_error:
> -       gpiochip_remove(pctl->chip);
> -       return ret;
>  }
>
>  int mtk_pctrl_common_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c
> index 23f04b24fd65..09098b68f725 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-paris.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c
> @@ -957,7 +957,7 @@ static int mtk_build_gpiochip(struct mtk_pinctrl *hw)
>         chip->base              = -1;
>         chip->ngpio             = hw->soc->npins;
>
> -       ret = gpiochip_add_data(chip, hw);
> +       ret = devm_gpiochip_add_data(hw->dev, chip, hw);
>         if (ret < 0)
>                 return ret;
>
> --
> 2.45.2
>
>