[PATCH v5 1/4] pinctrl: mediatek: use devm_gpiochip_add_data() for GPIO chip
Chen-Yu Tsai
wenst at chromium.org
Thu Jul 9 01:26:36 PDT 2026
On Thu, Jul 9, 2026 at 2:35 PM Justin Yeh <justin.yeh at 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 at mediatek.com>
Reviewed-by: Chen-Yu Tsai <wenst at 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
>
>
More information about the Linux-mediatek
mailing list