[PATCH] pinctrl: pinctrl-generic-mux: Fix provider resource leak on re-parse

[PATCH] pinctrl: pinctrl-generic-mux: Fix provider resource leak on re-parse

[Chancel Liu]

From: Chancel Liu <chancel.liu@nxp.com>

The pinctrl core re-parses a consumer's pinctrl on every probe attempt
(pinctrl_bind_pins() runs before the driver's probe(), and again on each
deferred-probe retry or re-bind), so .dt_node_to_map() must be side-effect
free and safe to call repeatedly.

mux_pinmux_dt_node_to_map() instead allocated provider-side resources on
every call - the mux_pin_function, the group name table and a mux_state
reference, plus a group name and a pinctrl_generic_add_group()
registration via pinctrl_generic_to_map(). All of these are allocated
with devm_*() against the provider device, so they live for the provider's
lifetime and are only released when the provider itself is unbound. The
.dt_free_map() path never touches them; it frees the per-consumer
pinctrl_map only. Every re-parse therefore adds another set of
provider-side allocations that are never reclaimed, so a consumer that
repeatedly defers probe or is re-bound leaks memory and mux_state
references on the provider without bound.

The groups and functions of a board-level mux are static and fully
described by the device tree, so build them once at probe time in the new
mux_pinctrl_probe_dt(). mux_pinmux_dt_node_to_map() then only looks up the
already registered group and builds the per-consumer mux map, which the
core frees via .dt_free_map. As the mux has no pin electrical
configuration, no config map is emitted.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260824022020.2812810-1-chancel.liu%40oss.nxp.com
Fixes: 34acc5a8adfb ("pinctrl: add generic board-level pinctrl driver using mux framework")
Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
Assisted-by: VeroCoder:claude-opus-4-8
---
 drivers/pinctrl/pinctrl-generic-mux.c | 76 ++++++++++++++++++++-------
 1 file changed, 56 insertions(+), 20 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-generic-mux.c b/drivers/pinctrl/pinctrl-generic-mux.c
index 202b72351efb..de85b4e75eb8 100644
--- a/drivers/pinctrl/pinctrl-generic-mux.c
+++ b/drivers/pinctrl/pinctrl-generic-mux.c
@@ -10,13 +10,11 @@
 #include <linux/mutex.h>
 #include <linux/mux/consumer.h>
 #include <linux/platform_device.h>
-#include <linux/pinctrl/pinconf-generic.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
 #include <linux/slab.h>
 
 #include "core.h"
-#include "pinconf.h"
 #include "pinmux.h"
 #include "pinctrl-utils.h"
 
@@ -38,39 +36,73 @@ mux_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
 			  struct pinctrl_map **maps, unsigned int *num_maps)
 {
 	unsigned int num_reserved_maps = 0;
-	struct mux_pin_function *function;
-	const char **group_names;
 	int ret;
 
-	function = devm_kzalloc(pctldev->dev, sizeof(*function), GFP_KERNEL);
-	if (!function)
-		return -ENOMEM;
-
-	group_names = devm_kcalloc(pctldev->dev, 1, sizeof(*group_names), GFP_KERNEL);
-	if (!group_names)
-		return -ENOMEM;
-
-	function->mux_state = devm_mux_state_get_from_np(pctldev->dev, NULL, np_config);
-	if (IS_ERR(function->mux_state))
-		return PTR_ERR(function->mux_state);
+	*maps = NULL;
+	*num_maps = 0;
 
-	ret = pinctrl_generic_to_map(pctldev, np_config, np_config, maps,
-				     num_maps, &num_reserved_maps, group_names,
-				     0, &np_config->name, NULL, 0);
+	if (pinctrl_get_group_selector(pctldev, np_config->name) < 0)
+		return -ENODEV;
 
+	ret = pinctrl_utils_reserve_map(pctldev, maps, &num_reserved_maps,
+					num_maps, 1);
 	if (ret)
 		return ret;
 
-	ret = pinmux_generic_add_function(pctldev, np_config->name, group_names,
-					  1, function);
+	ret = pinctrl_utils_add_map_mux(pctldev, maps, &num_reserved_maps,
+					num_maps, np_config->name,
+					np_config->name);
 	if (ret < 0) {
 		pinctrl_utils_free_map(pctldev, *maps, *num_maps);
+		*maps = NULL;
+		*num_maps = 0;
 		return ret;
 	}
 
 	return 0;
 }
 
