Re: [PATCH] regulator: max20086: fix invalid memory access

[Cosmin Tanislav <demonsingur@gmail.com>]

On 5/8/25 9:49 AM, Cosmin Tanislav wrote:
> max20086_parse_regulators_dt() calls of_regulator_match() using an
> array of struct of_regulator_match allocated on the stack for the
> matches argument.
> 
> of_regulator_match() calls devm_of_regulator_put_matches(), which calls
> devres_alloc() to allocate a struct devm_of_regulator_matches which will
> be de-allocated using devm_of_regulator_put_matches().
> 
> struct devm_of_regulator_matches is populated with the stack allocated
> matches array.
> 
> If the device fails to probe, devm_of_regulator_put_matches() will be
> called and will try to call of_node_put() on that stack pointer,
> generating the following dmesg entries:
> 
> max20086 6-0028: Failed to read DEVICE_ID reg: -121
> kobject: '\xc0$\xa5\x03' (000000002cebcb7a): is not initialized, yet
> kobject_put() is being called.
> 
> Followed by a stack trace matching the call flow described above.
> 
> Switch to allocating the matches array using devm_kcalloc() to
> avoid accessing the stack pointer long after it's out of scope.
> 
> This also has the advantage of allowing multiple max20086 to probe
> without overriding the data stored inside the global of_regulator_match.
> 

I've made an error, the above paragraph is not relevant here, although
it is relevant for other usages of of_regulator_match() where the struct
of_regulator_match is a global array, which causes the data stored in it
to be overridden if another device using the same driver probes again,
which could cause of_node_put() to be called twice on the same of_node.

I'm sure that's not a common issue since it hasn't been fixed yet.

> Fixes: bfff546aae50 ("regulator: Add MAX20086-MAX20089 driver")
> Signed-off-by: Cosmin Tanislav <demonsingur@gmail.com>
> ---
>   drivers/regulator/max20086-regulator.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/regulator/max20086-regulator.c b/drivers/regulator/max20086-regulator.c
> index 59eb23d467ec..198d45f8e884 100644
> --- a/drivers/regulator/max20086-regulator.c
> +++ b/drivers/regulator/max20086-regulator.c
> @@ -132,7 +132,7 @@ static int max20086_regulators_register(struct max20086 *chip)
>   
>   static int max20086_parse_regulators_dt(struct max20086 *chip, bool *boot_on)
>   {
> -	struct of_regulator_match matches[MAX20086_MAX_REGULATORS] = { };
> +	struct of_regulator_match *matches;
>   	struct device_node *node;
>   	unsigned int i;
>   	int ret;
> @@ -143,6 +143,11 @@ static int max20086_parse_regulators_dt(struct max20086 *chip, bool *boot_on)
>   		return -ENODEV;
>   	}
>   
> +	matches = devm_kcalloc(chip->dev, chip->info->num_outputs,
> +			       sizeof(*matches), GFP_KERNEL);
> +	if (!matches)
> +		return -ENOMEM;
> +
>   	for (i = 0; i < chip->info->num_outputs; ++i)
>   		matches[i].name = max20086_output_names[i];
>