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