+static int mux_pinctrl_probe_dt(struct pinctrl_dev *pctldev,
+				struct device_node *np)
+{
+	struct device *dev = pctldev->dev;
+
+	for_each_available_child_of_node_scoped(np, grp) {
+		struct mux_pin_function *function;
+		const char **group_names;
+		int ret;
+
+		function = devm_kzalloc(dev, sizeof(*function), GFP_KERNEL);
+		if (!function)
+			return -ENOMEM;
+
+		group_names = devm_kcalloc(dev, 1, sizeof(*group_names), GFP_KERNEL);
+		if (!group_names)
+			return -ENOMEM;
+
+		group_names[0] = grp->name;
+
+		function->mux_state = devm_mux_state_get_from_np(dev, NULL, grp);
+		if (IS_ERR(function->mux_state))
+			return dev_err_probe(dev, PTR_ERR(function->mux_state),
+					     "failed to get mux-state for %pOFn\n",
+					     grp);
+
+		ret = pinctrl_generic_add_group(pctldev, grp->name, NULL, 0, NULL);
+		if (ret < 0)
+			return dev_err_probe(dev, ret,
+					     "failed to add group %pOFn\n", grp);
+
+		ret = pinmux_generic_add_function(pctldev, grp->name, group_names,
+						  1, function);
+		if (ret < 0)
+			return dev_err_probe(dev, ret,
+					     "failed to add function %pOFn\n", grp);
+	}
+
+	return 0;
+}
+
 static const struct pinctrl_ops mux_pinctrl_ops = {
 	.get_groups_count = pinctrl_generic_get_group_count,
 	.get_group_name = pinctrl_generic_get_group_name,
@@ -157,6 +189,10 @@ static int mux_pinctrl_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to register pinctrl.\n");
 
+	ret = mux_pinctrl_probe_dt(mpctl->pctl, dev->of_node);
+	if (ret)
+		return ret;
+
 	ret = pinctrl_enable(mpctl->pctl);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to enable pinctrl.\n");
-- 
2.50.1

Re: [PATCH] pinctrl: pinctrl-generic-mux: Fix provider resource leak on re-parse

[Frank Li]

On Wed, Sep 16, 2026 at 12:04:35PM +0900, Chancel Liu wrote:
> From: Chancel Liu <chancel.liu@nxp.com>
>
> The pinctrl core re-parses a consumer's pinctrl on every probe attempt
> (pinctrl_bind_pins() runs before the driver's probe(), and again on each
> deferred-probe retry or re-bind), so .dt_node_to_map() must be side-effect
> free and safe to call repeatedly.
>
> mux_pinmux_dt_node_to_map() instead allocated provider-side resources on
> every call - the mux_pin_function, the group name table and a mux_state
> reference, plus a group name and a pinctrl_generic_add_group()
> registration via pinctrl_generic_to_map(). All of these are allocated
> with devm_*() against the provider device, so they live for the provider's
> lifetime and are only released when the provider itself is unbound. The
> .dt_free_map() path never touches them; it frees the per-consumer
> pinctrl_map only. Every re-parse therefore adds another set of
> provider-side allocations that are never reclaimed, so a consumer that
> repeatedly defers probe or is re-bound leaks memory and mux_state
> references on the provider without bound.
>
> The groups and functions of a board-level mux are static and fully
> described by the device tree, so build them once at probe time in the new
> mux_pinctrl_probe_dt(). mux_pinmux_dt_node_to_map() then only looks up the
> already registered group and builds the per-consumer mux map, which the
> core frees via .dt_free_map. As the mux has no pin electrical
> configuration, no config map is emitted.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260824022020.2812810-1-chancel.liu%40oss.nxp.com
> Fixes: 34acc5a8adfb ("pinctrl: add generic board-level pinctrl driver using mux framework")
> Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
> Assisted-by: VeroCoder:claude-opus-4-8
> ---
>  drivers/pinctrl/pinctrl-generic-mux.c | 76 ++++++++++++++++++++-------
>  1 file changed, 56 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-generic-mux.c b/drivers/pinctrl/pinctrl-generic-mux.c
> index 202b72351efb..de85b4e75eb8 100644
> --- a/drivers/pinctrl/pinctrl-generic-mux.c
> +++ b/drivers/pinctrl/pinctrl-generic-mux.c
> @@ -10,13 +10,11 @@
>  #include <linux/mutex.h>
>  #include <linux/mux/consumer.h>
>  #include <linux/platform_device.h>
> -#include <linux/pinctrl/pinconf-generic.h>
>  #include <linux/pinctrl/pinctrl.h>
>  #include <linux/pinctrl/pinmux.h>
>  #include <linux/slab.h>
>
>  #include "core.h"
> -#include "pinconf.h"
>  #include "pinmux.h"
>  #include "pinctrl-utils.h"
>
> @@ -38,39 +36,73 @@ mux_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
>  			  struct pinctrl_map **maps, unsigned int *num_maps)
>  {
>  	unsigned int num_reserved_maps = 0;
> -	struct mux_pin_function *function;
> -	const char **group_names;
>  	int ret;
>
> -	function = devm_kzalloc(pctldev->dev, sizeof(*function), GFP_KERNEL);
> -	if (!function)
> -		return -ENOMEM;
> -
> -	group_names = devm_kcalloc(pctldev->dev, 1, sizeof(*group_names), GFP_KERNEL);
> -	if (!group_names)
> -		return -ENOMEM;


devm alloc is quite popular under drivers/pinctrl

airoha/pinctrl-airoha.c	dt_node_to_map	devm_kzalloc, devm_pinctrl_register_and_init
meson/pinctrl-amlogic-a4.c	dt_node_to_map	devm_kcalloc
pinctrl-at91.c	at91_dt_node_to_map	devm_kcalloc, devm_kfree
pinctrl-axp209.c	dt_node_to_map	devm_kcalloc
pinctrl-generic-mux.c	mux_pinmux_dt_node_to_map	devm_kcalloc, devm_kzalloc, devm_mux_state_get_from_np
pinctrl-generic.c	pinctrl_generic_dt_node_to_map	devm_kcalloc
pinctrl-single.c	pcs_dt_node_to_map	devm_kcalloc, devm_kfree, devm_kzalloc
pinctrl-st.c	st_pctl_dt_node_to_map	devm_kcalloc, devm_kfree
pinctrl-th1520.c	th1520_pinctrl_dt_node_to_map	devm_kasprintf, devm_kcalloc
pinctrl-tps6594.c	dt_node_to_map	devm_gpio_regmap_register, devm_kzalloc, devm_pinctrl_register
renesas/pinctrl-rza1.c	rza1_dt_node_to_map	devm_kcalloc, devm_kzalloc
renesas/pinctrl-rza2.c	rza2_dt_node_to_map	devm_kcalloc, devm_kzalloc
sophgo/pinctrl-sophgo-common.c	sophgo_pctrl_dt_node_to_map	devm_kasprintf, devm_kcalloc
starfive/pinctrl-starfive-jh7100.c	starfive_dt_node_to_map	devm_kasprintf, devm_kcalloc
starfive/pinctrl-starfive-jh7110.c	jh7110_dt_node_to_map	devm_kasprintf, devm_kcalloc
sunplus/sppctl.c	dt_node_to_map	devm_kcalloc
ti/pinctrl-ti-iodelay.c	ti_iodelay_dt_node_to_map	devm_kcalloc, devm_kfree, devm_kzalloc

does all have similar problem? I suspect this when do this patch, but I
have not tracked these life cycle.

Frank

> -
> -	function->mux_state = devm_mux_state_get_from_np(pctldev->dev, NULL, np_config);
> -	if (IS_ERR(function->mux_state))
> -		return PTR_ERR(function->mux_state);
> +	*maps = NULL;
> +	*num_maps = 0;
>
> -	ret = pinctrl_generic_to_map(pctldev, np_config, np_config, maps,
> -				     num_maps, &num_reserved_maps, group_names,
> -				     0, &np_config->name, NULL, 0);
> +	if (pinctrl_get_group_selector(pctldev, np_config->name) < 0)
> +		return -ENODEV;
>
> +	ret = pinctrl_utils_reserve_map(pctldev, maps, &num_reserved_maps,
> +					num_maps, 1);
>  	if (ret)
>  		return ret;
>
> -	ret = pinmux_generic_add_function(pctldev, np_config->name, group_names,
> -					  1, function);
> +	ret = pinctrl_utils_add_map_mux(pctldev, maps, &num_reserved_maps,
> +					num_maps, np_config->name,
> +					np_config->name);
>  	if (ret < 0) {
>  		pinctrl_utils_free_map(pctldev, *maps, *num_maps);
> +		*maps = NULL;
> +		*num_maps = 0;
>  		return ret;
>  	}
>
>  	return 0;
>  }
>
> +static int mux_pinctrl_probe_dt(struct pinctrl_dev *pctldev,
> +				struct device_node *np)
> +{
> +	struct device *dev = pctldev->dev;
> +
> +	for_each_available_child_of_node_scoped(np, grp) {
> +		struct mux_pin_function *function;
> +		const char **group_names;
> +		int ret;
> +
> +		function = devm_kzalloc(dev, sizeof(*function), GFP_KERNEL);
> +		if (!function)
> +			return -ENOMEM;
> +
> +		group_names = devm_kcalloc(dev, 1, sizeof(*group_names), GFP_KERNEL);
> +		if (!group_names)
> +			return -ENOMEM;
> +
> +		group_names[0] = grp->name;
> +
> +		function->mux_state = devm_mux_state_get_from_np(dev, NULL, grp);
> +		if (IS_ERR(function->mux_state))
> +			return dev_err_probe(dev, PTR_ERR(function->mux_state),
> +					     "failed to get mux-state for %pOFn\n",
> +					     grp);
> +
> +		ret = pinctrl_generic_add_group(pctldev, grp->name, NULL, 0, NULL);
> +		if (ret < 0)
> +			return dev_err_probe(dev, ret,
> +					     "failed to add group %pOFn\n", grp);
> +
> +		ret = pinmux_generic_add_function(pctldev, grp->name, group_names,
> +						  1, function);
> +		if (ret < 0)
> +			return dev_err_probe(dev, ret,
> +					     "failed to add function %pOFn\n", grp);
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct pinctrl_ops mux_pinctrl_ops = {
>  	.get_groups_count = pinctrl_generic_get_group_count,
>  	.get_group_name = pinctrl_generic_get_group_name,
> @@ -157,6 +189,10 @@ static int mux_pinctrl_probe(struct platform_device *pdev)
>  	if (ret)
>  		return dev_err_probe(dev, ret, "Failed to register pinctrl.\n");
>
> +	ret = mux_pinctrl_probe_dt(mpctl->pctl, dev->of_node);
> +	if (ret)
> +		return ret;
> +
>  	ret = pinctrl_enable(mpctl->pctl);
>  	if (ret)
>  		return dev_err_probe(dev, ret, "Failed to enable pinctrl.\n");
> --
> 2.50.1
>

Re: [PATCH] pinctrl: pinctrl-generic-mux: Fix provider resource leak on re-parse

[Chancel Liu]

>> @@ -38,39 +36,73 @@ mux_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
>>  			  struct pinctrl_map **maps, unsigned int *num_maps)
>>  {
>>  	unsigned int num_reserved_maps = 0;
>> -	struct mux_pin_function *function;
>> -	const char **group_names;
>>  	int ret;
>>
>> -	function = devm_kzalloc(pctldev->dev, sizeof(*function), GFP_KERNEL);
>> -	if (!function)
>> -		return -ENOMEM;
>> -
>> -	group_names = devm_kcalloc(pctldev->dev, 1, sizeof(*group_names), GFP_KERNEL);
>> -	if (!group_names)
>> -		return -ENOMEM;
> 
> 
> devm alloc is quite popular under drivers/pinctrl
> 
> airoha/pinctrl-airoha.c	dt_node_to_map	devm_kzalloc, devm_pinctrl_register_and_init
> meson/pinctrl-amlogic-a4.c	dt_node_to_map	devm_kcalloc
> pinctrl-at91.c	at91_dt_node_to_map	devm_kcalloc, devm_kfree
> pinctrl-axp209.c	dt_node_to_map	devm_kcalloc
> pinctrl-generic-mux.c	mux_pinmux_dt_node_to_map	devm_kcalloc, devm_kzalloc, devm_mux_state_get_from_np
> pinctrl-generic.c	pinctrl_generic_dt_node_to_map	devm_kcalloc
> pinctrl-single.c	pcs_dt_node_to_map	devm_kcalloc, devm_kfree, devm_kzalloc
> pinctrl-st.c	st_pctl_dt_node_to_map	devm_kcalloc, devm_kfree
> pinctrl-th1520.c	th1520_pinctrl_dt_node_to_map	devm_kasprintf, devm_kcalloc
> pinctrl-tps6594.c	dt_node_to_map	devm_gpio_regmap_register, devm_kzalloc, devm_pinctrl_register
> renesas/pinctrl-rza1.c	rza1_dt_node_to_map	devm_kcalloc, devm_kzalloc
> renesas/pinctrl-rza2.c	rza2_dt_node_to_map	devm_kcalloc, devm_kzalloc
> sophgo/pinctrl-sophgo-common.c	sophgo_pctrl_dt_node_to_map	devm_kasprintf, devm_kcalloc
> starfive/pinctrl-starfive-jh7100.c	starfive_dt_node_to_map	devm_kasprintf, devm_kcalloc
> starfive/pinctrl-starfive-jh7110.c	jh7110_dt_node_to_map	devm_kasprintf, devm_kcalloc
> sunplus/sppctl.c	dt_node_to_map	devm_kcalloc
> ti/pinctrl-ti-iodelay.c	ti_iodelay_dt_node_to_map	devm_kcalloc, devm_kfree, devm_kzalloc
> 
> does all have similar problem? I suspect this when do this patch, but I
> have not tracked these life cycle.
> 
> Frank
> 

Not all, but many do. devm_* in .dt_node_to_map() only leaks when the
allocation is bound to the provider dev and isn't reclaimed by
.dt_free_map(). So it accumulates on every re-parse.

Here's the calling:
really_probe()
  └─ pinctrl_bind_pins(dev)
       └─ devm_pinctrl_get(dev)
            └─ create_pinctrl()
                 └─ pinctrl_dt_to_map(p, pctldev)
                      └─ dt_to_map_one_config()
                           └─ ops->dt_node_to_map()

Every deferred-probe retry and every re-bind re-invokes .dt_node_to_map().

Regards,
Chancel Liu